Yair Zaslavsky has posted comments on this change.

Change subject: aaa: using the new extensions API in InternalDirectory
......................................................................


Patch Set 6:

(4 comments)

http://gerrit.ovirt.org/#/c/26477/6/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalDirectory.java
File 
backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalDirectory.java:

Line 54:                 doFetchPrincipalRecord(input, output);
Line 55:             } else if 
(command.equals(Authz.InvokeCommands.QUERY_CLOSE)) {
Line 56:                 // Do nothing
Line 57:             } else if (QUERY_COMMANDS.contains(command)) {
Line 58:                 fillOpaque(output, command);
> so why do you need this one time function?
Done
Line 59:             } else if 
(command.equals(Authz.InvokeCommands.QUERY_EXECUTE)) {
Line 60:                 doQueryExecute(input, output);
Line 61:             } else {
Line 62:                 output.put(Base.InvokeKeys.COMMAND, 
Base.InvokeResult.UNSUPPORTED);


Line 68:             output.mput(
Line 69:                     Base.InvokeKeys.RESULT, Base.InvokeResult.FAILED
Line 70:            ).mput(
Line 71:                    Base.InvokeKeys.MESSAGE, ex.getMessage()
Line 72:            );
> I do not understand why event that is not indented correctly... what pushes
as i wrote previously, unfortunately i have to ident this manually, i guess i 
missed here.
Line 73:         }
Line 74:     }
Line 75: 
Line 76: 


Line 75: 
Line 76: 
Line 77:     private void doQueryExecute(ExtMap input, ExtMap output) {
Line 78:         Opaque opaque = input.<Opaque> 
get(Authz.InvokeKeys.QUERY_OPAQUE);
Line 79:         if 
(opaque.queryType.equals(InvokeCommands.QUERY_PRINCIPALS_BY_IDS_OPEN) || 
opaque.queryType.equals(InvokeCommands.QUERY_PRINCIPALS_OPEN)) {
> had we in python or C I would have put function pointer in opaque that was 
hmm, well for principals you still need to call the execute twice (two 
iterations)
iteration 1 - get the single user
iteration 2 - return null.

so when i suggested this, i didnt have groups in my mind.
Line 80:             // Since there is a single user in the directory, execute
Line 81:             output.put(Authz.InvokeKeys.QUERY_RESULT, opaque.firstCall 
? Arrays.asList(adminUser) : null);
Line 82:             opaque.firstCall = false;
Line 83: 


Line 85:     }
Line 86: 
Line 87:     private void fillOpaque(ExtMap output, ExtUUID queryType) {
Line 88:         output.put(Authz.InvokeKeys.QUERY_OPAQUE, new 
Opaque(queryType));
Line 89:     }
> can be removed.
Done
Line 90: 
Line 91: 
Line 92: 
Line 93:     private void doFetchPrincipalRecord(ExtMap input, ExtMap output) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I958443292da0455e0a12039fac98eebb9b17dee2
Gerrit-PatchSet: 6
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