Yair Zaslavsky has posted comments on this change. Change subject: aaa: Fix sync ......................................................................
Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/28561/6/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 169: } Line 170: } Line 171: } Line 172: Line 173: Map<String, DirectoryGroup> cache = new HashMap<>(); > I still think you should go via users first, as it will reduce the # of fet ok, can you think of some concrete example that will make me understand? :) Line 174: populateRecrusiveGroups(authz, cache, toFetch); Line 175: Line 176: Line 177: Line 170: } Line 171: } Line 172: Line 173: Map<String, DirectoryGroup> cache = new HashMap<>(); Line 174: populateRecrusiveGroups(authz, cache, toFetch); > there is some loop missing.... as here you call populateRecrusiveGroups whi i thought about it as well, and for some reason i got convinced we don't need. i will explain - wem ust have at least one call with "1st level" and one with "recursive" - right? now, isn't the recursive call fetches me ALL the "groups tree" ? why should i continue to resolve? after I fetch all the groups trees, i flat them... Line 175: Line 176: Line 177: Line 178: for (DirectoryUser directoryUser : directoryUsers) { Line 220: } else { Line 221: for (DirectoryGroup group : toCache) { Line 222: if (!cache.containsKey(group.getId())) { Line 223: Set<DirectoryGroup> flatGroups = new HashSet<>(); Line 224: DirectoryGroup fetchedGroup = AuthzUtils.findGroupById(authz, group.getNamespace(), group.getId(), true); > hmmm.... starting to understand, I think. good idea, thanks! Line 225: cache.put(fetchedGroup.getId(), fetchedGroup); Line 226: DirectoryUtils.flatGroups(flatGroups, fetchedGroup.getGroups()); Line 227: for (DirectoryGroup fetchedMemberOf : flatGroups) { Line 228: cache.put(fetchedMemberOf.getId(), fetchedMemberOf); http://gerrit.ovirt.org/#/c/28561/6/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalAuthz.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalAuthz.java: Line 123: Authz.PrincipalRecord.ID, Line 124: configuration.getProperty("config.authz.user.id") Line 125: ).mput( Line 126: Authz.PrincipalRecord.GROUPS, Line 127: Collections.emptyList() > again, no need. right, i will remove. Line 128: ); Line 129: Line 130: } Line 131: -- 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: 6 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
