From ad026b78796aaf5579d2179bbf96d1469ac6a7f5 Mon Sep 17 00:00:00 2001 From: BVier <26220150+BVier@users.noreply.github.com> Date: Wed, 7 Aug 2019 18:10:59 +0200 Subject: [PATCH] TSK-870: Create new Id for imported workbasket --- .../taskana/impl/WorkbasketServiceImpl.java | 8 +++++-- .../taskana/mappings/WorkbasketMapper.java | 3 +++ .../workbasket/CreateWorkbasketAccTest.java | 24 +++++++++++++++++++ ...WorkbasketDefinitionControllerIntTest.java | 18 ++++++++++++++ .../rest/WorkbasketDefinitionController.java | 10 +++++++- 5 files changed, 60 insertions(+), 3 deletions(-) diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java index a2dc4dda3..9dc7d8786 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java @@ -104,7 +104,7 @@ public class WorkbasketServiceImpl implements WorkbasketService { public Workbasket createWorkbasket(Workbasket newWorkbasket) throws InvalidWorkbasketException, NotAuthorizedException, WorkbasketAlreadyExistException, DomainNotFoundException { - LOGGER.debug("entry to createtWorkbasket(workbasket)", newWorkbasket); + LOGGER.debug("entry to createWorkbasket(workbasket)", newWorkbasket); taskanaEngine.checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); WorkbasketImpl workbasket = (WorkbasketImpl) newWorkbasket; @@ -143,7 +143,11 @@ public class WorkbasketServiceImpl implements WorkbasketService { try { taskanaEngine.openConnection(); workbasket.setModified(Instant.now()); - workbasketMapper.update(workbasket); + if (workbasket.getId() == null || workbasket.getId().isEmpty()) { + workbasketMapper.updateByKeyAndDomain(workbasket); + } else { + workbasketMapper.update(workbasket); + } LOGGER.debug("Method updateWorkbasket() updated workbasket '{}'", workbasket.getId()); return workbasket; } finally { diff --git a/lib/taskana-core/src/main/java/pro/taskana/mappings/WorkbasketMapper.java b/lib/taskana-core/src/main/java/pro/taskana/mappings/WorkbasketMapper.java index 10009e536..e708f42c4 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/mappings/WorkbasketMapper.java +++ b/lib/taskana-core/src/main/java/pro/taskana/mappings/WorkbasketMapper.java @@ -160,6 +160,9 @@ public interface WorkbasketMapper { @Update("UPDATE WORKBASKET SET MODIFIED = #{workbasket.modified}, KEY = #{workbasket.key}, NAME = #{workbasket.name}, DOMAIN = #{workbasket.domain}, TYPE = #{workbasket.type}, DESCRIPTION = #{workbasket.description}, OWNER = #{workbasket.owner}, CUSTOM_1 = #{workbasket.custom1}, CUSTOM_2 = #{workbasket.custom2}, CUSTOM_3 = #{workbasket.custom3}, CUSTOM_4 = #{workbasket.custom4}, ORG_LEVEL_1 = #{workbasket.orgLevel1}, ORG_LEVEL_2 = #{workbasket.orgLevel2}, ORG_LEVEL_3 = #{workbasket.orgLevel3}, ORG_LEVEL_4 = #{workbasket.orgLevel4}, MARKED_FOR_DELETION = #{workbasket.markedForDeletion} WHERE id = #{workbasket.id}") void update(@Param("workbasket") WorkbasketImpl workbasket); + @Update("UPDATE WORKBASKET SET MODIFIED = #{workbasket.modified}, NAME = #{workbasket.name}, TYPE = #{workbasket.type}, DESCRIPTION = #{workbasket.description}, OWNER = #{workbasket.owner}, CUSTOM_1 = #{workbasket.custom1}, CUSTOM_2 = #{workbasket.custom2}, CUSTOM_3 = #{workbasket.custom3}, CUSTOM_4 = #{workbasket.custom4}, ORG_LEVEL_1 = #{workbasket.orgLevel1}, ORG_LEVEL_2 = #{workbasket.orgLevel2}, ORG_LEVEL_3 = #{workbasket.orgLevel3}, ORG_LEVEL_4 = #{workbasket.orgLevel4}, MARKED_FOR_DELETION = #{workbasket.markedForDeletion} WHERE KEY = #{workbasket.key} AND DOMAIN = #{workbasket.domain}") + void updateByKeyAndDomain(@Param("workbasket") WorkbasketImpl workbasket); + @Delete("DELETE FROM WORKBASKET where id = #{id}") void delete(@Param("id") String id); } diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/CreateWorkbasketAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/CreateWorkbasketAccTest.java index c0c8d97f1..b2ea36cb2 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/CreateWorkbasketAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/CreateWorkbasketAccTest.java @@ -188,6 +188,30 @@ public class CreateWorkbasketAccTest extends AbstractAccTest { duplicateWorkbasketWithSmallX = workbasketService.createWorkbasket(duplicateWorkbasketWithSmallX); } + @WithAccessId( + userName = "user_1_2", + groupNames = {"businessadmin"}) + @Test(expected = WorkbasketAlreadyExistException.class) + public void testCreateWorkbasketWithAlreadyExistingKeyAndDomainAndEmptyIdUpdatesOlderWorkbasket() + throws DomainNotFoundException, InvalidWorkbasketException, NotAuthorizedException, WorkbasketAlreadyExistException { + WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + // First create a new Workbasket. + Workbasket wb = workbasketService.newWorkbasket("newKey", "DOMAIN_A"); + wb.setType(WorkbasketType.GROUP); + wb.setName("this name"); + wb = workbasketService.createWorkbasket(wb); + + // Second create a new Workbasket with same Key and Domain. + Workbasket sameKeyAndDomain = workbasketService.newWorkbasket("newKey", "DOMAIN_A"); + sameKeyAndDomain.setType(WorkbasketType.TOPIC); + sameKeyAndDomain.setName("new name"); + sameKeyAndDomain = workbasketService.createWorkbasket(sameKeyAndDomain); + + assertEquals(wb.getId(), sameKeyAndDomain.getId()); + assertEquals(WorkbasketType.TOPIC, sameKeyAndDomain.getType()); + assertEquals("new name", sameKeyAndDomain.getName()); + } + @WithAccessId( userName = "user_1_2", groupNames = {"businessadmin"}) 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 0fa6878da..46aecdd64 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 @@ -116,6 +116,24 @@ public class WorkbasketDefinitionControllerIntTest { assertEquals(HttpStatus.OK, responseImport.getStatusCode()); } + @Test + public void testNoErrorWhenImportWithSameIdButDifferentKeyAndDomain() + throws IOException { + ResponseEntity> response = template.exchange( + server + port + "/v1/workbasket-definitions?domain=DOMAIN_A", + HttpMethod.GET, request, new ParameterizedTypeReference>() { + + }); + + List list = new ArrayList<>(); + WorkbasketDefinitionResource wbDef = response.getBody().get(0); + list.add(objMapper.writeValueAsString(wbDef)); + wbDef.getWorkbasket().setKey("new Key for this WB"); + list.add(objMapper.writeValueAsString(wbDef)); + ResponseEntity responseImport = importRequest(list); + assertEquals(HttpStatus.OK, responseImport.getStatusCode()); + } + private ResponseEntity importRequest(List clList) throws IOException { File tmpFile = File.createTempFile("test", ".tmp"); FileWriter writer = new FileWriter(tmpFile); 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 c8b8a0963..b7c1fde63 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 @@ -38,6 +38,7 @@ import pro.taskana.exceptions.WorkbasketAlreadyExistException; import pro.taskana.exceptions.WorkbasketNotFoundException; import pro.taskana.rest.resource.WorkbasketDefinitionResource; import pro.taskana.rest.resource.WorkbasketDefinitionResourceAssembler; +import pro.taskana.rest.resource.WorkbasketResource; /** * Controller for all {@link WorkbasketDefinitionResource} related endpoints. @@ -133,7 +134,8 @@ public class WorkbasketDefinitionController { if (systemIds.containsKey(logicalId(importedWb))) { workbasket = workbasketService.updateWorkbasket(importedWb); } else { - workbasket = workbasketService.createWorkbasket(importedWb); + Workbasket wbWithoutId = removeId(importedWb); + workbasket = workbasketService.createWorkbasket(wbWithoutId); } // Since we would have a n² runtime when doing a lookup and updating the access items we decided to @@ -171,6 +173,12 @@ public class WorkbasketDefinitionController { return response; } + private Workbasket removeId(Workbasket importedWb) { + WorkbasketResource wbRes = new WorkbasketResource(importedWb); + wbRes.setWorkbasketId(null); + return workbasketDefinitionAssembler.toModel(wbRes); + } + private String logicalId(WorkbasketSummary workbasket) { return logicalId(workbasket.getKey(), workbasket.getDomain()); }