65571: REST changes to delete of an item

- Copy relationship data on item delete
This commit is contained in:
Peter Nijs
2019-11-06 14:44:08 +01:00
parent a31dc75c3e
commit ca37eee3fe
6 changed files with 347 additions and 1 deletions

View File

@@ -695,6 +695,11 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl<Item> implements It
log.info(LogManager.getHeader(context, "delete_item", "item_id=" log.info(LogManager.getHeader(context, "delete_item", "item_id="
+ item.getID())); + item.getID()));
// Remove relationships
for (Relationship relationship : relationshipService.findByItem(context, item)) {
relationshipService.delete(context, relationship, false, false);
}
// Remove bundles // Remove bundles
removeAllBundles(context, item); removeAllBundles(context, item);

View File

@@ -10,9 +10,11 @@ package org.dspace.app.rest.repository;
import java.io.IOException; import java.io.IOException;
import java.sql.SQLException; import java.sql.SQLException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.UUID; import java.util.UUID;
import java.util.stream.Collectors;
import javax.servlet.ServletInputStream; import javax.servlet.ServletInputStream;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
@@ -32,10 +34,14 @@ import org.dspace.app.rest.repository.patch.ItemPatch;
import org.dspace.authorize.AuthorizeException; import org.dspace.authorize.AuthorizeException;
import org.dspace.content.Collection; import org.dspace.content.Collection;
import org.dspace.content.Item; import org.dspace.content.Item;
import org.dspace.content.Relationship;
import org.dspace.content.RelationshipType;
import org.dspace.content.WorkspaceItem; import org.dspace.content.WorkspaceItem;
import org.dspace.content.service.CollectionService; import org.dspace.content.service.CollectionService;
import org.dspace.content.service.InstallItemService; import org.dspace.content.service.InstallItemService;
import org.dspace.content.service.ItemService; import org.dspace.content.service.ItemService;
import org.dspace.content.service.RelationshipService;
import org.dspace.content.service.RelationshipTypeService;
import org.dspace.content.service.WorkspaceItemService; import org.dspace.content.service.WorkspaceItemService;
import org.dspace.core.Context; import org.dspace.core.Context;
import org.dspace.util.UUIDUtils; import org.dspace.util.UUIDUtils;
@@ -78,6 +84,12 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
@Autowired @Autowired
InstallItemService installItemService; InstallItemService installItemService;
@Autowired
RelationshipService relationshipService;
@Autowired
RelationshipTypeService relationshipTypeService;
public ItemRestRepository(ItemService dsoService, public ItemRestRepository(ItemService dsoService,
ItemConverter dsoConverter, ItemConverter dsoConverter,
ItemPatch dsoPatch) { ItemPatch dsoPatch) {
@@ -159,6 +171,9 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
@Override @Override
@PreAuthorize("hasAuthority('ADMIN')") @PreAuthorize("hasAuthority('ADMIN')")
protected void delete(Context context, UUID id) throws AuthorizeException { protected void delete(Context context, UUID id) throws AuthorizeException {
String[] copyVirtual =
requestService.getCurrentRequest().getServletRequest().getParameterValues("copyVirtualMetadata");
Item item = null; Item item = null;
try { try {
item = is.find(context, id); item = is.find(context, id);
@@ -178,12 +193,56 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
throw new RuntimeException(e.getMessage(), e); throw new RuntimeException(e.getMessage(), e);
} }
try { try {
deleteMultipleRelationshipsCopyVirtualMetadata(context, copyVirtual, item);
is.delete(context, item); is.delete(context, item);
} catch (SQLException | IOException e) { } catch (SQLException | IOException e) {
throw new RuntimeException(e.getMessage(), e); throw new RuntimeException(e.getMessage(), e);
} }
} }
private void deleteMultipleRelationshipsCopyVirtualMetadata(Context context, String[] copyVirtual, Item item)
throws SQLException, AuthorizeException {
if (copyVirtual == null) {
return;
}
if (copyVirtual.length == 1 && StringUtils.equals("all", copyVirtual[0])) {
for (Relationship relationship : relationshipService.findByItem(context, item)) {
deleteRelationshipCopyVirtualMetadata(item, relationship);
}
} else if (copyVirtual.length == 1 && StringUtils.equals("configured", copyVirtual[0])) {
for (Relationship relationship : relationshipService.findByItem(context, item)) {
relationshipService.delete(obtainContext(), relationship);
}
} else if (copyVirtual.length > 0) {
List<Integer> relationshipIds =
Arrays.stream(copyVirtual).filter(StringUtils::isNumeric).collect(Collectors.toList())
.stream().map(Integer::parseInt).collect(Collectors.toList());
for (Integer relationshipId : relationshipIds) {
RelationshipType relationshipType = relationshipTypeService.find(context, relationshipId);
for (Relationship relationship : relationshipService
.findByItemAndRelationshipType(context, item, relationshipType)) {
deleteRelationshipCopyVirtualMetadata(item, relationship);
}
}
}
}
private void deleteRelationshipCopyVirtualMetadata(Item itemToDelete, Relationship relationshipToDelete)
throws SQLException, AuthorizeException {
boolean copyToLeft = relationshipToDelete.getRightItem().equals(itemToDelete);
boolean copyToRight = relationshipToDelete.getLeftItem().equals(itemToDelete);
if (copyToLeft && copyToRight) {
copyToLeft = false;
copyToRight = false;
}
relationshipService.delete(obtainContext(), relationshipToDelete, copyToLeft, copyToRight);
}
@Override @Override
@PreAuthorize("hasAuthority('ADMIN')") @PreAuthorize("hasAuthority('ADMIN')")
protected ItemRest createAndReturn(Context context) throws AuthorizeException, SQLException { protected ItemRest createAndReturn(Context context) throws AuthorizeException, SQLException {

View File

@@ -8,6 +8,7 @@
package org.dspace.app.rest; package org.dspace.app.rest;
import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
@@ -17,6 +18,7 @@ import java.util.List;
import org.dspace.app.rest.builder.CollectionBuilder; import org.dspace.app.rest.builder.CollectionBuilder;
import org.dspace.app.rest.builder.CommunityBuilder; import org.dspace.app.rest.builder.CommunityBuilder;
import org.dspace.app.rest.builder.GroupBuilder;
import org.dspace.app.rest.builder.ItemBuilder; import org.dspace.app.rest.builder.ItemBuilder;
import org.dspace.app.rest.builder.RelationshipBuilder; import org.dspace.app.rest.builder.RelationshipBuilder;
import org.dspace.app.rest.test.AbstractEntityIntegrationTest; import org.dspace.app.rest.test.AbstractEntityIntegrationTest;
@@ -29,6 +31,10 @@ import org.dspace.content.RelationshipType;
import org.dspace.content.service.EntityTypeService; import org.dspace.content.service.EntityTypeService;
import org.dspace.content.service.ItemService; import org.dspace.content.service.ItemService;
import org.dspace.content.service.RelationshipTypeService; import org.dspace.content.service.RelationshipTypeService;
import org.dspace.core.I18nUtil;
import org.dspace.eperson.EPerson;
import org.dspace.eperson.service.EPersonService;
import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
@@ -44,12 +50,21 @@ public class RelationshipDeleteRestRepositoryIT extends AbstractEntityIntegratio
@Autowired @Autowired
private RelationshipTypeService relationshipTypeService; private RelationshipTypeService relationshipTypeService;
@Autowired
private EPersonService ePersonService;
private Item leftItem; private Item leftItem;
private Item rightItem; private Item rightItem;
private Collection collection; private Collection collection;
private RelationshipType relationshipType; private RelationshipType relationshipType;
private Relationship relationship; private Relationship relationship;
private String adminAuthToken; private String adminAuthToken;
private EPerson collectionAdmin;
private Item personItem;
private Item projectItem;
private Item publicationItem;
private RelationshipType personProjectRelationshipType;
private RelationshipType publicationPersonRelationshipType;
@Before @Before
@Override @Override
@@ -58,15 +73,47 @@ public class RelationshipDeleteRestRepositoryIT extends AbstractEntityIntegratio
adminAuthToken = getAuthToken(admin.getEmail(), password); adminAuthToken = getAuthToken(admin.getEmail(), password);
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
collectionAdmin = ePersonService.findByEmail(context, "collectionAdminTest@email.com");
if (collectionAdmin == null) {
// This EPerson creation should only happen once (i.e. for first test run)
collectionAdmin = ePersonService.create(context);
collectionAdmin.setFirstName(context, "first");
collectionAdmin.setLastName(context, "last");
collectionAdmin.setEmail("collectionAdminTest@email.com");
collectionAdmin.setCanLogIn(true);
collectionAdmin.setLanguage(context, I18nUtil.getDefaultLocale().getLanguage());
ePersonService.setPassword(collectionAdmin, password);
// actually save the eperson to unit testing DB
ePersonService.update(context, collectionAdmin);
}
Community community = CommunityBuilder.createCommunity(context) Community community = CommunityBuilder.createCommunity(context)
.withName("Parent community") .withName("Parent community")
.build(); .build();
collection = CollectionBuilder.createCollection(context, community) collection = CollectionBuilder.createCollection(context, community)
.withName("Collection") .withName("Collection")
.withAdminGroup(collectionAdmin)
.build(); .build();
context.restoreAuthSystemState(); context.restoreAuthSystemState();
} }
@After
@Override
public void destroy() throws Exception {
try {
context.turnOffAuthorisationSystem();
collectionAdmin = ePersonService.findByEmail(context, "collectionAdminTest@email.com");
if (collectionAdmin != null) {
ePersonService.delete(context, collectionAdmin);
context.commit();
}
} catch (Exception e) {
throw new RuntimeException(e);
}
super.destroy();
}
private void initPublicationAuthor() throws Exception { private void initPublicationAuthor() throws Exception {
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
@@ -115,6 +162,63 @@ public class RelationshipDeleteRestRepositoryIT extends AbstractEntityIntegratio
context.restoreAuthSystemState(); context.restoreAuthSystemState();
} }
private void initPersonProjectPublication() throws Exception {
context.turnOffAuthorisationSystem();
personItem = ItemBuilder.createItem(context, collection)
.withTitle("Person 1")
.withPersonIdentifierFirstName("Donald")
.withPersonIdentifierLastName("Smith")
.withRelationshipType("Person")
.build();
projectItem = ItemBuilder.createItem(context, collection)
.withTitle("Project 1")
.withRelationshipType("Project")
.build();
publicationItem = ItemBuilder.createItem(context, collection)
.withTitle("Publication 1")
.withRelationshipType("Publication")
.build();
personProjectRelationshipType = relationshipTypeService.findbyTypesAndLabels(context,
entityTypeService.findByEntityType(context, "Person"),
entityTypeService.findByEntityType(context, "Project"),
"isProjectOfPerson",
"isPersonOfProject");
publicationPersonRelationshipType = relationshipTypeService.findbyTypesAndLabels(context,
entityTypeService.findByEntityType(context, "Publication"),
entityTypeService.findByEntityType(context, "Person"),
"isAuthorOfPublication",
"isPublicationOfAuthor");
RelationshipBuilder
.createRelationshipBuilder(context, personItem, projectItem, personProjectRelationshipType)
.withLeftPlace(0)
.build();
RelationshipBuilder
.createRelationshipBuilder(context, publicationItem, personItem, publicationPersonRelationshipType)
.withLeftPlace(0)
.build();
context.restoreAuthSystemState();
publicationItem = itemService.find(context, publicationItem.getID());
List<MetadataValue> publicationAuthorList =
itemService.getMetadata(publicationItem, "dc", "contributor", "author", Item.ANY);
assertThat(publicationAuthorList.size(), equalTo(1));
assertThat(publicationAuthorList.get(0).getValue(), equalTo("Smith, Donald"));
assertThat(publicationAuthorList.get(0).getAuthority(), startsWith("virtual::"));
List<MetadataValue> publicationRelationships = itemService.getMetadata(publicationItem,
"relation", "isAuthorOfPublication", Item.ANY, Item.ANY);
assertThat(publicationRelationships.size(), equalTo(1));
projectItem = itemService.find(context, projectItem.getID());
List<MetadataValue> projectAuthorList =
itemService.getMetadata(projectItem, "project", "contributor", "author", Item.ANY);
assertThat(projectAuthorList.size(), equalTo(1));
assertThat(projectAuthorList.get(0).getValue(), equalTo("Smith, Donald"));
assertThat(projectAuthorList.get(0).getAuthority(), startsWith("virtual::"));
List<MetadataValue> projectRelationships = itemService.getMetadata(projectItem,
"relation", "isPersonOfProject", Item.ANY, Item.ANY);
assertThat(projectRelationships.size(), equalTo(1));
}
@Test @Test
public void testDeleteAuthorRelationshipCopyToLeftItem() throws Exception { public void testDeleteAuthorRelationshipCopyToLeftItem() throws Exception {
initPublicationAuthor(); initPublicationAuthor();
@@ -236,4 +340,145 @@ public class RelationshipDeleteRestRepositoryIT extends AbstractEntityIntegratio
assertThat(issueList.size(), equalTo(1)); assertThat(issueList.size(), equalTo(1));
assertThat(issueList.get(0).getValue(), equalTo("2")); assertThat(issueList.get(0).getValue(), equalTo("2"));
} }
@Test
public void deleteItemCopyVirtualMetadataAll() throws Exception {
initPersonProjectPublication();
getClient(adminAuthToken).perform(
delete("/api/core/items/" + personItem.getID() + "?copyVirtualMetadata=all"))
.andExpect(status().isNoContent());
publicationItem = itemService.find(context, publicationItem.getID());
List<MetadataValue> publicationAuthorList = itemService.getMetadata(publicationItem,
"dc", "contributor", "author", Item.ANY);
assertThat(publicationAuthorList.size(), equalTo(1));
assertThat(publicationAuthorList.get(0).getValue(), equalTo("Smith, Donald"));
assertNull(publicationAuthorList.get(0).getAuthority());
List<MetadataValue> publicationRelationships = itemService.getMetadata(publicationItem,
"relation", "isAuthorOfPublication", Item.ANY, Item.ANY);
assertThat(publicationRelationships.size(), equalTo(0));
projectItem = itemService.find(context, projectItem.getID());
List<MetadataValue> projectAuthorList = itemService.getMetadata(projectItem,
"project", "contributor", "author", Item.ANY);
assertThat(projectAuthorList.size(), equalTo(1));
assertThat(projectAuthorList.get(0).getValue(), equalTo("Smith, Donald"));
assertNull(projectAuthorList.get(0).getAuthority());
List<MetadataValue> projectRelationships = itemService.getMetadata(projectItem,
"relation", "isPersonOfProject", Item.ANY, Item.ANY);
assertThat(projectRelationships.size(), equalTo(0));
}
@Test
public void deleteItemCopyVirtualMetadataOneType() throws Exception {
initPersonProjectPublication();
getClient(adminAuthToken).perform(
delete("/api/core/items/" + personItem.getID() + "?copyVirtualMetadata="
+ publicationPersonRelationshipType.getID()))
.andExpect(status().isNoContent());
publicationItem = itemService.find(context, publicationItem.getID());
List<MetadataValue> publicationAuthorList = itemService.getMetadata(publicationItem,
"dc", "contributor", "author", Item.ANY);
assertThat(publicationAuthorList.size(), equalTo(1));
assertThat(publicationAuthorList.get(0).getValue(), equalTo("Smith, Donald"));
assertNull(publicationAuthorList.get(0).getAuthority());
List<MetadataValue> publicationRelationships = itemService.getMetadata(publicationItem,
"relation", "isAuthorOfPublication", Item.ANY, Item.ANY);
assertThat(publicationRelationships.size(), equalTo(0));
projectItem = itemService.find(context, projectItem.getID());
List<MetadataValue> projectAuthorList = itemService.getMetadata(projectItem,
"project", "contributor", "author", Item.ANY);
assertThat(projectAuthorList.size(), equalTo(0));
List<MetadataValue> projectRelationships = itemService.getMetadata(projectItem,
"relation", "isPersonOfProject", Item.ANY, Item.ANY);
assertThat(projectRelationships.size(), equalTo(0));
}
@Test
public void deleteItemCopyVirtualMetadataTwoTypes() throws Exception {
initPersonProjectPublication();
getClient(adminAuthToken).perform(
delete("/api/core/items/" + personItem.getID()
+ "?copyVirtualMetadata=" + publicationPersonRelationshipType.getID()
+ "&copyVirtualMetadata=" + personProjectRelationshipType.getID()))
.andExpect(status().isNoContent());
publicationItem = itemService.find(context, publicationItem.getID());
List<MetadataValue> publicationAuthorList = itemService.getMetadata(publicationItem,
"dc", "contributor", "author", Item.ANY);
assertThat(publicationAuthorList.size(), equalTo(1));
assertThat(publicationAuthorList.get(0).getValue(), equalTo("Smith, Donald"));
assertNull(publicationAuthorList.get(0).getAuthority());
List<MetadataValue> publicationRelationships = itemService.getMetadata(publicationItem,
"relation", "isAuthorOfPublication", Item.ANY, Item.ANY);
assertThat(publicationRelationships.size(), equalTo(0));
projectItem = itemService.find(context, projectItem.getID());
List<MetadataValue> projectAuthorList = itemService.getMetadata(projectItem,
"project", "contributor", "author", Item.ANY);
assertThat(projectAuthorList.size(), equalTo(1));
assertThat(projectAuthorList.get(0).getValue(), equalTo("Smith, Donald"));
assertNull(projectAuthorList.get(0).getAuthority());
List<MetadataValue> projectRelationships = itemService.getMetadata(projectItem,
"relation", "isPersonOfProject", Item.ANY, Item.ANY);
assertThat(projectRelationships.size(), equalTo(0));
}
@Test
public void deleteItemCopyVirtualMetadataNone() throws Exception {
initPersonProjectPublication();
getClient(adminAuthToken).perform(
delete("/api/core/items/" + personItem.getID()))
.andExpect(status().isNoContent());
publicationItem = itemService.find(context, publicationItem.getID());
List<MetadataValue> publicationAuthorList = itemService.getMetadata(publicationItem,
"dc", "contributor", "author", Item.ANY);
assertThat(publicationAuthorList.size(), equalTo(0));
List<MetadataValue> publicationRelationships = itemService.getMetadata(publicationItem,
"relation", "isAuthorOfPublication", Item.ANY, Item.ANY);
assertThat(publicationRelationships.size(), equalTo(0));
projectItem = itemService.find(context, projectItem.getID());
List<MetadataValue> projectAuthorList = itemService.getMetadata(projectItem,
"project", "contributor", "author", Item.ANY);
assertThat(projectAuthorList.size(), equalTo(0));
List<MetadataValue> projectRelationships = itemService.getMetadata(projectItem,
"relation", "isPersonOfProject", Item.ANY, Item.ANY);
assertThat(projectRelationships.size(), equalTo(0));
}
@Test
public void deleteItemCopyVirtualMetadataAllNoPermissions() throws Exception {
initPersonProjectPublication();
getClient(getAuthToken(eperson.getEmail(), password)).perform(
delete("/api/core/items/" + personItem.getID()))
.andExpect(status().isForbidden());
}
@Test
public void deleteItemCopyVirtualMetadataAllInsufficientPermissions() throws Exception {
initPersonProjectPublication();
getClient(getAuthToken(collectionAdmin.getEmail(), password)).perform(
delete("/api/core/items/" + personItem.getID()))
.andExpect(status().isForbidden());
}
@Test
public void deleteItemCopyVirtualMetadataTypeInsufficientPermissions() throws Exception {
initPersonProjectPublication();
getClient(getAuthToken(collectionAdmin.getEmail(), password)).perform(
delete("/api/core/items/" + personItem.getID()
+ "?copyVirtualMetadata=" + publicationPersonRelationshipType.getID()))
.andExpect(status().isForbidden());
}
} }

View File

@@ -226,7 +226,12 @@ public abstract class AbstractDSpaceObjectBuilder<T extends DSpaceObject>
if (attachedDso != null) { if (attachedDso != null) {
getService().delete(c, attachedDso); getService().delete(c, attachedDso);
} }
c.complete(); try {
c.complete();
}
catch (Exception e) {
throw new RuntimeException(e);
}
} }
indexingService.commit(); indexingService.commit();

View File

@@ -98,6 +98,23 @@ public class CollectionBuilder extends AbstractDSpaceObjectBuilder<Collection> {
return this; return this;
} }
/**
* Create an admin group for the collection with the specified members
*
* @param members epersons to add to the admin group
* @return this builder
* @throws SQLException
* @throws AuthorizeException
*/
public CollectionBuilder withAdminGroup(EPerson... members) throws SQLException, AuthorizeException {
Group g = collectionService.createAdministrators(context, collection);
for (EPerson e : members) {
groupService.addMember(context, g, e);
}
groupService.update(context, g);
return this;
}
@Override @Override
public Collection build() { public Collection build() {
try { try {
@@ -114,6 +131,7 @@ public class CollectionBuilder extends AbstractDSpaceObjectBuilder<Collection> {
@Override @Override
public void cleanup() throws Exception { public void cleanup() throws Exception {
deleteWorkflowGroups(collection); deleteWorkflowGroups(collection);
//collectionService.removeAdministrators(context, collection);
delete(collection); delete(collection);
} }

View File

@@ -17,4 +17,18 @@
- project.identifier.expectedcompletion > NOT FOUND - project.identifier.expectedcompletion > NOT FOUND
--> -->
<dc-type>
<schema>project</schema>
<element>contributor</element>
<qualifier>author</qualifier>
<scope_note>Virtual metadata field that holds the full name of the author</scope_note>
</dc-type>
<dc-type>
<schema>project</schema>
<element>contributor</element>
<qualifier>other</qualifier>
<scope_note>Virtual metadata field that holds the value of organization.legalName</scope_note>
</dc-type>
</dspace-dc-types> </dspace-dc-types>