Yair Zaslavsky has posted comments on this change. Change subject: aaa: Fix sync ......................................................................
Patch Set 10: (3 comments) http://gerrit.ovirt.org/#/c/28561/10/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DbUserCacheManager.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DbUserCacheManager.java: Line 156: } else { Line 157: for (DirectoryGroup directoryGroup : fetchedGroups) { Line 158: cache.put(directoryGroup.getId(), directoryGroup); Line 159: } Line 160: } > I still think that for query users you can iterate namespace then users. not sure i understood. i do not have namespaces at hand, before going over the collection of db-users. after that - i have the namespaces. and i can already also map which db users belong to which hamespace. then i would like to fetch the users (in order to get the groups) per namespace. do you think this is wrong? and what about the cache? i'm using the cache to understand if i already loaded a group or not. Line 161: Line 162: // Repeat the same for groups from db. Line 163: if (dbGroups.get(namespace) != null) { Line 164: List<DirectoryGroup> directoryGroups = new ArrayList<>(dbGroups.get(namespace).size()); Line 223: } Line 224: } Line 225: if (toFetch == 0) { Line 226: break; Line 227: } > please have break statement at beginning of loop or at end of loop, but bes idsToFtchByNamespace.size() returns you the number of entries of the "outer map" (I hope you understood me here) - i.e - the number of namespaces, not the number of groups to fetch. Line 228: for (String namespace : idsToFetchByNamespace.keySet()) { Line 229: for (DirectoryGroup group : AuthzUtils.findGroupsByIds(authz, namespace, new ArrayList<>(idsToFetchByNamespace.get(namespace)), false)) { Line 230: groupsToFetch.addAll(group.getGroups()); Line 231: } Line 233: } Line 234: } Line 235: } Line 236: Line 237: private Collection<DirectoryGroup> flatGroupsIfRecursive(boolean toFlat, List<DirectoryGroup> groups) { > not sure why can't you always do this, as worse case you do not have groups I can of course. do u think i should do it? Line 238: Collection<DirectoryGroup> results = groups; Line 239: if (toFlat) { Line 240: Set<DirectoryGroup> accumulator = new HashSet<>(); Line 241: DirectoryUtils.flatGroups(accumulator, groups); -- To view, visit http://gerrit.ovirt.org/28561 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id49b51517a967c7a83e8e73f52181673baa31700 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
