diff --git a/src/main/java/org/fairdatapoint/service/resource/ResourceDefinitionService.java b/src/main/java/org/fairdatapoint/service/resource/ResourceDefinitionService.java index c59ec6dc0..547b4a376 100644 --- a/src/main/java/org/fairdatapoint/service/resource/ResourceDefinitionService.java +++ b/src/main/java/org/fairdatapoint/service/resource/ResourceDefinitionService.java @@ -246,8 +246,13 @@ protected void createDependents(ResourceDefinition definition, ResourceDefinitio @Transactional @PreAuthorize("hasRole('ADMIN')") protected void deleteDependents(ResourceDefinition resourceDefinition) { - resourceDefinition.getChildren().forEach(this::deleteChild); - linkRepository.deleteAll(resourceDefinition.getExternalLinks()); + // Iterate over list copies because we need to modify the originals (prevent ConcurrentModificationException) + // todo: refactor after fixing #825 + List.copyOf(resourceDefinition.getChildren()).forEach(this::deleteChild); + List.copyOf(resourceDefinition.getExternalLinks()).forEach(link -> { + resourceDefinition.getExternalLinks().remove(link); + linkRepository.delete(link); + }); usageRepository.deleteAll(resourceDefinition.getMetadataSchemaUsages()); entityManager.flush(); entityManager.refresh(resourceDefinition); @@ -257,7 +262,14 @@ protected void deleteDependents(ResourceDefinition resourceDefinition) { @PreAuthorize("hasRole('ADMIN')") protected void deleteChild(ResourceDefinitionChild child) { childMetadataRepository.deleteAll(child.getMetadata()); - childRepository.delete(child); + // Remember `child` is not actually a child but rather a link entity, + // where `source` is the parent and `target` is the actual child (#821). + // We need to clear both sides of the bi-directional relation. + // todo: refactor after fixing #825 + child.getSource().getChildren().remove(child); + child.getTarget().getParents().remove(child); + child.setSource(null); + child.setTarget(null); } public List getTargetClassUris(ResourceDefinition resourceDefinition) { diff --git a/src/test/java/org/fairdatapoint/acceptance/resource/Detail_PUT.java b/src/test/java/org/fairdatapoint/acceptance/resource/Detail_PUT.java index d4b59c9a1..1366bfc38 100644 --- a/src/test/java/org/fairdatapoint/acceptance/resource/Detail_PUT.java +++ b/src/test/java/org/fairdatapoint/acceptance/resource/Detail_PUT.java @@ -24,7 +24,6 @@ import org.fairdatapoint.WebIntegrationTest; import org.fairdatapoint.api.dto.resource.ResourceDefinitionChangeDTO; -import org.fairdatapoint.api.dto.resource.ResourceDefinitionChildDTO; import org.fairdatapoint.api.dto.resource.ResourceDefinitionDTO; import org.fairdatapoint.database.db.repository.ResourceDefinitionRepository; import org.fairdatapoint.entity.resource.ResourceDefinition; @@ -37,7 +36,6 @@ import org.springframework.http.*; import java.net.URI; -import java.util.List; import java.util.UUID; import static java.lang.String.format; diff --git a/src/test/java/org/fairdatapoint/service/resource/ResourceDefinitionServiceTest.java b/src/test/java/org/fairdatapoint/service/resource/ResourceDefinitionServiceTest.java new file mode 100644 index 000000000..047026166 --- /dev/null +++ b/src/test/java/org/fairdatapoint/service/resource/ResourceDefinitionServiceTest.java @@ -0,0 +1,112 @@ +/** + * The MIT License + * Copyright © 2016-2024 FAIR Data Team + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package org.fairdatapoint.service.resource; + +import org.fairdatapoint.BaseIntegrationTest; +import org.fairdatapoint.api.dto.resource.ResourceDefinitionChangeDTO; +import org.fairdatapoint.entity.resource.ResourceDefinition; +import org.fairdatapoint.util.KnownUUIDs; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.orm.jpa.AutoConfigureTestEntityManager; +import org.springframework.boot.test.autoconfigure.orm.jpa.TestEntityManager; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.validation.BindException; + +import java.util.HashMap; +import java.util.UUID; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@AutoConfigureTestEntityManager +@Transactional // required for TestEntityManager outside of @DataJpaTest +public class ResourceDefinitionServiceTest extends BaseIntegrationTest { + + final HashMap uuids = new HashMap<>(); + + final ResourceDefinitionService resourceDefinitionService; + + final ResourceDefinitionMapper resourceDefinitionMapper; + + final TestEntityManager testEntityManager; + + /** + * Constructor + * + * @param resourceDefinitionService The service to test + */ + @Autowired + public ResourceDefinitionServiceTest( + ResourceDefinitionService resourceDefinitionService, + ResourceDefinitionMapper resourceDefinitionMapper, + TestEntityManager testEntityManager + ) { + this.resourceDefinitionService = resourceDefinitionService; + this.resourceDefinitionMapper = resourceDefinitionMapper; + this.testEntityManager = testEntityManager; + } + + /** + * Provides arguments for @ParameterizedTest + * @return UUID of resource + */ + static Stream uuidProvider() { + return Stream.of( + KnownUUIDs.RD_FDP_UUID, + KnownUUIDs.RD_CATALOG_UUID, + KnownUUIDs.RD_DATASET_UUID, + KnownUUIDs.RD_DISTRIBUTION_UUID + ); + } + + /** + * Reproduces #830, under the assumption that at least one of the default resources has children + * (in this case fdp, catalog, and dataset) and at least one of the resources has external links (distribution). + */ + @ParameterizedTest + @MethodSource("uuidProvider") + @WithMockUser(roles="ADMIN") + public void testUpdateDoesNotDuplicateRelatedItems(UUID uuid) throws BindException { + // create DTO from existing resource (without making any actual changes) + ResourceDefinition resourceDefinition = testEntityManager.find(ResourceDefinition.class, uuid); + ResourceDefinitionChangeDTO changeDTO = resourceDefinitionMapper.toChangeDTO(resourceDefinition); + // count related items, for reference + final int expectedChildCount = resourceDefinition.getChildren().size(); + final int expectedParentCount = resourceDefinition.getParents().size(); + final int expectedExternalLinkCount = resourceDefinition.getExternalLinks().size(); + final int expectedMetadataSchemaUsageCount = resourceDefinition.getMetadataSchemaUsages().size(); + assertTrue(expectedMetadataSchemaUsageCount > 0); + // call update method + resourceDefinitionService.update(uuid, changeDTO); + // check for duplicates (or other inconsistencies in the number of related items) + testEntityManager.refresh(resourceDefinition); + assertEquals(expectedChildCount, resourceDefinition.getChildren().size()); + assertEquals(expectedParentCount, resourceDefinition.getParents().size()); + assertEquals(expectedExternalLinkCount, resourceDefinition.getExternalLinks().size()); + assertEquals(expectedMetadataSchemaUsageCount, resourceDefinition.getMetadataSchemaUsages().size()); + } +}