Yair Zaslavsky has posted comments on this change. Change subject: aaa: using the new extensions API in InternalDirectory ......................................................................
Patch Set 7: (14 comments) http://gerrit.ovirt.org/#/c/26477/7/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java: Line 30: Line 31: @Deprecated Line 32: @Override Line 33: public DirectoryUser findUser(String name) { Line 34: ExtMap inputMap = new ExtMap().mput( > no need for inputMap temp variable. Done Line 35: Base.InvokeKeys.COMMAND, Line 36: Authz.InvokeCommands.FETCH_PRINCIPAL_RECORD Line 37: ).mput( Line 38: Authn.InvokeKeys.AUTH_RECORD, Line 44: ExtMap outputMap = extension.invoke(inputMap); Line 45: ExtMap principalRecord = outputMap.<ExtMap>get(Authz.InvokeKeys.PRINCIPAL_RECORD); Line 46: if (principalRecord == null) { Line 47: return null; Line 48: } > this condition is not required. Done Line 49: Line 50: return mapPrincipalRecord(principalRecord); Line 51: } Line 52: Line 62: Line 63: @Deprecated Line 64: @Override Line 65: public List<DirectoryUser> findUsers(List<String> ids) { Line 66: ExtMap inputMap = new ExtMap().mput( > no need for inputMap temp variable Done Line 67: Base.InvokeKeys.COMMAND, Line 68: Authz.InvokeCommands.QUERY_PRINCIPALS_BY_IDS_OPEN Line 69: ).mput( Line 70: Authz.InvokeKeys.PRINCIPAL_IDS, Line 87: Line 88: @Deprecated Line 89: @Override Line 90: public List<DirectoryUser> queryUsers(String query) { Line 91: ExtMap inputMap = new ExtMap().mput( > same Done Line 92: Base.InvokeKeys.COMMAND, Line 93: Authz.InvokeCommands.QUERY_PRINCIPALS_OPEN Line 94: ).mput( Line 95: Authz.InvokeKeys.QUERY, Line 94: ).mput( Line 95: Authz.InvokeKeys.QUERY, Line 96: query Line 97: ); Line 98: ExtMap outputMap = extension.invoke(inputMap); > no need for outputMap temp variable, please notice findUsers and this... on Done Line 99: return populateUsers(outputMap); Line 100: } Line 101: Line 102: private List<DirectoryUser> populateUsers(ExtMap inputMap) { Line 106: public void handle(ExtMap queryResult) { Line 107: directoryUsers.add(mapPrincipalRecord(queryResult)); Line 108: } Line 109: }; Line 110: return queryImpl(inputMap, directoryUsers, handler); > why do you need handler temp variable? Done Line 111: } Line 112: Line 113: Line 114: private List<DirectoryGroup> populateGroups(ExtMap inputMap) { Line 118: public void handle(ExtMap queryResult) { Line 119: directoryGroups.add(mapGroupRecord(queryResult)); Line 120: } Line 121: }; Line 122: return queryImpl(inputMap, directoryGroups, handler); > same... you do not need hander temp variable. Done Line 123: } Line 124: Line 125: private <T> List<T> queryImpl(ExtMap inputMap, Line 126: final List<T> results, Line 122: return queryImpl(inputMap, directoryGroups, handler); Line 123: } Line 124: Line 125: private <T> List<T> queryImpl(ExtMap inputMap, Line 126: final List<T> results, > why do you need results? Done Line 127: QueryResultHandler handler) { Line 128: try { Line 129: ExtMap outputMap = extension.invoke(inputMap); Line 130: queryOpaque = outputMap.get(Authz.InvokeKeys.QUERY_OPAQUE); Line 139: ExtMap result = outputMap.get(Authz.InvokeKeys.QUERY_RESULT); Line 140: if (result == null) { Line 141: break; Line 142: } Line 143: handler.handle(result); > consider return boolean and break loop if false, as for example you found w well, here i'm not searching for some element, but have to iterate on all the records. Not sure I see such a great benefit from your offer, I will upload the next version, please comment if you think it is still needed. Line 144: } Line 145: return results; Line 146: } finally { Line 147: extension.invoke(new ExtMap().mput( Line 141: break; Line 142: } Line 143: handler.handle(result); Line 144: } Line 145: return results; > why do you return results? what is it used for? Done Line 146: } finally { Line 147: extension.invoke(new ExtMap().mput( Line 148: Base.InvokeKeys.COMMAND, Line 149: Authz.InvokeCommands.QUERY_CLOSE Line 157: Line 158: @Deprecated Line 159: @Override Line 160: public List<DirectoryGroup> queryGroups(String query) { Line 161: ExtMap inputMap = new ExtMap().mput( > no need for temp var Done Line 162: Base.InvokeKeys.COMMAND, Authz.InvokeCommands.QUERY_GROUPS_OPEN Line 163: ).mput(Authz.InvokeKeys.QUERY, query); Line 164: ExtMap outputMap = extension.invoke(inputMap); Line 165: return populateGroups(outputMap); Line 160: public List<DirectoryGroup> queryGroups(String query) { Line 161: ExtMap inputMap = new ExtMap().mput( Line 162: Base.InvokeKeys.COMMAND, Authz.InvokeCommands.QUERY_GROUPS_OPEN Line 163: ).mput(Authz.InvokeKeys.QUERY, query); Line 164: ExtMap outputMap = extension.invoke(inputMap); > no need for temp var Done Line 165: return populateGroups(outputMap); Line 166: } Line 167: Line 168: private DirectoryUser mapPrincipalRecord(ExtMap principalRecord) { Line 162: Base.InvokeKeys.COMMAND, Authz.InvokeCommands.QUERY_GROUPS_OPEN Line 163: ).mput(Authz.InvokeKeys.QUERY, query); Line 164: ExtMap outputMap = extension.invoke(inputMap); Line 165: return populateGroups(outputMap); Line 166: } > move to public section? Done Line 167: Line 168: private DirectoryUser mapPrincipalRecord(ExtMap principalRecord) { Line 169: DirectoryUser directoryUser = null; Line 170: if (principalRecord != null) { Line 189: return directoryUser; Line 190: } Line 191: Line 192: private DirectoryGroup mapGroupRecord(ExtMap group) { Line 193: DirectoryGroup directoryGroup = new DirectoryGroup( > why don't you ask if group != null similar to above function? Done Line 194: extension.getContext().<String> get(Base.ContextKeys.INSTANCE_NAME), Line 195: group.<String> get(Authz.GroupRecord.NAME), Line 196: group.<String> get(Authz.GroupRecord.ID) Line 197: ); -- 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: 7 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
