Univention Bugzilla – Bug 33344
wrong referenced policy is displayed
Last modified: 2016-05-04 18:10:30 CEST
When opening more than one policy the referenced policy is always displayed as the same. To reproduce: * create 2 UCR policies * reference them to different objects (e.g. to LDAP-base and cn=computers) * open both policies one after the other * on the "Referencing objects" tab the referenced object from the first opened policy is displayed
It seems that this is a error in the backend. The information is stored as static class member on the LDAP_Search syntax.
It seems also that the wrong polcies are also displayed in the policy tab of any udm object.
*** Bug 33367 has been marked as a duplicate of this bug. ***
*** Bug 34681 has been marked as a duplicate of this bug. ***
As noted in Bug #34681: The frontend cache is most likely the cause of this bug. It caches the properties for each module. Among them the LDAP_Search syntax, which now has its ldap_dn "hardcoded" for all instances of this UDM module
I stumble upon this issue quite often in projects where I want to review and modify policies. Reviewing and debugging policies becomes extremely hard through this issue.
Also: open a policy, change object to use the policy, open policy again → reference is not shown (due to frontend cache).
(In reply to Dirk Wiesenthal from comment #5) > As noted in Bug #34681: The frontend cache is most likely the cause of this > bug. It caches the properties for each module. Among them the LDAP_Search > syntax, which now has its ldap_dn "hardcoded" for all instances of this UDM > module Yes, it seems to be the frontend cache. I would really like to see the referenced objects being send along with a udm/get request of the actual policy. The current implementation via pseudo syntax leads to awkward workarounds both on the frontend or the backend side. A module's list of properties and a module's layout should be static values that can be easily cached. In current implementations, the DN always needs to be send for a list of module properties. Are there any problems that such a change could lead to?
> Reviewing and debugging policies becomes extremely hard through this issue. +1 Additionally, it's very confusing for customers who try to solve policy issues in their own and don't know about this bug. Setting Version to 4.1, because it's still broken there.
Noticed by a customer, mentioned on summit.
Created attachment 7582 [details] Proposed patch
(In reply to Jürn Brodersen from comment #12) > Created attachment 7582 [details] > Proposed patch Can you explain a little bit why you moved logic from the backend into the frontend? At least the ldap-filter is broken in some cases as the value of the policy DN is not escaped properly. I'll have a closer look tomorrow.
(In reply to Florian Best from comment #13) > (In reply to Jürn Brodersen from comment #12) > > Created attachment 7582 [details] > > Proposed patch > Can you explain a little bit why you moved logic from the backend into the > frontend? See comment #9: > [...] The current implementation via pseudo > syntax leads to awkward workarounds both on the frontend or the backend > side. A module's list of properties and a module's layout should be static > values that can be easily cached. In current implementations, the DN always > needs to be send for a list of module properties. Jürn actually just moved the logic from the backend to the frontend. In this way the policy property/layout information can still be cached, yet the search for referenced objects of the specific policy can be triggered for each policy separately. > At least the ldap-filter is broken in some cases as the value of the policy > DN is not escaped properly. > I'll have a closer look tomorrow. In fact, the current fix does not introduce any regression as the filter had not been escaped before. However, I agree that it would make sense to include an additional UMC command (e.g., "udm/policy/referenced_objects") which expects an DN and returns all referenced objects. The escaping could then be done easily in the backend.
Created attachment 7587 [details] proposed patch 2 I added a new umc command that returns all objects that are referenced by a given policy and use that command instead of building the LDAP filter in js. For escaping the input "LDAPSearchSanitizer" is used.
Created attachment 7588 [details] patch for po files
(In reply to Jürn Brodersen from comment #15) > Created attachment 7587 [details] > proposed patch 2 > > I added a new umc command that returns all objects that are referenced by a > given policy and use that command instead of building the LDAP filter in js. > For escaping the input "LDAPSearchSanitizer" is used. This patch looks very nice! I have an idea for improvement: Wouldn't it be better to add these values as key "$references$" into the udm/get call? Then the object representation would contain these values directly and one wouldn't need an extra UMC-call. Also it would be a little bit more generic in the naming as it is not necessarily bound to policies. Maybe one day we want to display also the network of a computer object in that way, etc.
Created attachment 7606 [details] proposed patch 3 (use of udm/get) This patch uses the existing udm/get command and adds a $references$ key to it. As discussed with Florian (comment 17) Also the reverencing objects tab is not shown for new policies anymore.
Created attachment 7608 [details] patch (In reply to Jürn Brodersen from comment #18) > Created attachment 7606 [details] > proposed patch 3 (use of udm/get) > > This patch uses the existing udm/get command and adds a $references$ key to > it. As discussed with Florian (comment 17) > > Also the reverencing objects tab is not shown for new policies anymore. Yes, that looks nicer. Can you have a look at my attached patch? It does it a more extensible manner. But there is a javascript exception when I try to this._addSubTab() it.
Created attachment 7619 [details] define widget in the backend Adding the referencing objects page from the frontend was problematic because it was not easily possible to decide early enough if the tab should be shown or not. As discussed I put the logic back in to the backend and added a widget (based on the LinkList widget) to the umc package for showing referencing objects.
(In reply to Jürn Brodersen from comment #20) > Created attachment 7619 [details] > define widget in the backend > > Adding the referencing objects page from the frontend was problematic > because it was not easily possible to decide early enough if the tab should > be shown or not. > > As discussed I put the logic back in to the backend and added a widget > (based on the LinkList widget) to the umc package for showing referencing > objects. That looks nice! The _setValueAttr() function in the new ReferencingObjects widget should replace all current childs. (e.g. if set('value', ...) is called twice it would be duplicated).
(In reply to Jürn Brodersen from comment #20) > Created attachment 7619 [details] > define widget in the backend > > Adding the referencing objects page from the frontend was problematic > because it was not easily possible to decide early enough if the tab should > be shown or not. > > As discussed I put the logic back in to the backend and added a widget > (based on the LinkList widget) to the umc package for showing referencing > objects. Nice :) ! You need call to this._set('value', value) at the end of _setValueAttr(). This is needed for the internal watch-mechanism to work properly. _set() will call registered observers for a value and will update this.value, as well: https://dojotoolkit.org/documentation/tutorials/1.8/understanding_widgetbase/
Created attachment 7623 [details] Proposed patch New Patch
UCS4.1-1: r68915: show the right referenced objects for policies r68916: YAML Package: univention-management-console-module-udm Version: 6.0.11-16.646.201604261629
### jshint udm/ReferencingObjects.js ### Extra comma. (udm/ReferencingObjects.js:40:34) > "umc/widgets/ContainerWidget", Extra comma. (udm/ReferencingObjects.js:42:75) > return declare("umc.modules.udm.ReferencingObjects", [ ContainerWidget, ], { Unnecessary semicolon. (udm/ReferencingObjects.js:64:14) > }; 'on' is defined but never used. (udm/ReferencingObjects.js:41:37) > ], function(declare, lang, array, on, topic, Button, tools, app, ContainerWidget) {
UCS4.1-1: r69017: code cleanup r69018: YAML Package: univention-management-console-module-udm Version: 6.0.11-17.647.201604291153
Ok, the changes are now very nice. YAML: adjusted in svn r68987
<http://errata.software-univention.de/ucs/4.1/167.html>