Michael Pasternak has posted comments on this change.
Change subject: restapi: Separate User and DomainUser user resources
......................................................................
Patch Set 1:
(10 comments)
Juan,
please run jenkins ci job to make sure nothing is broken,
eedri can help you.
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDomainUserResource.java
Line 40: parent.getDirectory().getName(),
Line 41: guid
Line 42: );
Line 43: User user = performGet(VdcQueryType.GetDirectoryUserById,
queryParameters, BaseResource.class);
Line 44: user = modifyDomain(user);
if we not set it in mapper this line won't be needed (see my comment in
collection)
thanks
Line 45: return user;
Line 46: }
Line 47:
Line 48: private User modifyDomain(User user) {
Line 44: user = modifyDomain(user);
Line 45: return user;
Line 46: }
Line 47:
Line 48: private User modifyDomain(User user) {
same method exist in collection
Line 49: // We need to remove the name of the domain because we want to
display
Line 50: // only the reference:
Line 51: Domain domain = user.getDomain();
Line 52: if (domain != null) {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDomainUsersResource.java
Line 70: Users collection = new Users();
Line 71: for (LdapUser entity : entities) {
Line 72: User user = map(entity);
Line 73: user = addLinks(user, true);
Line 74: user = modifyDomain(user);
please add populate(...)
Line 75: collection.getUsers().add(user);
Line 76: }
Line 77: return collection;
Line 78: }
Line 78: }
Line 79:
Line 80: private User modifyDomain(User user) {
Line 81: // We need to remove the name of the domain because we want to
display
Line 82: // only the reference:
i understand this is a old code, but if we have no constraints, i'd prefer name
not to be set in mapper,
Line 83: Domain domain = user.getDomain();
Line 84: if (domain != null) {
Line 85: domain.setName(null);
Line 86: }
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendUsersResource.java
Line 38: extends AbstractBackendCollectionResource<User, DbUser>
Line 39: implements UsersResource {
Line 40:
Line 41: static final String[] SUB_COLLECTIONS = { "permissions", "roles",
"tags" };
Line 42: protected static final String SEARCH_TEMPLATE = "ADUSER@{0}: ";
this is DB context not AD
Line 43: private static final String USERS_SEARCH_PATTERN = "usrname !=
\"\"";
Line 44: private static final String AND_SEARCH_PATTERN = " and ";
Line 45:
Line 46: private BackendDomainResource parent;
Line 62: String user_defined_pattern =
QueryHelper.getConstraint(getUriInfo(), "", modelType);
Line 63: return user_defined_pattern.equals("Users : ") ?
Line 64: user_defined_pattern + USERS_SEARCH_PATTERN
Line 65: :
Line 66: user_defined_pattern + AND_SEARCH_PATTERN +
USERS_SEARCH_PATTERN;
now when you split, we should not allow seeing AD users in this context.
Line 67: }
Line 68:
Line 69: protected String getDomain(User user) {
Line 70: if (user.isSetDomain() && user.getDomain().isSetName()) {
Line 105:
Line 106: sb.append(StringUtils.isEmpty(constraint) ?
Line 107: "allnames=" + username
Line 108: :
Line 109: constraint);
we should not support AD search in this context after split
Line 110:
Line 111: return sb.toString();
Line 112: }
Line 113:
Line 125:
Line 126: protected Users mapDbUserCollection(List<DbUser> entities) {
Line 127: Users collection = new Users();
Line 128: for (DbUser entity : entities) {
Line 129:
collection.getUsers().add(addLinks(modifyDomain(map(entity)),
please add populate() call before addLinks()
Line 130: BaseResource.class));
Line 131: }
Line 132: return collection;
Line 133: }
Line 141: }
Line 142:
Line 143: protected User modifyDomain(User user) {
Line 144: if(user.getDomain()!=null)
Line 145: user.getDomain().setName(null);
same comment as before, it would be good if we could not set it in mapper
Line 146: return user;
Line 147: }
Line 148:
Line 149: protected User mapUser(DbUser dbUser) {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java
Line 167: SearchType.AdUser,
Line 168: getAdUser(0));
Line 169: setUpCreationExpectations(VdcActionType.AddUser,
Line 170: AddUserParameters.class,
Line 171: new String[] { "AdUser.UserId" },
are you sure? we in context of user not aduser afaiu.
Line 172: new Object[] { GUIDS[0] },
Line 173: true,
Line 174: true,
Line 175: null,
--
To view, visit http://gerrit.ovirt.org/17684
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I428f98772ac57cd7825781fb3cf282b2ac56e995
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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
