Oved Ourfali has posted comments on this change.

Change subject: [WIP] Introduce new directory interface
......................................................................


Patch Set 1: (7 inline comments)

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/users/DirectoryEntryStatus.java
Line 10:  * check performed by the engine.
Line 11:  */
Line 12: public enum DirectoryEntryStatus implements Identifiable {
Line 13:     INACTIVE(0),
Line 14:     ACTIVE(1);
minor thought - I'd use AVAILABLE and UNAVAILABLE (although I know we currently 
do use active/inactive).
Line 15: 
Line 16:     private int value;
Line 17: 
Line 18:     private static Map<Integer, DirectoryEntryStatus> mappings = new 
HashMap<Integer, DirectoryEntryStatus>();


....................................................
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);
I'd use findUser, with two different overloads.
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);
I'd use findUsers
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 63:      * @param id the identifier of the group
Line 64:      * @return the group corresponding to the given identifier or
Line 65:      *   <code>null</code> if no such group can be found
Line 66:      */
Line 67:     DirectoryGroup findGroupById(Guid id);
I'd use findGroup
Line 68: 
Line 69:     /**
Line 70:      * Search the directory looking for users that match the given 
search query.
Line 71:      *


Line 71:      *
Line 72:      * @param query the query
Line 73:      * @return a list containing the users that match the given query
Line 74:      */
Line 75:     Set<DirectoryUser> findUsersByQuery(String query);
I'd use findUsers
Line 76: 
Line 77:     /**
Line 78:      * Search the directory looking for groups that match the given 
search
Line 79:      * query.


Line 72:      * @param query the query
Line 73:      * @return a list containing the users that match the given query
Line 74:      */
Line 75:     Set<DirectoryUser> findUsersByQuery(String query);
Line 76: 
Just wondering, do you think we also need some query interface?
Line 77:     /**
Line 78:      * Search the directory looking for groups that match the given 
search
Line 79:      * query.
Line 80:      *


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);
I'd use findGroups


-- 
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: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to