Yair Zaslavsky has posted comments on this change. Change subject: aaa: Fix sync ......................................................................
Patch Set 13: (6 comments) http://gerrit.ovirt.org/#/c/28561/13/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/sync/SyncAAAEntries.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/sync/SyncAAAEntries.java: Line 30: // 3. Fetch groups from DB Line 31: // 4. Resolve the fetched groups (for each group that was not already resolved) Line 32: // 5. Resolve recursively all groups Line 33: // 6. Compare the user from db and the fetched user from the directory + store in DB if there is a change Line 34: public void sync( > fetchUsers? well, this is not just fetching user, but information that can be used for sync. Line 35: Map<String, Map<String, Set<String>>> userIdsToFetch, Line 36: Set<String> activeGroupIds, Line 37: Map<String, DirectoryGroup> outDirectoryGroupsByIds, Line 38: Map<String, DirectoryUser> outDirectoryUserByIds, Line 31: // 4. Resolve the fetched groups (for each group that was not already resolved) Line 32: // 5. Resolve recursively all groups Line 33: // 6. Compare the user from db and the fetched user from the directory + store in DB if there is a change Line 34: public void sync( Line 35: Map<String, Map<String, Set<String>>> userIdsToFetch, > userIdsToFetchPerAuthz? to make it clear what is the 2nd dimension. hm, i thought about it, but u have 3 dimensions here - authz, namespace, users. Line 36: Set<String> activeGroupIds, Line 37: Map<String, DirectoryGroup> outDirectoryGroupsByIds, Line 38: Map<String, DirectoryUser> outDirectoryUserByIds, Line 39: List<String> outNonActiveUserIds, Line 36: Set<String> activeGroupIds, Line 37: Map<String, DirectoryGroup> outDirectoryGroupsByIds, Line 38: Map<String, DirectoryUser> outDirectoryUserByIds, Line 39: List<String> outNonActiveUserIds, Line 40: List<String> outNonActiveGroupIds) { > these should be concluded by the caller based on all above data. If we return info on non active groups, we can optimize groups fetching on next rounds (well, if some of the groups became inactive). about your other comment - i agree, now that only DirectoryUtils.sync calls this method. Line 41: Map<String, DirectoryGroup> fetchedGroupsCache = new HashMap<>(); Line 42: Set<String> authzEntries = userIdsToFetch.keySet(); Line 43: for (String authz : authzEntries) { Line 44: ExtensionProxy authzExtension = null; Line 38: Map<String, DirectoryUser> outDirectoryUserByIds, Line 39: List<String> outNonActiveUserIds, Line 40: List<String> outNonActiveGroupIds) { Line 41: Map<String, DirectoryGroup> fetchedGroupsCache = new HashMap<>(); Line 42: Set<String> authzEntries = userIdsToFetch.keySet(); > temp variable not needed done. Line 43: for (String authz : authzEntries) { Line 44: ExtensionProxy authzExtension = null; Line 45: try { Line 46: authzExtension = EngineExtensionsManager.getInstance().getExtensionByName(authz); Line 103: } Line 104: groupIdsToFetch = getIdsFromGroups(groupsToFetch, fetchedGroupsCache); Line 105: } Line 106: } Line 107: outDirectoryGroupsByIds.putAll(fetchedGroupsCache); > I am unsure why you need both. what do you mean ? the outXXX means this is an "out" parameter, wheres fetchedGroupsCache is an internal param. Line 108: } Line 109: Line 110: Line 111: private void addGroupsToFetch(Set<String> activeGroupIds, http://gerrit.ovirt.org/#/c/28561/13/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/aaa/DirectoryUtils.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/aaa/DirectoryUtils.java: Line 59: dbGroupsPerExternalId.put(dbGroup.getExternalId(), dbGroup); Line 60: if (dbGroup.isActive()) { Line 61: activeGroupExternalIds.add(dbGroup.getExternalId()); Line 62: } Line 63: } > ok, so now I am confused... why do we need to populate groups? can we read we're constructing activeGroupExternalIds structure to know which groups are active - only "active groups" should be relevant for setting the group ids. Line 64: Line 65: Map<String, DirectoryGroup> outDirectoryGroupsByIds = new HashMap<>(); Line 66: Map<String, DirectoryUser> outDirectoryUserByIds = new HashMap<>(); Line 67: List<String> outNonActiveUserExternalIds = new ArrayList<>(); -- 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: 13 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
