diff --git a/dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java index b61c33dab8..53cfb34282 100644 --- a/dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java @@ -412,11 +412,8 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl imp authorizeService.authorizeAction(context, parentCommunity, Constants.ADD); Community c; - if (uuid != null) { - c = create(parentCommunity, context, handle, uuid); - } else { - c = create(parentCommunity, context, handle); - } + c = create(parentCommunity, context, handle, uuid); + addSubcommunity(context, parentCommunity, c); return c; diff --git a/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java index b435df4d4e..2244d4b149 100644 --- a/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java @@ -176,34 +176,28 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl implements It @Override public Item create(Context context, WorkspaceItem workspaceItem) throws SQLException, AuthorizeException { - authorizeService.authorizeAction(context, workspaceItem.getCollection(), Constants.ADD); - if (workspaceItem.getItem() != null) { - throw new IllegalArgumentException( - "Attempting to create an item for a workspace item that already contains an item"); - } - Item item = createItem(context); - workspaceItem.setItem(item); - - - log.info(LogHelper.getHeader(context, "create_item", "item_id=" - + item.getID())); - - return item; + return create(context, workspaceItem, null); } @Override public Item create(Context context, WorkspaceItem workspaceItem, UUID uuid) throws SQLException, AuthorizeException { - authorizeService.authorizeAction(context, workspaceItem.getCollection(), Constants.ADD); + Collection collection = workspaceItem.getCollection(); + authorizeService.authorizeAction(context, collection, Constants.ADD); if (workspaceItem.getItem() != null) { throw new IllegalArgumentException( "Attempting to create an item for a workspace item that already contains an item"); } - Item item = createItem(context, uuid); + Item item = null; + if (uuid != null) { + item = createItem(context, uuid); + } else { + item = createItem(context); + } workspaceItem.setItem(item); - log.info(LogManager.getHeader(context, "create_item", "item_id=" + log.info(LogHelper.getHeader(context, "create_item", "item_id=" + item.getID())); return item; diff --git a/dspace-api/src/test/java/org/dspace/app/packager/PackagerIT.java b/dspace-api/src/test/java/org/dspace/app/packager/PackagerIT.java index f09b30fb31..037404ef40 100644 --- a/dspace-api/src/test/java/org/dspace/app/packager/PackagerIT.java +++ b/dspace-api/src/test/java/org/dspace/app/packager/PackagerIT.java @@ -8,7 +8,9 @@ package org.dspace.app.packager; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import java.io.File; @@ -26,12 +28,15 @@ import org.dspace.builder.ItemBuilder; import org.dspace.content.Collection; import org.dspace.content.Community; import org.dspace.content.Item; +import org.dspace.content.WorkspaceItem; import org.dspace.content.crosswalk.MetadataValidationException; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.packager.METSManifest; import org.dspace.content.service.CollectionService; import org.dspace.content.service.CommunityService; +import org.dspace.content.service.InstallItemService; import org.dspace.content.service.ItemService; +import org.dspace.content.service.WorkspaceItemService; import org.dspace.services.ConfigurationService; import org.dspace.services.factory.DSpaceServicesFactory; import org.jdom.Element; @@ -42,6 +47,9 @@ public class PackagerIT extends AbstractIntegrationTestWithDatabase { private ItemService itemService = ContentServiceFactory.getInstance().getItemService(); private CollectionService collectionService = ContentServiceFactory.getInstance().getCollectionService(); private CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService(); + private WorkspaceItemService workspaceItemService = ContentServiceFactory.getInstance().getWorkspaceItemService(); + protected static final InstallItemService installItemService = ContentServiceFactory.getInstance() + .getInstallItemService(); protected ConfigurationService configService = DSpaceServicesFactory.getInstance().getConfigurationService(); protected Community child1; protected Collection col1; @@ -134,6 +142,29 @@ public class PackagerIT extends AbstractIntegrationTestWithDatabase { } } + @Test + public void packagerUUIDAlreadyExistWithoutForceTest() throws Exception { + context.turnOffAuthorisationSystem(); + createTemplate(); + createFiles(); + try { + //should fail to restore the item because the uuid already exists. + performExportScript(article.getHandle(), tempFile); + UUID id = article.getID(); + itemService.delete(context, article); + WorkspaceItem workspaceItem = workspaceItemService.create(context, col1, id, false); + installItemService.installItem(context, workspaceItem, "123456789/100"); + performImportNoForceScript(tempFile); + Iterator items = itemService.findByCollection(context, col1); + Item testItem = items.next(); + assertFalse(items.hasNext()); //check to make sure there is only 1 item + assertSame("123456789/100", testItem.getHandle()); //check to make sure the item wasn't overwritten as + // it would have the old handle. + } finally { + tempFile.delete(); + } + } + protected void createTemplate() { parentCommunity = CommunityBuilder.createCommunity(context) .withName("Parent Community") @@ -179,6 +210,11 @@ public class PackagerIT extends AbstractIntegrationTestWithDatabase { "AIP", outputFile.getPath()); } + private void performImportNoForceScript(File outputFile) throws Exception { + runDSpaceScript("packager", "-r", "-u", "-e", "admin@email.com", "-t", + "AIP", outputFile.getPath()); + } + private void performImportScript(File outputFile) throws Exception { runDSpaceScript("packager", "-r", "-f", "-u", "-e", "admin@email.com", "-t", "AIP", outputFile.getPath()); diff --git a/dspace-api/src/test/java/org/dspace/content/WorkspaceItemTest.java b/dspace-api/src/test/java/org/dspace/content/WorkspaceItemTest.java index 609768bf67..43afbead70 100644 --- a/dspace-api/src/test/java/org/dspace/content/WorkspaceItemTest.java +++ b/dspace-api/src/test/java/org/dspace/content/WorkspaceItemTest.java @@ -14,6 +14,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.spy; @@ -33,12 +35,15 @@ import org.dspace.content.service.CommunityService; import org.dspace.content.service.ItemService; import org.dspace.content.service.WorkspaceItemService; import org.dspace.core.Constants; +import org.dspace.core.Context; import org.dspace.eperson.EPerson; import org.dspace.eperson.factory.EPersonServiceFactory; import org.dspace.eperson.service.EPersonService; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; import org.springframework.test.util.ReflectionTestUtils; /** @@ -46,6 +51,7 @@ import org.springframework.test.util.ReflectionTestUtils; * * @author pvillega */ +@RunWith(MockitoJUnitRunner.class) public class WorkspaceItemTest extends AbstractUnitTest { /** @@ -98,6 +104,7 @@ public class WorkspaceItemTest extends AbstractUnitTest { // "Wire" our spy to be used by the current loaded object services // (To ensure these services use the spy instead of the real service) ReflectionTestUtils.setField(workspaceItemService, "authorizeService", authorizeServiceSpy); + ReflectionTestUtils.setField(itemService, "authorizeService", authorizeServiceSpy); ReflectionTestUtils.setField(collectionService, "authorizeService", authorizeServiceSpy); ReflectionTestUtils.setField(communityService, "authorizeService", authorizeServiceSpy); } catch (AuthorizeException ex) { @@ -158,7 +165,9 @@ public class WorkspaceItemTest extends AbstractUnitTest { @Test public void testCreateAuth() throws Exception { // Allow Collection ADD perms - doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADD); + //doNothing().when(authorizeServiceSpy).authorizeAction(context, collection, Constants.ADD); + doNothing().when(authorizeServiceSpy).authorizeAction(any(Context.class), + any(Collection.class), eq(Constants.ADD)); boolean template; WorkspaceItem created;