Juan Hernandez has posted comments on this change. Change subject: restapi: Display VM session infromation (WIP) ......................................................................
Patch Set 1: (8 comments) http://gerrit.ovirt.org/#/c/25591/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmSessionsResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmSessionsResource.java: Line 69: for (User user : usersResource.list().getUsers()) { Line 70: if (user.getName().equals(session.getUser().getName())) { Line 71: session.setUser(user); Line 72: return; Line 73: } Don't do this. Adding a new query to lookup users by name shouldn't be a problem. Getting all the users isn't an option, as there may be thousands. Line 74: } Line 75: // Console user is an ovirt user and must be found. Line 76: throw new WebApplicationException(Response.Status.NOT_FOUND); Line 77: } http://gerrit.ovirt.org/#/c/25591/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java: Line 44: protected HttpHeaders httpHeaders; Line 45: protected ValidatorLocator validatorLocator; Line 46: protected MappingLocator mappingLocator; Line 47: Line 48: // protected <S extends AbstractBackendResource<B extends BaseResource, A>> S inject(S resource) { Please remove this comment. Line 49: protected <S extends BaseBackendResource> S inject(S resource) { Line 50: resource.setBackend(backend); Line 51: resource.setMappingLocator(mappingLocator); Line 52: resource.setValidatorLocator(validatorLocator); http://gerrit.ovirt.org/#/c/25591/1/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java: Line 1362: * method. Line 1363: * Line 1364: * @param vm Line 1365: * @param sessions Line 1366: * @return These javadoc @param and @return without a description aren't useful. Remove them or add a description. Line 1367: */ Line 1368: public static Sessions map(org.ovirt.engine.core.common.businessentities.VM vm, Sessions sessions) { Line 1369: if (sessions == null) { Line 1370: sessions = new Sessions(); Line 1369: if (sessions == null) { Line 1370: sessions = new Sessions(); Line 1371: } Line 1372: mapConsoleSession(vm, sessions); Line 1373: mapGusetSessions(vm, sessions); Replace "Guset" with "Guest". Line 1374: return sessions; Line 1375: } Line 1376: Line 1377: /** Line 1380: * engine makes available only the name and IP of this user. In the future it may make available also the connection Line 1381: * protocol used in the session (spice/vnc). Line 1382: * Line 1383: * @param vm Line 1384: * @param sessions Same regarding @param. Line 1385: */ Line 1386: private static Sessions mapConsoleSession(org.ovirt.engine.core.common.businessentities.VM vm, Sessions sessions) { Line 1387: String consoleUserName = vm.getConsoleCurentUserName(); Line 1388: if (consoleUserName != null && !consoleUserName.isEmpty()) { Line 1409: * multiple 'guest' users, along with their IPs and perhaps also the connection protocols that they are using (SSH, Line 1410: * RDP...) Line 1411: * Line 1412: * @param vm Line 1413: * @param sessions Same. Line 1414: */ Line 1415: private static Sessions mapGusetSessions(org.ovirt.engine.core.common.businessentities.VM vm, Sessions sessions) { Line 1416: String guestUserName = vm.getGuestCurentUserName(); Line 1417: if (guestUserName != null && !guestUserName.isEmpty()) { http://gerrit.ovirt.org/#/c/25591/1/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/utils/GuidUtils.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/utils/GuidUtils.java: Line 26: * one or more strings, guid will be generated from them Line 27: * @return unique Guid generated from the given strings. Line 28: */ Line 29: public static Guid generateGuidUsingMd5(String... args) { Line 30: String id = ""; Use StringBuilder instead of String here, if possible, it is faster, specially if you know the size of the resulting string: int size = 0; for (String arg : args) { size += arg.length(); } StringBuilder buffer = new StringBuilder(size); for (String arg : args) { buffer.append(arg); } Line 31: for (String arg : args) { Line 32: id = id + arg; Line 33: } Line 34: byte[] hash; Line 32: id = id + arg; Line 33: } Line 34: byte[] hash; Line 35: try { Line 36: hash = MessageDigest.getInstance(MD5_SECURITY_ALGORITHM).digest(id.getBytes()); When using the String.getBytes method you should always explictly indicate what is the character encoding, otherwise it depends on the default character encoding set in the OS, which the admin may change: id.getBytes("UTF-8") Line 37: } catch (NoSuchAlgorithmException e) { Line 38: throw new IllegalStateException(e); // never happens, MD5 algorithm exists Line 39: } Line 40: return new Guid(hash, true); -- To view, visit http://gerrit.ovirt.org/25591 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icb6488ea7118826adb169c9d1388fab7ab2792d8 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ori Liel <[email protected]> Gerrit-Reviewer: Juan Hernandez <[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
