Alon Bar-Lev has posted comments on this change. Change subject: aaa: using the new extensions API in InternalDirectory ......................................................................
Patch Set 6: (4 comments) great! just remove the fillOpaque it is not needed. 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? 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 the first Base more than the second one? is it the ide? 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 resolved to nothing in case of group and this one in case of principal, so no conditionals during execute. for this simple use case it is more difficult to do in java for no real gain. 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. 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 <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
