From 1d41e99f9db91a09fb18963c46e629ed4aa95183 Mon Sep 17 00:00:00 2001 From: BVier <26220150+BVier@users.noreply.github.com> Date: Wed, 7 Aug 2019 12:11:44 +0200 Subject: [PATCH] TSK-869: Throw error for non-unique input-file --- ...sificationDefinitionControllerIntTest.java | 20 +++++++++++++++++- ...WorkbasketDefinitionControllerIntTest.java | 21 +++++++++++++++++++ .../ClassificationDefinitionController.java | 18 ++++++++++++++++ .../rest/TaskanaRestExceptionHandler.java | 6 ++++++ .../rest/WorkbasketDefinitionController.java | 20 ++++++++++++++++++ 5 files changed, 84 insertions(+), 1 deletion(-) diff --git a/rest/taskana-rest-spring-test/src/test/java/pro/taskana/rest/ClassificationDefinitionControllerIntTest.java b/rest/taskana-rest-spring-test/src/test/java/pro/taskana/rest/ClassificationDefinitionControllerIntTest.java index 06c6c7e46..f02b5ba2b 100644 --- a/rest/taskana-rest-spring-test/src/test/java/pro/taskana/rest/ClassificationDefinitionControllerIntTest.java +++ b/rest/taskana-rest-spring-test/src/test/java/pro/taskana/rest/ClassificationDefinitionControllerIntTest.java @@ -6,6 +6,7 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.File; import java.io.FileOutputStream; @@ -366,7 +367,7 @@ public class ClassificationDefinitionControllerIntTest { ResponseEntity response = importRequest(clList); assertEquals(HttpStatus.OK, response.getStatusCode()); Thread.sleep(10); - LOGGER.debug("Wait for 10 s to give the system a chance to update"); + LOGGER.debug("Wait 10 ms to give the system a chance to update"); ClassificationSummaryResource childWithNewParent = this.getClassificationWithKeyAndDomain("L110105", "DOMAIN_A"); @@ -378,6 +379,23 @@ public class ClassificationDefinitionControllerIntTest { assertEquals(child2.parentKey, childWithoutParent.parentKey); } + @Test + public void testFailOnImportDuplicates() throws IOException { + ClassificationSummaryResource classification = this.getClassificationWithKeyAndDomain("L110105", "DOMAIN_A"); + String classificationString = objMapper.writeValueAsString(classification); + + List clList = new ArrayList<>(); + clList.add(classificationString); + clList.add(classificationString); + + try { + importRequest(clList); + fail("Expected http-Status 409"); + } catch (HttpClientErrorException e) { + assertEquals(HttpStatus.CONFLICT, e.getStatusCode()); + } + } + private ClassificationResource createClassification(String id, String key, String domain, String parentId, String parentKey) { ClassificationResource classificationResource = new ClassificationResource(); diff --git a/rest/taskana-rest-spring-test/src/test/java/pro/taskana/rest/WorkbasketDefinitionControllerIntTest.java b/rest/taskana-rest-spring-test/src/test/java/pro/taskana/rest/WorkbasketDefinitionControllerIntTest.java index 46aecdd64..98974ef92 100644 --- a/rest/taskana-rest-spring-test/src/test/java/pro/taskana/rest/WorkbasketDefinitionControllerIntTest.java +++ b/rest/taskana-rest-spring-test/src/test/java/pro/taskana/rest/WorkbasketDefinitionControllerIntTest.java @@ -5,6 +5,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; import java.io.File; import java.io.FileWriter; @@ -35,6 +36,7 @@ import org.springframework.http.converter.json.MappingJackson2HttpMessageConvert import org.springframework.test.context.junit4.SpringRunner; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; import com.fasterxml.jackson.databind.DeserializationFeature; @@ -116,6 +118,25 @@ public class WorkbasketDefinitionControllerIntTest { assertEquals(HttpStatus.OK, responseImport.getStatusCode()); } + @Test + public void testFailOnImportDuplicates() throws IOException { + ResponseEntity> response = template.exchange( + server + port + "/v1/workbasket-definitions?domain=DOMAIN_A", + HttpMethod.GET, request, new ParameterizedTypeReference>() { + + }); + + List list = new ArrayList<>(); + list.add(objMapper.writeValueAsString(response.getBody().get(0))); + list.add(objMapper.writeValueAsString(response.getBody().get(0))); + try { + importRequest(list); + fail("Expected http-Status 409"); + } catch (HttpClientErrorException e) { + assertEquals(HttpStatus.CONFLICT, e.getStatusCode()); + } + } + @Test public void testNoErrorWhenImportWithSameIdButDifferentKeyAndDomain() throws IOException { diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/ClassificationDefinitionController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/ClassificationDefinitionController.java index c41ad6d62..12082904a 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/ClassificationDefinitionController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/ClassificationDefinitionController.java @@ -11,6 +11,7 @@ import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.dao.DuplicateKeyException; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; @@ -95,6 +96,7 @@ public class ClassificationDefinitionController { LOGGER.debug("Entry to importClassifications()"); Map systemIds = getSystemIds(); List classificationsResources = extractClassificationResourcesFromFile(file); + checkForDuplicates(classificationsResources); Map childrenInFile = mapChildrenToParentKeys(classificationsResources, systemIds); insertOrUpdateClassificationsWithoutParent(classificationsResources, systemIds); @@ -123,6 +125,22 @@ public class ClassificationDefinitionController { return classificationsDefinitions; } + private void checkForDuplicates(List classificationList) { + List identifiers = new ArrayList<>(); + Set duplicates = new HashSet<>(); + for (ClassificationResource classification : classificationList) { + String identifier = classification.key + "|" + classification.domain; + if (identifiers.contains(identifier)) { + duplicates.add(identifier); + } else { + identifiers.add(identifier); + } + } + if (!duplicates.isEmpty()) { + throw new DuplicateKeyException("The 'key|domain'-identifier is not unique for the value(s): " + duplicates.toString()); + } + } + private Map mapChildrenToParentKeys(List classificationResources, Map systemIds) { LOGGER.debug("Entry to mapChildrenToParentKeys()"); diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskanaRestExceptionHandler.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskanaRestExceptionHandler.java index 9544374d5..b82a9618d 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskanaRestExceptionHandler.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskanaRestExceptionHandler.java @@ -4,6 +4,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; +import org.springframework.dao.DuplicateKeyException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ControllerAdvice; @@ -86,6 +87,11 @@ public class TaskanaRestExceptionHandler extends ResponseEntityExceptionHandler return buildResponse(ex, req, HttpStatus.CONFLICT); } + @ExceptionHandler(DuplicateKeyException.class) + protected ResponseEntity handleDuplicateKey(DuplicateKeyException ex, WebRequest req) { + return buildResponse(ex, req, HttpStatus.CONFLICT); + } + @ExceptionHandler(ConcurrencyException.class) protected ResponseEntity handleConcurrencyException(ConcurrencyException ex, WebRequest req) { return buildResponse(ex, req, HttpStatus.LOCKED); diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/WorkbasketDefinitionController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/WorkbasketDefinitionController.java index b7c1fde63..33b385c75 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/WorkbasketDefinitionController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/WorkbasketDefinitionController.java @@ -3,12 +3,15 @@ package pro.taskana.rest; import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.dao.DuplicateKeyException; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; @@ -122,6 +125,7 @@ public class WorkbasketDefinitionController { .list() .stream() .collect(Collectors.toMap(this::logicalId, WorkbasketSummary::getId)); + checkForDuplicates(definitions); // key: old system ID // value: system ID @@ -179,6 +183,22 @@ public class WorkbasketDefinitionController { return workbasketDefinitionAssembler.toModel(wbRes); } + private void checkForDuplicates(List definitions) { + List identifiers = new ArrayList<>(); + Set duplicates = new HashSet<>(); + for (WorkbasketDefinitionResource definition : definitions) { + String identifier = logicalId(workbasketDefinitionAssembler.toModel(definition.getWorkbasket())); + if (identifiers.contains(identifier)) { + duplicates.add(identifier); + } else { + identifiers.add(identifier); + } + } + if (!duplicates.isEmpty()) { + throw new DuplicateKeyException("The 'key|domain'-identifier is not unique for the value(s): " + duplicates.toString()); + } + } + private String logicalId(WorkbasketSummary workbasket) { return logicalId(workbasket.getKey(), workbasket.getDomain()); }