Martin Peřina has posted comments on this change.
Change subject: core: Use new authentication interfaces
......................................................................
Patch Set 1:
(5 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AdGroupsHandlingCommandBase.java
Line 46: protected DirectoryGroup getAdGroup() {
Line 47: if (mGroup == null && !getGroupId().equals(Guid.Empty)) {
Line 48: DbGroup dbGroup =
DbFacade.getInstance().getDbGroupDao().get(getGroupId());
Line 49: Directory directory =
DirectoryManager.getInstance().getDirectory(dbGroup.getDomain());
Line 50: // XXX: Should check for null directory.
Please check!
Line 51: mGroup = directory.findGroup(dbGroup.getExternalId());
Line 52: }
Line 53: return mGroup;
Line 54: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DbUserCacheManager.java
Line 212: if (!StringUtils.equals(dbUser.getEmail(),
directoryUser.getEmail())) {
Line 213: dbUser.setEmail(directoryUser.getEmail());
Line 214: update = true;
Line 215: }
Line 216:
Wouldn't it be better to create some compare(DbUser, DirectoryUser) method and
if they don't match just overwrite all relevant attributes instead of doing it
one by one?
Line 217: // Compare the new list of group names and identifiers:
Line 218: StringBuilder groupNamesBuffer = new StringBuilder();
Line 219: StringBuilder groupIdsBuffer = new StringBuilder();
Line 220: boolean first = true;
Line 224: groupIdsBuffer.append(',');
Line 225: }
Line 226: groupNamesBuffer.append(directoryGruop.getName());
Line 227: groupIdsBuffer.append(directoryGruop.getId());
Line 228: }
I would extract this groups parsing code to some utils method ...
Line 229: String groupNames = groupNamesBuffer.toString();
Line 230: String groupIds = groupIdsBuffer.toString();
Line 231: if (!StringUtils.equals(dbUser.getGroupNames(), groupNames)) {
Line 232: dbUser.setGroupNames(groupNames);
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitBackendServicesOnStartupBean.java
Line 75:
AuthenticationProfileManager.getInstance().registerProfile(profile);
Line 76: }
Line 77: }
Line 78:
Line 79: AsyncTaskManager.getInstance().InitAsyncTaskManager();
Wouldn't it be better to move this initialization code to init method inside
AuthenticationProfileManager?
Also I would add some logging about initialization of this service ...
Line 80: ResourceManager.getInstance().init();
Line 81: OvfDataUpdater.getInstance().initOvfDataUpdater();
Line 82:
Line 83: SchedulingManager.getInstance().setMigrationHandler(new
MigrationHandler() {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbUser.java
Line 105: idsBuffer.append(group.getId());
Line 106: }
Line 107: groupNames = namesBuffer.toString();
Line 108: groupIds = idsBuffer.toString();
Line 109:
I would extract this groups parsing code to some utils method ...
Line 110: role = "";
Line 111: note = "";
Line 112: }
Line 113:
--
To view, visit http://gerrit.ovirt.org/20412
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d5d6ee41b5373328eaf2f8448efe8e598609651
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
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
