[Engine-patches] Change in ovirt-engine[master]: aaa: Fix sync

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: aaa: Fix sync
......................................................................


Patch Set 9:

(3 comments)

http://gerrit.ovirt.org/#/c/28561/9/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 151:             }
Line 152: 
Line 153:             Map<String, DirectoryGroup> cache = new HashMap<>();
Line 154:             if (!recursiveSupportedByProvider) {
Line 155:                 fetchAllGroups(authz, namespace, fetchedGroups, 
cache, fetchedGroups);
can't you just put in cache whatever you have now?

and add these groups into the idsToFetch? so in next loop you have all groups 
as equal?
Line 156:             } else {
Line 157:                 for (DirectoryGroup directoryGroup : fetchedGroups) {
Line 158:                     cache.put(directoryGroup.getId(), directoryGroup);
Line 159:                 }


Line 159:                 }
Line 160:             }
Line 161: 
Line 162:             // Repeat the same for groups from db.
Line 163:             if (dbGroups.get(namespace) != null) {
this is somewhat strange... as the namespace loop should be inner loop and not 
outer loop, as while you resolve users/groups you can get groups of other name 
space.
Line 164:                 List<DirectoryGroup> directoryGroups = new 
ArrayList<>(dbGroups.get(namespace).size());
Line 165:                 for (DbGroup dbGroup : dbGroups.get(namespace)) {
Line 166:                     if (groupFromAuthzProvider(dbGroup, 
AuthzUtils.getName(authz))) {
Line 167:                         directoryGroups.add(


Line 218:             String namespace,
Line 219:             Set<DirectoryGroup> fetchedGroups,
Line 220:             Map<String, DirectoryGroup> cache,
Line 221:             Collection<DirectoryGroup> current) {
Line 222:         while (true) {
I think that the input of this function should be cache and toFetch... not sure 
what current is... so loop is:

 while(toFetch.size() > 0) {
     update cache
     update toFetch
 }
Line 223:             Set<String> toFetch = new HashSet<>();
Line 224:             for (DirectoryGroup group : current) {
Line 225:                 if (!cache.containsKey(group.getId())) {
Line 226:                     for (DirectoryGroup memberOf : group.getGroups()) 
{


-- 
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: 9
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