Alon Bar-Lev has posted comments on this change. Change subject: aaa: Fix sync ......................................................................
Patch Set 12: (4 comments) yes, I understand that we do not need to sync groups if we have flat groups at user level. for (2) I mean to use single entities during the sync, without the need to translate into database entities during the algorithm, I think it will make the implementation much simpler to follow. http://gerrit.ovirt.org/#/c/28561/12/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SyncAAAEntries.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SyncAAAEntries.java: Line 29: public class SyncAAAEntries { Line 30: Line 31: private static final Log log = LogFactory.getLog(SyncAAAEntries.class); Line 32: Line 33: public void refreshAllUsers(List<DbUser> dbUsers) { > you mean at usersPerAuthz? no... just call this refreshUsers :) Line 34: final Map<String, List<DbUser>> usersPerAuthz = new HashMap<>(); Line 35: final Map<String, List<DbGroup>> groupsPerAuthz = new HashMap<>(); Line 36: Line 37: for (DbUser dbUser : dbUsers) { Line 37: for (DbUser dbUser : dbUsers) { Line 38: MultiValueMapUtils.addToMap(dbUser.getDomain(), dbUser, usersPerAuthz); Line 39: } Line 40: Line 41: for (DbGroup dbGroup : DbFacade.getInstance().getDbGroupDao().getAll()) { > why not? i must modify the group membership correctly, so i must sync accor but you need to sync only user assignment in this case, no? Line 42: MultiValueMapUtils.addToMap(dbGroup.getDomain(), dbGroup, groupsPerAuthz); Line 43: } Line 44: Line 45: Set<String> authzNames = new HashSet<String>() { Line 59: authzName)); Line 60: } Line 61: refreshedUsers.addAll(refreshAllUsers(extension, Line 62: usersPerAuthz.get(authzName), Line 63: groupsPerAuthz.get(authzName))); > no, i think what should be done is include "continue" at the exception. please avoid continue... this is spagetti programming... the try/catch is meant exactly for that. Line 64: } Line 65: Line 66: for (DbUser dbUser : refreshedUsers) { Line 67: DbFacade.getInstance().getDbUserDao().update(dbUser); Line 62: usersPerAuthz.get(authzName), Line 63: groupsPerAuthz.get(authzName))); Line 64: } Line 65: Line 66: for (DbUser dbUser : refreshedUsers) { > why? the users were refreshed (or synched). now it's time to update in db. just now... but you load the content into this variable... this is why it is confusing. not critical. Line 67: DbFacade.getInstance().getDbUserDao().update(dbUser); Line 68: } Line 69: } Line 70: -- 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: 12 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
