Alon Bar-Lev has posted comments on this change.

Change subject: aaa: Fix apperance of empty profile
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.ovirt.org/#/c/25993/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitBackendServicesOnStartupBean.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitBackendServicesOnStartupBean.java:

Line 132:     }
Line 133: 
Line 134:     private void createKerberosLdapAAAConfigurations() {
Line 135: 
Line 136:         for (String domain : Config.<String> 
getValue(ConfigValues.DomainName).split("[,]", 0)) {
> 0 will just discard empty trailing strings, why do we need them?
this is limit... per documentation[1]

Maybe you mean the following regexp: \b*,\b* ?

But then if you assume there can be spaces you need also to trim the result, as 
there can be leading/trailing spaces... and if you trim anyway, why not just 
split based on "," which will achieve the same result.

[1] 
http://docs.oracle.com/javase/7/docs/api/java/lang/String.html#split(java.lang.String,%20int)
Line 137:             if (!domain.isEmpty()) {
Line 138:                 Properties authConfig = new Properties();
Line 139:                 authConfig.put(ExtensionsManager.CLASS,
Line 140:                         
"org.ovirt.engine.extensions.aaa.builtin.kerberosldap.KerberosLdapAuthenticator");


Line 133: 
Line 134:     private void createKerberosLdapAAAConfigurations() {
Line 135: 
Line 136:         for (String domain : Config.<String> 
getValue(ConfigValues.DomainName).split("[,]", 0)) {
Line 137:             if (!domain.isEmpty()) {
> ok, then we should assume the value set in the config cannot be "," and oth
how can it? you control it, nobody manage this manually.

but if you assume it can, leave it as-is... but then you need to trim domain 
anyway.
Line 138:                 Properties authConfig = new Properties();
Line 139:                 authConfig.put(ExtensionsManager.CLASS,
Line 140:                         
"org.ovirt.engine.extensions.aaa.builtin.kerberosldap.KerberosLdapAuthenticator");
Line 141:                 authConfig.put(ExtensionsManager.PROVIDES, 
"org.ovirt.engine.authentication");


-- 
To view, visit http://gerrit.ovirt.org/25993
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icbf9384f40ad061ec35c03c4fbcbd1dd70085526
Gerrit-PatchSet: 1
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

Reply via email to