Bug 55430 - UDM REST API fetches objects twice on certain operations
UDM REST API fetches objects twice on certain operations
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UDM - REST API
UCS 5.0
Other Linux
: P5 normal (vote)
: UCS 5.0-2-errata
Assigned To: Florian Best
Iván.Delgado
https://git.knut.univention.de/univen...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2022-11-16 14:06 CET by Florian Best
Modified: 2022-11-30 13:27 CET (History)
2 users (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): Large environments, UCS Performance
Max CVSS v3 score:


Attachments
Screenshot performance comparision (51.23 KB, image/png)
2022-11-16 14:06 CET, Florian Best
Details
Screenshot performance comparision (142.46 KB, image/png)
2022-11-17 11:16 CET, Florian Best
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2022-11-16 14:06:47 CET
Created attachment 11012 [details]
Screenshot performance comparision

The UDM REST API fetched objects twice from the LDAP server when it wants to remove them.
This costs unnecessary performance and should be optimized. Attached is a screenshot of performance comparison.
Comment 2 Florian Best univentionstaff 2022-11-17 11:14:24 CET
actually affected are all of `GET /{object_type}/{dn}`, `PATCH /{object_type}/{dn}` and `DELETE /{object_type}/{dn}`.
Comment 3 Florian Best univentionstaff 2022-11-17 11:16:22 CET
Created attachment 11013 [details]
Screenshot performance comparision

The performance comparison shows a trend that it improves much. The tests are not completely deterministic as the users had different amount of assigned groups.
Comment 4 Florian Best univentionstaff 2022-11-17 13:00:03 CET
Objects are now fetched only once:

univention-directory-manager-rest.yaml
237981aa2f59 | perf(udm-rest): prevent duplicated LDAP object receival

univention-directory-manager-rest (10.0.4-8)
237981aa2f59 | perf(udm-rest): prevent duplicated LDAP object receival
Comment 5 Florian Best univentionstaff 2022-11-25 15:20:55 CET
The changes caused a exception in the users/self module:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/tornado/web.py", line 1592, in _execute
    result = yield result
  File "/usr/lib/python3/dist-packages/tornado/gen.py", line 1133, in run
    value = future.result()
  File "/usr/lib/python3/dist-packages/univention/admin/rest/module.py", line 3006, in get
    module, obj = await self.pool_submit(self.get_module_object, object_type, dn)
  File "/usr/lib/python3/dist-packages/tornado/gen.py", line 1141, in run
    yielded = self.gen.throw(*exc_info)
  File "/usr/lib/python3/dist-packages/univention/admin/rest/module.py", line 401, in pool_submit
    return (yield future)
  File "/usr/lib/python3/dist-packages/tornado/gen.py", line 1133, in run
    value = future.result()
  File "/usr/lib/python3.7/concurrent/futures/_base.py", line 425, in result
    return self.__get_result()
  File "/usr/lib/python3.7/concurrent/futures/_base.py", line 384, in __get_result
    raise self._exception
  File "/usr/lib/python3.7/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/lib/python3/dist-packages/univention/admin/rest/module.py", line 500, in get_module_object
    obj = module.get(dn)
  File "/usr/lib/python3/dist-packages/univention/management/console/modules/udm/udm_ldap.py", line 719, in get
    obj = self.module.object(None, ldap_connection, None, ldap_dn, superordinate, attributes=attributes)
  File "/usr/lib/python3/dist-packages/univention/admin/handlers/users/user.py", line 1124, in __init__
    univention.admin.handlers.simpleLdap.__init__(self, co, lo, position, dn, superordinate, attributes=attributes)
  File "/usr/lib/python3/dist-packages/univention/admin/handlers/__init__.py", line 221, in __init__
    attr = self._ldap_attributes()
  File "/usr/lib/python3/dist-packages/univention/admin/handlers/users/user.py", line 2046, in _ldap_attributes
    return super(object, cls)._ldap_attributes() + ['pwdAccountLockedTime']
TypeError: super(type, obj): obj must be an instance or subtype of type

Reproducer via some requests:
curl -H 'Accept: application/json' http://Administrator:univention@localhost/univention/udm/users/user/add > /dev/null
curl -H 'Accept: application/json' http://Administrator:univention@localhost/univention/udm/users/self/uid=Administrator,cn=users,l=school,l=dev | python -m json.tool
    
Reproducer via python script:
> import univention.admin.uldap
> lo,po=univention.admin.uldap.getMachineConnection()
> univention.admin.modules.init(lo, po, univention.admin.modules.get('users/user'), force_reload=True)
> univention.admin.modules.get('users/self').object(None, lo, None, 'uid=Administrator,cn=users,l=school,l=dev')

Producing:
    Traceback (most recent call last):
      File "repr.py", line 8, in <module>
        univention.admin.modules.get('users/self').object(None, lo, None, 'uid=Administrator,cn=users,l=school,l=dev')
      File "/usr/lib/python3/dist-packages/univention/admin/handlers/users/user.py", line 1124, in __init__
        univention.admin.handlers.simpleLdap.__init__(self, co, lo, position, dn, superordinate, attributes=attributes)
      File "/usr/lib/python3/dist-packages/univention/admin/handlers/__init__.py", line 221, in __init__
        attr = self._ldap_attributes()
      File "/usr/lib/python3/dist-packages/univention/admin/handlers/users/user.py", line 2046, in _ldap_attributes
        return super(object, cls)._ldap_attributes() + ['pwdAccountLockedTime']
    TypeError: super(type, obj): obj must be an instance or subtype of type

This was casued by users/self inheriting from users/user.
`modules.init('users/user', force_reload=True)` reloads the python module, which causes that users/self still has a reference to the old users/user object instance.

We can (probably get rid of the implemented workaround when we drop Python 2 support because calling `super()` instead of `super(obj, cls)` in all places in users/user will detect this situation correctly.

The users/self module has also been optimized and modernized.
A slight change in the UDM REST API response happens: users/self now returns "objectType: users/self" instead of "users/user" - which is more correct.

univention-directory-manager-modules.yaml
9e997003cd70 | feat(udm users/self): complement users/self module

univention-directory-manager-modules (15.0.13-25)
9e997003cd70 | feat(udm users/self): complement users/self module
47dc565dae27 | fix(udm users/self): fix inheritance errors after reloading python modules
Comment 6 Dirk Wiesenthal univentionstaff 2022-11-28 16:55:11 CET
OK: Ivan tested the changes
OK: YAML