Alon Bar-Lev has posted comments on this change. Change subject: 7. [WIP] core: Introducing AuthenticationProfileRepository ......................................................................
Patch Set 11: (3 comments) http://gerrit.ovirt.org/#/c/24366/11/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationProfileRepository.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationProfileRepository.java: Line 29: } Line 30: return instance; Line 31: } Line 32: Line 33: public void createProfiles() throws ConfigurationException { you did not answered my question... what good is this as public if calling it twice will cause some unexpected results? I think it should be called by private constructor. Line 34: Line 35: // Get the extensions that correspond to authn (authentication) service. Line 36: // For each extension - get the relevant authn extension. Line 37: Line 40: new AuthenticationProfile(AuthenticatorManager.getInstance() Line 41: .parseAuthenticator(authnExtension.getConfig()), DirectoryManager.getInstance() Line 42: .parseDirectory(ConfigurationLoader.getInstance() Line 43: .getExtensionByName(authnExtension.getConfig().getString(AUTHN_AUTHZ_PLUGIN)) Line 44: .getConfig()))); great! now I suggest to indent... every parameter at own indent, while their internal at own. in this case: registerProfile( new AuthenticationProfile( AuthenticatorManager.getInstance().parseAuthenticator(authnExtension.getConfig()), DirectoryManager.getInstance().parseDirectory( ConfigurationLoader.getInstance().getExtensionByName( authnExtension.getConfig().getString(AUTHN_AUTHZ_PLUGIN) ).getConfig() ) ) ) correct indent makes it much easier to understand without any need for temp variables with custom naming. Line 45: } Line 46: } Line 47: Line 48: /** Line 47: Line 48: /** Line 49: * Returns an unmodifiable list containing all the authentication profiles that have been previously loaded. Line 50: */ Line 51: public List<AuthenticationProfile> getProfiles() { stupid question... why can't you return collection? Line 52: return new ArrayList<>(profiles.values()); Line 53: } Line 54: Line 55: /** -- To view, visit http://gerrit.ovirt.org/24366 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If375fccea98544c64d9ec41cc9dbcb855bf02fb7 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: Martin Peřina <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[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
