Yair Zaslavsky has uploaded a new change for review. Change subject: aaa: Change API to 3.3 behavior ......................................................................
aaa: Change API to 3.3 behavior Changing back the behavior of the API to 3.3 behavior. Changing also the behavior of internal id allocation - this is cancelled, and internal id will always be the guid returned from the directory. Although in 3.5 we will have directories that are non ldap, hence can have ids that are not GUID, we would like to handle the user id representation differently, and this is a first stage. Topic: AAA Change-Id: Iff51fb9df33b4b679eba5b1c3ef2bea6ea7b3e07 Signed-off-by: Yair Zaslavsky <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddGroupCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutomaticLoginFilter.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UserCommandBase.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbGroup.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbUser.java M backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendBaseTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendGroupResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendGroupsResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUserResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java M backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/GroupMapper.java M backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UserMapper.java A packaging/dbscripts/upgrade/03_04_0710_change_group_ids.sql A packaging/dbscripts/upgrade/03_04_0720_change_group_ids.sql 17 files changed, 82 insertions(+), 70 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/53/26053/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddGroupCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddGroupCommand.java index df10277..cf89258 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddGroupCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddGroupCommand.java @@ -56,7 +56,7 @@ DbGroup dbGroup = dao.getByExternalId(directoryGroup.getDirectory().getName(), directoryGroup.getId()); if (dbGroup == null) { dbGroup = new DbGroup(directoryGroup); - dbGroup.setId(Guid.newGuid()); + dbGroup.setId(new Guid(directoryGroup.getId().getBytes(), true)); dao.save(dbGroup); } else { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java index eb40b6a..9342e4b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java @@ -216,24 +216,6 @@ return null; } } - - // In older versions of the engine the internal id and external id were the same, thus we need to try again - // using the internal id as if it were an external id: - if (directory != null && id != null) { - externalId = ExternalId.fromHex(id.toString()); - DirectoryIdParameters parameters = new DirectoryIdParameters(); - parameters.setDirectory(directory); - parameters.setId(externalId); - VdcReturnValueBase result = getBackend().runInternalAction(VdcActionType.AddUser, parameters); - if (result.getCanDoAction()) { - id = (Guid) result.getActionReturnValue(); - if (id != null) { - return getDbUserDAO().get(id); - } - return null; - } - } - // There is no such user in the directory: return null; } @@ -253,24 +235,6 @@ return null; } } - - // In older versions of the engine the internal id and external id were the same, thus we need to try again - // using the internal id as if it were an external id: - if (directory != null && id != null) { - externalId = ExternalId.fromHex(id.toString()); - DirectoryIdParameters parameters = new DirectoryIdParameters(); - parameters.setDirectory(directory); - parameters.setId(externalId); - VdcReturnValueBase result = getBackend().runInternalAction(VdcActionType.AddGroup, parameters); - if (result.getCanDoAction()) { - id = (Guid) result.getActionReturnValue(); - if (id != null) { - return getAdGroupDAO().get(id); - } - return null; - } - } - // There is no such group in the directory: return null; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java index aa3e290..75d0423 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java @@ -14,7 +14,6 @@ import org.ovirt.engine.core.common.businessentities.DbUser; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.utils.ExternalId; -import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dao.DbUserDAO; public class AddUserCommand<T extends DirectoryIdParameters> extends CommandBase<T> { @@ -92,15 +91,12 @@ DbUser dbUser = dao.getByExternalId(directoryUser.getDirectory().getName(), directoryUser.getId()); if (dbUser == null) { dbUser = new DbUser(directoryUser); - dbUser.setId(Guid.newGuid()); String groupIds = DirectoryUtils.getGroupIdsFromUser(directoryUser); dbUser.setGroupIds(groupIds); dao.save(dbUser); } else { - Guid id = dbUser.getId(); dbUser = new DbUser(directoryUser); - dbUser.setId(id); String groupIds = DirectoryUtils.getGroupIdsFromUser(directoryUser); dbUser.setGroupIds(groupIds); dao.update(dbUser); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutomaticLoginFilter.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutomaticLoginFilter.java index 5a34198..8c14b79 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutomaticLoginFilter.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutomaticLoginFilter.java @@ -118,7 +118,6 @@ dbUser = dbUserDao.getByExternalId(directory.getName(), directoryUser.getId()); if (dbUser == null) { dbUser = new DbUser(directoryUser); - dbUser.setId(Guid.newGuid()); dbUserDao.save(dbUser); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UserCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UserCommandBase.java index a772140..4259a2a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UserCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UserCommandBase.java @@ -84,7 +84,6 @@ } else { dbUser = new DbUser(directoryUser); - dbUser.setId(Guid.newGuid()); dao.save(dbUser); } return dbUser; diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbGroup.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbGroup.java index cf5f185..9cd1c91 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbGroup.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbGroup.java @@ -38,7 +38,8 @@ public DbGroup(DirectoryGroup directoryGroup) { externalId = directoryGroup.getId(); - domain = directoryGroup.getDirectory().getName(); + setId(new Guid(directoryGroup.getId().getBytes(), true)); + domain = directoryGroup.getDirectoryName(); name = directoryGroup.getName(); } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbUser.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbUser.java index 964c8b4..fc1c411 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbUser.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DbUser.java @@ -108,6 +108,7 @@ active = true; role = ""; note = ""; + id = new Guid(directoryUser.getId().getBytes(), true); StringBuilder namesBuffer = new StringBuilder(); boolean first = true; diff --git a/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd b/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd index fa71127..af45e7e 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd +++ b/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd @@ -1617,7 +1617,6 @@ <xs:extension base="BaseResource"> <xs:sequence> <xs:element ref="domain" minOccurs="0"/> - <xs:element name="external_id" type="xs:string" minOccurs="0" maxOccurs="1"/> <xs:element name="department" type="xs:string" minOccurs="0" maxOccurs="1"/> <xs:element name="logged_in" type="xs:boolean" minOccurs="0" maxOccurs="1"/> <xs:element name="last_name" type="xs:string" minOccurs="0" maxOccurs="1"/> @@ -1687,7 +1686,6 @@ <xs:extension base="BaseResource"> <xs:sequence> <xs:element ref="domain" minOccurs="0"/> - <xs:element name="external_id" type="xs:string" minOccurs="0" maxOccurs="1"/> <!-- used only to represent the initial role assignments for a new group, therafter modification of role assignments are only supported via the rel="roles" sub-collection --> diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendBaseTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendBaseTest.java index 40158d1..a8f6fd7 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendBaseTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendBaseTest.java @@ -65,10 +65,9 @@ * External identifiers used by directory users and groups. */ protected static final ExternalId[] EXTERNAL_IDS = { - new ExternalId(0), - new ExternalId(1), - new ExternalId(2), - new ExternalId(3), + ExternalId.fromHex(GUIDS[0].toString()), + ExternalId.fromHex(GUIDS[1].toString()), + ExternalId.fromHex(GUIDS[2].toString()), }; /** diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendGroupResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendGroupResourceTest.java index a39b706..1876a87 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendGroupResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendGroupResourceTest.java @@ -78,7 +78,6 @@ protected void verifyModel(Group model, int index) { assertEquals(GUIDS[index].toString(), model.getId()); - assertEquals(EXTERNAL_IDS[index].toHex(), model.getExternalId()); assertEquals(NAMES[index], model.getName()); assertNotNull(model.getDomain()); verifyLinks(model); diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendGroupsResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendGroupsResourceTest.java index df37608..f9cbf91 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendGroupsResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendGroupsResourceTest.java @@ -378,7 +378,6 @@ @Override protected void verifyModel(Group model, int index) { assertEquals(GUIDS[index].toString(), model.getId()); - assertEquals(EXTERNAL_IDS[index].toHex(), model.getExternalId()); assertEquals(GROUP_NAMES[index], model.getName()); assertNotNull(model.getDomain()); assertEquals(new Guid(DOMAIN.getBytes(), true).toString(), model.getDomain().getId()); diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUserResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUserResourceTest.java index b6ad4d3..9cfdf58 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUserResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUserResourceTest.java @@ -82,7 +82,6 @@ protected void verifyModel(User model, int index) { assertEquals(GUIDS[index].toString(), model.getId()); - assertEquals(EXTERNAL_IDS[index].toHex(), model.getExternalId()); assertEquals(NAMES[index], model.getName()); assertNotNull(model.getDomain()); assertTrue(model.isSetGroups()); diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java index 033a153..1f89630 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java @@ -220,7 +220,6 @@ @Override protected void verifyModel(User model, int index) { assertEquals(GUIDS[index].toString(), model.getId()); - assertEquals(EXTERNAL_IDS[index].toHex(), model.getExternalId()); assertEquals(NAMES[index] + "@" + DOMAIN, model.getUserName()); assertNotNull(model.getDomain()); assertEquals(new Guid(DOMAIN.getBytes(), true).toString(), model.getDomain().getId()); diff --git a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/GroupMapper.java b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/GroupMapper.java index 896d64e..8b21983 100644 --- a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/GroupMapper.java +++ b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/GroupMapper.java @@ -15,7 +15,6 @@ @Mapping(from = DbGroup.class, to = Group.class) public static Group map(DbGroup entity, Group template) { Group model = template != null ? template : new Group(); - model.setExternalId(entity.getExternalId().toHex()); model.setName(entity.getName()); model.setId(entity.getId().toString()); if (!StringUtils.isEmpty(entity.getDomain())) { @@ -49,16 +48,11 @@ String id = model.getId(); try { entity.setId(GuidUtils.asGuid(id)); + entity.setExternalId(ExternalId.fromHex(entity.getId().toString())); } catch (MalformedIdException exception) { // The identifier won't be a UUID if the group comes from /domains/{domain:id}/groups. } - if (!model.isSetExternalId()) { - entity.setExternalId(ExternalId.fromHex(id)); - } - } - if (model.isSetExternalId()) { - entity.setExternalId(ExternalId.fromHex(model.getExternalId())); } if (model.isSetDomain()) { Domain domain = model.getDomain(); diff --git a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UserMapper.java b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UserMapper.java index 46ec7a1..537011d 100644 --- a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UserMapper.java +++ b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UserMapper.java @@ -18,7 +18,6 @@ @Mapping(from = DbUser.class, to = User.class) public static User map(DbUser entity, User template) { User model = template != null ? template : new User(); - model.setExternalId(entity.getExternalId().toHex()); model.setName(entity.getFirstName()); model.setUserName(entity.getLoginName() + "@" + entity.getDomain()); model.setId(entity.getId().toString()); @@ -45,8 +44,8 @@ public static User map(DirectoryUser entity, User template) { User model = template != null ? template : new User(); model.setName(entity.getFirstName()); - model.setUserName(entity.getName() + "@" + entity.getDirectory().getName()); - model.setId(entity.getId().toHex()); + model.setUserName(entity.getName() + "@" + entity.getDirectoryName()); + model.setId(new Guid(entity.getId().getBytes(), true).toString()); model.setLastName(entity.getLastName()); model.setEmail(entity.getEmail()); model.setDepartment(entity.getDepartment()); @@ -76,16 +75,11 @@ String id = model.getId(); try { entity.setId(GuidUtils.asGuid(id)); + entity.setExternalId(ExternalId.fromHex(entity.getId().toString())); } catch (MalformedIdException exception) { // The identifier won't be a UUID if the user comes from /domains/{domain:id}/users. } - if (!model.isSetExternalId()) { - entity.setExternalId(ExternalId.fromHex(id)); - } - } - if (model.isSetExternalId()) { - entity.setExternalId(ExternalId.fromHex(model.getExternalId())); } if (model.isSetDomain()) { Domain domain = model.getDomain(); diff --git a/packaging/dbscripts/upgrade/03_04_0710_change_group_ids.sql b/packaging/dbscripts/upgrade/03_04_0710_change_group_ids.sql new file mode 100644 index 0000000..b1ba44d --- /dev/null +++ b/packaging/dbscripts/upgrade/03_04_0710_change_group_ids.sql @@ -0,0 +1,43 @@ +--groups.external_id holds a hex representation of the id of groups at ldap directories. +-- This script sets the guid representation at ad_groups.id, and modifies all relevant references, +--using the following steps: + +--1. Adding temp column for mapping from old uuid to new uuid +SELECT fn_db_add_column('ad_groups', 'temp_id', 'uuid'); +--Filling the new column with guid repreentation of external_id +UPDATE ad_groups SET temp_id = CAST(substring(external_id::text FROM 3 FOR 8) || + substring(external_id::text FROM 11 FOR 4) || + substring(external_id::text FROM 15 FOR 4) || + substring(external_id::text FROM 19 FOR 4) || + substring(external_id::text FROM 23 FOR 12) AS uuid); + +--2. Changing relevant group_id appearances in other tables +ALTER TABLE tags_user_group_map DROP constraint "tags_user_map_user_group"; + +UPDATE tags_user_group_map m set group_id = ( + SELECT temp_id FROM ad_groups WHERE id = m.group_id +); + +UPDATE permissions p SET ad_element_id = ( + SELECT temp_id FROM ad_groups g1 WHERE g1.id = p.ad_element_id +) WHERE EXISTS ( + SELECT id from ad_groups where id = p.ad_element_id +); + +--3. Fixing group_ids at users +CREATE temp TABLE tmp_users_groups AS + SELECT fnsplitteruuid(group_ids) as group_id, user_id from users; +UPDATE tmp_users_groups t SET group_id = ( + SELECT temp_id FROM ad_groups WHERE id = t.group_id +); +CREATE temp TABLE tmp_users_group_ids AS + SELECT user_id, array_to_string(array_agg(group_id), ',') group_ids FROM tmp_users_groups GROUP BY user_id; +UPDATE users u SET group_ids = ( + SELECT group_ids FROM tmp_users_group_ids WHERE user_id = u.user_id +); +UPDATE ad_groups SET id = temp_id; +--4. Cleanup +DROP TABLE tmp_users_group_ids; +DROP TABLE tmp_users_groups; +ALTER TABLE tags_user_group_map ADD CONSTRAINT tags_user_map_user_group FOREIGN KEY (group_id) REFERENCES ad_groups(id); +SELECT fn_db_drop_column('ad_groups','temp_id'); diff --git a/packaging/dbscripts/upgrade/03_04_0720_change_group_ids.sql b/packaging/dbscripts/upgrade/03_04_0720_change_group_ids.sql new file mode 100644 index 0000000..e80b1a3 --- /dev/null +++ b/packaging/dbscripts/upgrade/03_04_0720_change_group_ids.sql @@ -0,0 +1,28 @@ +--users.external_id holds a hex representation of the id of users at ldap directories. +-- This script sets the guid representation at users.id, and modifies all relevant references, +--using the following steps: + +--1. Adding temp column for mapping from old uuid to new uuid +SELECT fn_db_add_column('users', 'temp_id', 'uuid'); +--Filling the new column with guid repreentation of external_id +UPDATE users SET temp_id = CAST(substring(external_id::text FROM 3 FOR 8) || + substring(external_id::text FROM 11 FOR 4) || + substring(external_id::text FROM 15 FOR 4) || + substring(external_id::text FROM 19 FOR 4) || + substring(external_id::text FROM 23 FOR 12) AS uuid); + +--2. Changing relevant group_id appearances in other tables +ALTER TABLE tags_user_map DROP constraint "tags_user_map_user"; +UPDATE tags_user_map m set user_id = ( + SELECT temp_id FROM users WHERE user_id = m.user_id); + +UPDATE permissions p SET ad_element_id = ( + SELECT temp_id FROM users u1 WHERE u1.user_id = p.ad_element_id +) WHERE EXISTS ( + SELECT user_id from users where user_id = p.ad_element_id +); + +UPDATE users SET user_id = temp_id; +--3. Cleanup +ALTER TABLE tags_user_map ADD CONSTRAINT tags_user_map_user FOREIGN KEY (user_id) REFERENCES users(user_id); +SELECT fn_db_drop_column('users','temp_id'); -- To view, visit http://gerrit.ovirt.org/26053 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iff51fb9df33b4b679eba5b1c3ef2bea6ea7b3e07 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Yair Zaslavsky <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
