Alon Bar-Lev has posted comments on this change. Change subject: aaa: Fix sync ......................................................................
Patch Set 11: (5 comments) http://gerrit.ovirt.org/#/c/28561/11/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 64: } Line 65: } Line 66: Line 67: /** Line 68: * Load all the users from the database and refresh them. The refresh logic is as follows : 1. Fetch users from DB > sure, this was like at in the IDE, i guess the formatter got crazy :) I guess you need to use <ul><li>... Line 69: * 2. Resolve the groups of users (get 1st level groups) 3. Fetch groups from DB 4. Resolve the fetched groups (for Line 70: * each group that was not already resolved) 5. Resolve recursively all groups 6. Compare the user from db and the Line 71: * fetched user from the directory + store in DB if there is a change Line 72: */ Line 98: } Line 99: Line 100: for (DbUser dbUser : refreshedUsers) { Line 101: DbFacade.getInstance().getDbUserDao().update(dbUser); Line 102: } > please explain what you mean? ok, so groups do not have other groups, all is flatten? so where do we add the new groups? or we don't? where do we disable users that are missing in directory? or we don't? groups that are missing? I am totally confused about this sync... Line 103: } Line 104: Line 105: private Collection<DbUser> refreshAllUsers(ExtensionProxy authz, List<DbUser> dbUsers, List<DbGroup> dbGroups) { Line 106: Line 140: while (!idsToFetch.isEmpty()) { Line 141: groupsToFetch = new HashSet<>(); Line 142: for (String namespace : idsToFetch.keySet()) { Line 143: List<DirectoryGroup> groups = Line 144: AuthzUtils.findGroupsByIds(authz, namespace, new ArrayList<>(idsToFetch.get(namespace)), true); > idstofetch.get returns a set, which is ok. i would like the ids to be kept so why the findGroupsByIds is not accepting collection? Line 145: idsToFetch = new HashMap<>(); Line 146: for (DirectoryGroup group : groups) { Line 147: cache.put(group.getId(), group); Line 148: for (DirectoryGroup memberOf: group.getGroups()) { Line 141: groupsToFetch = new HashSet<>(); Line 142: for (String namespace : idsToFetch.keySet()) { Line 143: List<DirectoryGroup> groups = Line 144: AuthzUtils.findGroupsByIds(authz, namespace, new ArrayList<>(idsToFetch.get(namespace)), true); Line 145: idsToFetch = new HashMap<>(); > Well, you probably meant remove all of the ids that belong to the namespace I mean to remove all what you have searched... instead of clearing all... by logic it is better. Line 146: for (DirectoryGroup group : groups) { Line 147: cache.put(group.getId(), group); Line 148: for (DirectoryGroup memberOf: group.getGroups()) { Line 149: groupsToFetch.add(memberOf); Line 152: } Line 153: Line 154: Line 155: } Line 156: idsToFetch = getIdsFromGroups(groupsToFetch, cache); > renaming - groupIdsToFetch is better? what do you think? so groupsToFetchPerNamespace? Line 157: } Line 158: List<DbUser> refreshed = new ArrayList<>(); Line 159: for (DbUser dbUser : dbUsers) { Line 160: DirectoryUser directoryUser = directoryUsersByIds.get(dbUser.getExternalId()); -- 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: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
