Alon Bar-Lev 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. 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. 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 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 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... once you do x and once y... :) 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? 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. 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? 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 what you are looking for. 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? 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 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 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? 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? 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 <[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
