Alon Bar-Lev has posted comments on this change. Change subject: [WIP] Introduce new directory interface ......................................................................
Patch Set 1: (5 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/directory/DirectoryManager.java Line 55: /** Line 56: * Loads the directory implementations that are available in the class path Line 57: * using the service loader mechanism. Line 58: */ Line 59: private void loadImplementations() { but I want to have all jars within the classpath per default. only based on configuration to load a provider with a configuration. moving files around will not good for packaging... as I cannot install all required files and configure later. or maybe I do not understand this loop. Line 60: ServiceLoader<DirectorySpi> loader = Line 61: ServiceLoader.load(DirectorySpi.class); Line 62: for (DirectorySpi implementation : loader) { Line 63: implementations.add(implementation); .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/users/Directory.java Line 26: * @param name the name of the user Line 27: * @return the user corresponding to the given name or Line 28: * <code>null</code> if no such user can be found Line 29: */ Line 30: DirectoryUser findUserByName(String name); Hi, all searches should be based on queries (within the generic provider anyway)... and I do think that in order to add user or remove user we might allow different searches, non of them generic but a set of attributes we do support. Line 31: Line 32: /** Line 33: * Retrieves an user from the directory given its identifier. Note that this Line 34: * identifier is the one assigned by the engine to the user, not the one Line 50: * with no particular order, note that the returned set may contain less Line 51: * elements than the given set of identifiers and that it may be in a Line 52: * different order Line 53: */ Line 54: Set<DirectoryUser> findUsersById(Set<Guid> ids); but these strings will enable us to provide any implementation including local repository of uids. please do not design this for ldap nor to specific directory implementations. Line 55: Line 56: /** Line 57: * Retrieves a group from the directory given its identifier. Note that this Line 58: * identifier is the one assigned by the engine to the group, not the one Line 80: * Line 81: * @param query the query Line 82: * @return a list containing the groups that match the given query Line 83: */ Line 84: Set<DirectoryGroup> findGroupsByQuery(String query); so we invent a new query language? is it good for us? maybe better to adopt some standard directory query language (such as ldap)? .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/users/DirectoryUser.java Line 15: private String firstName; Line 16: private String lastName; Line 17: private String title; Line 18: private String email; Line 19: private String department; it will enable much simpler implementation... why additional 160 bytes in heap should make implementation more complex? Line 20: Line 21: // Flag indicating if this user has the administrator role: Line 22: private boolean isAdmin = false; Line 23: -- To view, visit http://gerrit.ovirt.org/15596 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If84a0c9d6553d81cdbbe224972696f169cca90d4 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches