Alon Bar-Lev has posted comments on this change.

Change subject: core: Introduce new authentication interfaces
......................................................................


Patch Set 7:

(4 comments)

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/authentication/Directory.java
these comments are done to this component but should be applied to all.

please move the file to a place we can wrap it as own jar for publish public 
interfaces, example:

 backend/manager/modules/public-interfaces
Line 1: package org.ovirt.engine.core.authentication;
Line 2: 
Line 3: import java.util.List;
Line 4: 


Line 1: package org.ovirt.engine.core.authentication;
please move interfaces used by external plugins into external supported 
interfaces for plugins, example:

 org.ovirt.engine.interfaces.authentication

So we can publish org.ovirt-engine.interfaces externally, and track its changes.
Line 2: 
Line 3: import java.util.List;
Line 4: 
Line 5: import org.ovirt.engine.core.common.utils.ExternalId;


Line 1: package org.ovirt.engine.core.authentication;
Line 2: 
Line 3: import java.util.List;
Line 4: 
Line 5: import org.ovirt.engine.core.common.utils.ExternalId;
please do not leak engine specific class into external interfaces.
Line 6: 
Line 7: /**
Line 8:  * A directory is an object that manages a collection of users and 
groups, usually stored in an external system like an
Line 9:  * LDAP database.


Line 14:      *
Line 15:      * @param name the name of the user
Line 16:      * @return the user corresponding to the given name or {@code 
null} if no such user can be found
Line 17:      */
Line 18:     DirectoryUser findUser(String name);
please do not use classes with implementation when interacting with external 
plugins, as it will be difficult to forward support old plugins.

A directory user should be a map with attributes, this way new attributes can 
be added by new providers without changing the interface.

The abstraction can be done at engine side (converting the map into java 
classes) to ease use.
Line 19: 
Line 20:     /**
Line 21:      * Retrieves an user from the directory given its identifier.
Line 22:      *


-- 
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: 7
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: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: mooli tayer <mta...@redhat.com>
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

Reply via email to