diff --git a/README.md b/README.md index af60ac0..e3796ab 100644 --- a/README.md +++ b/README.md @@ -14,5 +14,7 @@ Java 25+ (Spring Boot 4) ## Database Database uses in-memory DB H2. It will automatically build and export the required tables and data. +The H2 console (`/h2-console`, JDBC URL `jdbc:h2:mem:test`) is available while running with devtools, e.g. `./mvnw spring-boot:run`. It is not served by the packaged jar. + ## Testing There are scripts to test the Rest API in Postman. \ No newline at end of file diff --git a/pom.xml b/pom.xml index a3b8a0d..30b4909 100755 --- a/pom.xml +++ b/pom.xml @@ -27,10 +27,6 @@ org.springframework.boot spring-boot-starter-data-jpa - - org.springframework.boot - spring-boot-starter-data-rest - org.springframework.boot spring-boot-starter-webmvc diff --git a/src/main/java/com/company/resourceapi/config/JpaAuditingConfig.java b/src/main/java/com/company/resourceapi/config/JpaAuditingConfig.java new file mode 100644 index 0000000..aa13020 --- /dev/null +++ b/src/main/java/com/company/resourceapi/config/JpaAuditingConfig.java @@ -0,0 +1,24 @@ +package com.company.resourceapi.config; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.Optional; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.auditing.DateTimeProvider; +import org.springframework.data.jpa.repository.config.EnableJpaAuditing; + +/** + * Populates the {@code @CreatedDate}/{@code @LastModifiedDate} fields of the entities. + * Kept out of the application class so web slice tests don't need a JPA context. + */ +@Configuration +@EnableJpaAuditing(dateTimeProviderRef = "auditingDateTimeProvider") +public class JpaAuditingConfig { + + /** The audit columns are {@code datetime}, so keep responses consistent with what is stored. */ + @Bean + DateTimeProvider auditingDateTimeProvider() { + return () -> Optional.of(Instant.now().truncatedTo(ChronoUnit.SECONDS)); + } +} diff --git a/src/main/java/com/company/resourceapi/controllers/ProjectRestController.java b/src/main/java/com/company/resourceapi/controllers/ProjectRestController.java index e2ffbc9..f6d0cbf 100755 --- a/src/main/java/com/company/resourceapi/controllers/ProjectRestController.java +++ b/src/main/java/com/company/resourceapi/controllers/ProjectRestController.java @@ -3,24 +3,28 @@ import static org.springframework.http.HttpStatus.BAD_REQUEST; import static org.springframework.http.HttpStatus.CONFLICT; import static org.springframework.http.HttpStatus.CREATED; -import static org.springframework.http.HttpStatus.OK; import static org.springframework.http.HttpStatus.NOT_FOUND; import com.company.resourceapi.entities.Project; import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.exceptions.ConflictException; import com.company.resourceapi.exceptions.NotFoundException; import com.company.resourceapi.services.ProjectService; import com.company.resourceapi.services.SdlcSystemService; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Parameter; import io.swagger.v3.oas.annotations.tags.Tag; +import jakarta.validation.ConstraintViolationException; import jakarta.validation.Valid; import java.net.URI; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.hibernate.exception.ConstraintViolationException.ConstraintKind; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PatchMapping; import org.springframework.web.bind.annotation.PathVariable; @@ -28,6 +32,7 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.server.ResponseStatusException; @RestController @RequestMapping(ProjectRestController.ENDPOINT) @@ -50,29 +55,23 @@ public class ProjectRestController { @GetMapping(ENDPOINT_ID) public ResponseEntity getProject( @Parameter(name = PATH_VARIABLE_ID, description = API_PARAM_ID, required = true) - @PathVariable(PATH_VARIABLE_ID) final long projectId) { - try { - return ResponseEntity.status(OK).body(projectService.getProject(projectId)); - } catch (NotFoundException e) { - log.debug(e.getMessage()); - return ResponseEntity.status(NOT_FOUND).build(); - } + @PathVariable(PATH_VARIABLE_ID) final long projectId) throws NotFoundException { + return ResponseEntity.ok(projectService.getProject(projectId)); } @Operation(summary = "Create a Project") @PostMapping public ResponseEntity createProject( @Valid - @RequestBody Project project) { - long systemId = project.getSdlcSystem().getId(); + @RequestBody Project project) throws NotFoundException { + var sdlcSystem = findSdlcSystem(project.getSdlcSystem()); - if (sdlcSystemService.getSdlcSystem(systemId).isEmpty()) { - return ResponseEntity.status(NOT_FOUND).build(); - } - if (projectService.getProject(project.getExternalId(), systemId).isPresent()) { + // Checked up front for a clean 409; the unique key still guards against concurrent requests + if (projectService.getProject(project.getExternalId(), sdlcSystem.getId()).isPresent()) { return ResponseEntity.status(CONFLICT).build(); } + project.setSdlcSystem(sdlcSystem); var createdProject = projectService.createProject(project); return ResponseEntity.status(CREATED) .location(URI.create(LOCATION_HEADER.formatted(createdProject.getId()))) @@ -84,11 +83,8 @@ public ResponseEntity createProject( public ResponseEntity updateProject( @Parameter(name = PATH_VARIABLE_ID, description = API_PARAM_ID, required = true) @PathVariable(PATH_VARIABLE_ID) long projectId, - @Valid @RequestBody PatchedProject patchedProject) { - - try { - var project = projectService.getProject(projectId); - + @RequestBody PatchedProject patchedProject) throws NotFoundException { + var updatedProject = projectService.updateProject(projectId, project -> { if (patchedProject.isNameDirty()) { project.setName(patchedProject.getName()); } @@ -98,28 +94,61 @@ public ResponseEntity updateProject( } if (patchedProject.isSdlcSystemDirty()) { - long systemId = patchedProject.getSdlcSystem().getId(); - if (sdlcSystemService.getSdlcSystem(systemId).isPresent()) { - project.setSdlcSystem(patchedProject.getSdlcSystem()); - } else { - throw new NotFoundException(SdlcSystem.class, systemId); - } + var patchedSystem = patchedProject.getSdlcSystem(); + project.setSdlcSystem(patchedSystem == null ? null : findSdlcSystem(patchedSystem)); } + }); + + return ResponseEntity.ok(updatedProject); + } + + @ExceptionHandler(ConstraintViolationException.class) + ResponseEntity handleConstraintViolation(ConstraintViolationException e) { + log.debug(e.getMessage()); + return ResponseEntity.status(BAD_REQUEST).build(); + } + + @ExceptionHandler(NotFoundException.class) + ResponseEntity handleNotFound(NotFoundException e) { + log.debug(e.getMessage()); + return ResponseEntity.status(NOT_FOUND).build(); + } + + @ExceptionHandler(ConflictException.class) + ResponseEntity handleConflict(ConflictException e) { + log.debug(e.getMessage()); + return ResponseEntity.status(CONFLICT).build(); + } - if ((patchedProject.isExternalIdDirty() || patchedProject.isSdlcSystemDirty()) - && projectService - .getProject(project.getExternalId(), project.getSdlcSystem().getId()) - .isPresent()) { - return ResponseEntity.status(CONFLICT).build(); + /** + * The (sdlcSystem, externalId) pair is unique, so a concurrent request can still insert a + * duplicate after the conflict checks have passed. Any other integrity violation is a + * server-side problem and is left to the default error handling. + */ + @ExceptionHandler(DataIntegrityViolationException.class) + ResponseEntity handleDataIntegrityViolation(DataIntegrityViolationException e) { + if (!isUniqueConstraintViolation(e)) { + throw e; + } + log.debug("Rejected duplicate project: {}", e.getMostSpecificCause().getMessage()); + return ResponseEntity.status(CONFLICT).build(); + } + + static boolean isUniqueConstraintViolation(DataIntegrityViolationException e) { + for (Throwable cause = e.getCause(); cause != null; cause = cause.getCause()) { + if (cause instanceof org.hibernate.exception.ConstraintViolationException violation) { + return violation.getKind() == ConstraintKind.UNIQUE; } + } + return false; + } - return projectService.updateProject(project) - .map(resultantProject -> ResponseEntity.status(OK).body(resultantProject)) - .orElseGet(() -> ResponseEntity.status(BAD_REQUEST).build()); - } catch (NotFoundException e) { - log.debug(e.getMessage()); - return ResponseEntity.status(NOT_FOUND).build(); + /** Resolves the SDLC system a request refers to; an absent id deserializes to 0. */ + private SdlcSystem findSdlcSystem(SdlcSystem reference) throws NotFoundException { + if (reference.getId() == 0) { + throw new ResponseStatusException(BAD_REQUEST, "sdlcSystem.id is required"); } + return sdlcSystemService.getSdlcSystem(reference.getId()); } @Getter @@ -133,10 +162,8 @@ static class PatchedProject { private boolean isSdlcSystemDirty = false; public void setExternalId(String externalId) { - if (externalId != null) { - this.externalId = externalId; - isExternalIdDirty = true; - } + this.externalId = externalId; + isExternalIdDirty = true; } public void setName(String name) { diff --git a/src/main/java/com/company/resourceapi/entities/Project.java b/src/main/java/com/company/resourceapi/entities/Project.java index 74d782f..3ed07c4 100755 --- a/src/main/java/com/company/resourceapi/entities/Project.java +++ b/src/main/java/com/company/resourceapi/entities/Project.java @@ -1,5 +1,8 @@ package com.company.resourceapi.entities; +import static com.fasterxml.jackson.annotation.JsonProperty.Access.READ_ONLY; + +import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.persistence.Column; import jakarta.persistence.Entity; import jakarta.persistence.EntityListeners; @@ -11,9 +14,11 @@ import jakarta.persistence.Table; import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; import java.time.Instant; import lombok.Data; import lombok.NoArgsConstructor; +import org.hibernate.annotations.DynamicUpdate; import org.springframework.data.annotation.CreatedDate; import org.springframework.data.annotation.LastModifiedDate; import org.springframework.data.jpa.domain.support.AuditingEntityListener; @@ -23,17 +28,24 @@ @EntityListeners(AuditingEntityListener.class) @Data @NoArgsConstructor +@DynamicUpdate public class Project { + /** Length of the varchar columns in schema.sql. */ + public static final int MAX_TEXT_LENGTH = 255; + @Id @GeneratedValue(strategy = GenerationType.IDENTITY) + @JsonProperty(access = READ_ONLY) private long id; @Column(name = "external_id", nullable = false) @NotBlank + @Size(max = MAX_TEXT_LENGTH) private String externalId; @Column(name = "name") + @Size(max = MAX_TEXT_LENGTH) private String name; @ManyToOne @@ -43,9 +55,11 @@ public class Project { @Column(name = "created_date", nullable = false) @CreatedDate - private Instant createdDate = Instant.now(); + @JsonProperty(access = READ_ONLY) + private Instant createdDate; @Column(name = "last_modified_date", nullable = false) @LastModifiedDate - private Instant lastModifiedDate = Instant.now(); + @JsonProperty(access = READ_ONLY) + private Instant lastModifiedDate; } diff --git a/src/main/java/com/company/resourceapi/exceptions/ConflictException.java b/src/main/java/com/company/resourceapi/exceptions/ConflictException.java new file mode 100644 index 0000000..b9a139f --- /dev/null +++ b/src/main/java/com/company/resourceapi/exceptions/ConflictException.java @@ -0,0 +1,8 @@ +package com.company.resourceapi.exceptions; + +public class ConflictException extends RuntimeException { + + public ConflictException(String message) { + super(message); + } +} diff --git a/src/main/java/com/company/resourceapi/repositories/ProjectRepository.java b/src/main/java/com/company/resourceapi/repositories/ProjectRepository.java index dc873c1..d1cc2df 100755 --- a/src/main/java/com/company/resourceapi/repositories/ProjectRepository.java +++ b/src/main/java/com/company/resourceapi/repositories/ProjectRepository.java @@ -1,8 +1,11 @@ package com.company.resourceapi.repositories; import com.company.resourceapi.entities.Project; +import jakarta.persistence.QueryHint; import java.util.Optional; +import org.hibernate.jpa.HibernateHints; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.QueryHints; import org.springframework.stereotype.Repository; @Repository @@ -10,4 +13,11 @@ public interface ProjectRepository extends JpaRepository{ Optional findBySdlcSystemIdAndId(long sdlcSystemId, long projectId); Optional findByExternalIdAndSdlcSystemId(String externalId, long sdlcSystemId); + + /** + * Finds another project with the same key. Doesn't flush pending changes first, so it can + * check a modified project before that change is written. + */ + @QueryHints(@QueryHint(name = HibernateHints.HINT_FLUSH_MODE, value = "COMMIT")) + boolean existsByExternalIdAndSdlcSystemIdAndIdNot(String externalId, long sdlcSystemId, long id); } diff --git a/src/main/java/com/company/resourceapi/services/ProjectService.java b/src/main/java/com/company/resourceapi/services/ProjectService.java index 8301db9..c4d86c9 100755 --- a/src/main/java/com/company/resourceapi/services/ProjectService.java +++ b/src/main/java/com/company/resourceapi/services/ProjectService.java @@ -10,5 +10,17 @@ public interface ProjectService { Project createProject(Project project); Optional getProject(String externalId, long sdlcSystemId); Optional getProjectStream(long id); - Optional updateProject(Project updatedProject); + + /** + * Applies {@code update} to the stored project and saves it in one transaction. The change is + * rolled back if the update throws, the result violates the entity's constraints + * ({@link jakarta.validation.ConstraintViolationException}) or another project in the same + * SDLC system already has the external ID ({@link com.company.resourceapi.exceptions.ConflictException}). + */ + Project updateProject(long id, ProjectUpdate update) throws NotFoundException; + + @FunctionalInterface + interface ProjectUpdate { + void applyTo(Project project) throws NotFoundException; + } } diff --git a/src/main/java/com/company/resourceapi/services/SdlcSystemService.java b/src/main/java/com/company/resourceapi/services/SdlcSystemService.java index 4dac0ab..a200c1c 100755 --- a/src/main/java/com/company/resourceapi/services/SdlcSystemService.java +++ b/src/main/java/com/company/resourceapi/services/SdlcSystemService.java @@ -1,8 +1,8 @@ package com.company.resourceapi.services; import com.company.resourceapi.entities.SdlcSystem; -import java.util.Optional; +import com.company.resourceapi.exceptions.NotFoundException; public interface SdlcSystemService { - Optional getSdlcSystem(long id); + SdlcSystem getSdlcSystem(long id) throws NotFoundException; } diff --git a/src/main/java/com/company/resourceapi/services/impl/ProjectServiceImpl.java b/src/main/java/com/company/resourceapi/services/impl/ProjectServiceImpl.java index b6e39bc..8aef1bf 100755 --- a/src/main/java/com/company/resourceapi/services/impl/ProjectServiceImpl.java +++ b/src/main/java/com/company/resourceapi/services/impl/ProjectServiceImpl.java @@ -1,18 +1,22 @@ package com.company.resourceapi.services.impl; import com.company.resourceapi.entities.Project; +import com.company.resourceapi.exceptions.ConflictException; import com.company.resourceapi.exceptions.NotFoundException; import com.company.resourceapi.repositories.ProjectRepository; import com.company.resourceapi.services.ProjectService; -import java.time.Instant; +import jakarta.validation.ConstraintViolationException; +import jakarta.validation.Validator; import java.util.Optional; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; @Service @RequiredArgsConstructor public class ProjectServiceImpl implements ProjectService { private final ProjectRepository projectRepository; + private final Validator validator; public Project getProject(long id) throws NotFoundException { return getProjectStream(id).orElseThrow(() -> new NotFoundException(Project.class, id)); @@ -31,8 +35,27 @@ public Optional getProjectStream(long id) { return projectRepository.findById(id); } - public Optional updateProject(Project updatedProject) { - updatedProject.setLastModifiedDate(Instant.now()); - return Optional.of(projectRepository.save(updatedProject)); + @Transactional(rollbackFor = NotFoundException.class) + public Project updateProject(long id, ProjectUpdate update) throws NotFoundException { + var project = getProject(id); + var externalIdBefore = project.getExternalId(); + var sdlcSystemBefore = project.getSdlcSystem(); + update.applyTo(project); + + var violations = validator.validate(project); + if (!violations.isEmpty()) { + throw new ConstraintViolationException(violations); + } + + boolean keyChanged = !project.getExternalId().equals(externalIdBefore) + || project.getSdlcSystem().getId() != sdlcSystemBefore.getId(); + if (keyChanged && projectRepository.existsByExternalIdAndSdlcSystemIdAndIdNot( + project.getExternalId(), project.getSdlcSystem().getId(), id)) { + throw new ConflictException("Project %s already exists in SdlcSystem %d." + .formatted(project.getExternalId(), project.getSdlcSystem().getId())); + } + + // Managed entity: flushed on commit, only the changed columns (@DynamicUpdate) + return project; } } diff --git a/src/main/java/com/company/resourceapi/services/impl/SdlcSystemServiceImpl.java b/src/main/java/com/company/resourceapi/services/impl/SdlcSystemServiceImpl.java index 8082fd8..a9b60ec 100755 --- a/src/main/java/com/company/resourceapi/services/impl/SdlcSystemServiceImpl.java +++ b/src/main/java/com/company/resourceapi/services/impl/SdlcSystemServiceImpl.java @@ -1,9 +1,9 @@ package com.company.resourceapi.services.impl; import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.exceptions.NotFoundException; import com.company.resourceapi.repositories.SdlcSystemRepository; import com.company.resourceapi.services.SdlcSystemService; -import java.util.Optional; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Service; @@ -13,7 +13,8 @@ public class SdlcSystemServiceImpl implements SdlcSystemService { private final SdlcSystemRepository sdlcSystemRepository; - public Optional getSdlcSystem(long id) { - return sdlcSystemRepository.findById(id); + public SdlcSystem getSdlcSystem(long id) throws NotFoundException { + return sdlcSystemRepository.findById(id) + .orElseThrow(() -> new NotFoundException(SdlcSystem.class, id)); } } diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 51bc616..154ee08 100755 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -1,9 +1,8 @@ -# h2 -spring.h2.console.enabled=true # datasource spring.datasource.url=jdbc:h2:mem:test;MODE=MySQL;DATABASE_TO_LOWER=TRUE spring.sql.init.platform=h2 spring.datasource.driver-class-name=org.h2.Driver # jpa spring.jpa.show-sql=true +spring.jpa.open-in-view=false spring.jpa.hibernate.ddl-auto=none diff --git a/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerConflictTest.java b/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerConflictTest.java new file mode 100644 index 0000000..ff4c17b --- /dev/null +++ b/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerConflictTest.java @@ -0,0 +1,54 @@ +package com.company.resourceapi.controllers; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.services.ProjectService; +import com.company.resourceapi.services.SdlcSystemService; +import java.sql.SQLException; +import java.util.Optional; +import org.hibernate.exception.ConstraintViolationException; +import org.hibernate.exception.ConstraintViolationException.ConstraintKind; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.http.MediaType; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.servlet.MockMvc; + +@WebMvcTest(ProjectRestController.class) +class ProjectRestControllerConflictTest { + + @Autowired + private MockMvc mockMvc; + + @MockitoBean + private ProjectService projectService; + + @MockitoBean + private SdlcSystemService sdlcSystemService; + + @Test + void givenConcurrentDuplicateWhenPostThenReturnStatusConflict() throws Exception { + // given: the conflict check passes, but a concurrent request inserted the same project first + var sdlcSystem = new SdlcSystem(); + sdlcSystem.setId(1); + when(sdlcSystemService.getSdlcSystem(1)).thenReturn(sdlcSystem); + when(projectService.getProject(anyString(), anyLong())).thenReturn(Optional.empty()); + when(projectService.createProject(any())).thenThrow(new DataIntegrityViolationException( + "Unique index or primary key violation", new ConstraintViolationException( + "duplicate", new SQLException(), ConstraintKind.UNIQUE, "system_project"))); + + // when / then + mockMvc.perform(post(ProjectRestController.ENDPOINT) + .content("{\"externalId\": \"DUPLICATE\", \"sdlcSystem\": {\"id\": 1}}") + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isConflict()); + } +} diff --git a/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerTest.java b/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerTest.java index 546c224..e9bcc28 100755 --- a/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerTest.java +++ b/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerTest.java @@ -1,6 +1,9 @@ package com.company.resourceapi.controllers; +import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.matchesPattern; +import static org.hamcrest.Matchers.not; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -9,6 +12,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; @@ -174,4 +179,185 @@ void givenIdAndPatchedProjectWhenPatchedThenReturnStatusConflict() throws Except // then .andExpect(status().isConflict()); } + + @Test + void givenProjectJsonWithIdWhenPostThenCreateNewProjectInsteadOfOverwriting() throws Exception { + final long EXISTING_PROJECT_ID = 1; + final String jsonContent = "{\"id\": " + EXISTING_PROJECT_ID + "," + + String.format(PROJECT_JSON_TEMPLATE, "NEWPROJECT", "New Project", 3).substring(1); + + mockMvc.perform(post(ProjectRestController.ENDPOINT) + .content(jsonContent) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.id", not(equalTo((int) EXISTING_PROJECT_ID)))) + .andExpect(jsonPath(EXTERNALID_JPATH, equalTo("NEWPROJECT"))); + + mockMvc.perform(get(String.format(ENDPOINT_WITH_ID, EXISTING_PROJECT_ID))) + .andExpect(status().isOk()) + .andExpect(jsonPath(EXTERNALID_JPATH, equalTo("SAMPLEPROJECT"))); + } + + @Test + void givenProjectJsonWhenPostThenReturnFullSdlcSystem() throws Exception { + final String jsonContent = String.format(PROJECT_JSON_TEMPLATE, "FULLSYSTEM", "Name", 2); + + mockMvc.perform(post(ProjectRestController.ENDPOINT) + .content(jsonContent) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.sdlcSystem.baseUrl", equalTo("http://bugzilla.company.com"))); + } + + @Test + void givenUnchangedExternalIdWhenPatchedThenReturnStatusOk() throws Exception { + final long PROJECT_ID = 8; + final String UNCHANGED_EXTERNALID = "PROJECTTHREE"; + String requestUri = String.format(ENDPOINT_WITH_ID, PROJECT_ID); + final String jsonContent = String.format("{\"externalId\": \"%s\"}", UNCHANGED_EXTERNALID); + + mockMvc.perform(patch(requestUri) + .contentType(MediaType.APPLICATION_JSON) + .content(jsonContent)) + .andExpect(status().isOk()) + .andExpect(jsonPath(EXTERNALID_JPATH, equalTo(UNCHANGED_EXTERNALID))); + } + + @Test + void givenNullSdlcSystemWhenPatchedThenReturnStatusBadRequest() throws Exception { + final long PROJECT_ID = 1; + String requestUri = String.format(ENDPOINT_WITH_ID, PROJECT_ID); + + mockMvc.perform(patch(requestUri) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"sdlcSystem\": null}")) + .andExpect(status().isBadRequest()); + } + + @Test + void givenNewSdlcSystemWhenPatchedThenReturnFullSdlcSystem() throws Exception { + final long PROJECT_ID = 7; + String requestUri = String.format(ENDPOINT_WITH_ID, PROJECT_ID); + + mockMvc.perform(patch(requestUri) + .contentType(MediaType.APPLICATION_JSON) + .content(String.format(SDLC_SYSTEM_JSON_TEMPLATE2, 2))) + .andExpect(status().isOk()) + .andExpect(jsonPath(SYSTEM_ID_JPATH, equalTo(2))) + .andExpect(jsonPath("$.sdlcSystem.baseUrl", equalTo("http://bugzilla.company.com"))); + } + + @Test + void givenTooLongExternalIdWhenPostThenReturnStatusBadRequest() throws Exception { + final String jsonContent = String.format(PROJECT_JSON_TEMPLATE, "X".repeat(256), "Name", 1); + + mockMvc.perform(post(ProjectRestController.ENDPOINT) + .content(jsonContent) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isBadRequest()); + } + + @Test + void givenTooLongNameWhenPatchedThenReturnStatusBadRequest() throws Exception { + String requestUri = String.format(ENDPOINT_WITH_ID, 1); + + mockMvc.perform(patch(requestUri) + .contentType(MediaType.APPLICATION_JSON) + .content(String.format("{\"name\": \"%s\"}", "N".repeat(256)))) + .andExpect(status().isBadRequest()); + } + + @Test + void givenBlankExternalIdWhenPatchedThenReturnStatusBadRequest() throws Exception { + String requestUri = String.format(ENDPOINT_WITH_ID, 1); + + mockMvc.perform(patch(requestUri) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"externalId\": \" \"}")) + .andExpect(status().isBadRequest()); + } + + @Test + void givenRepositoryPathWhenGetThenReturnStatusNotFound() throws Exception { + // Repositories must only be reachable through the validated controller endpoints + mockMvc.perform(get("/projects")) + .andExpect(status().isNotFound()); + } + + @Test + void givenNullExternalIdWhenPatchedThenReturnStatusBadRequest() throws Exception { + String requestUri = String.format(ENDPOINT_WITH_ID, 1); + + mockMvc.perform(patch(requestUri) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"externalId\": null}")) + .andExpect(status().isBadRequest()); + } + + @ParameterizedTest + @ValueSource(strings = {"\\u0001", "\\u3000", "\\t"}) + void givenUnusualExternalIdThenPostAndPatchApplySameRule(String externalId) throws Exception { + int postStatus = mockMvc.perform(post(ProjectRestController.ENDPOINT) + .content(String.format(PROJECT_JSON_TEMPLATE, externalId, "Name", 2)) + .contentType(MediaType.APPLICATION_JSON)) + .andReturn().getResponse().getStatus(); + int patchStatus = mockMvc.perform(patch(String.format(ENDPOINT_WITH_ID, 3)) + .content(String.format("{\"externalId\": \"%s\"}", externalId)) + .contentType(MediaType.APPLICATION_JSON)) + .andReturn().getResponse().getStatus(); + + assertThat(postStatus == 400).isEqualTo(patchStatus == 400); + } + + @Test + void givenRejectedPatchThenProjectIsUnchanged() throws Exception { + final long PROJECT_ID = 2; + String requestUri = String.format(ENDPOINT_WITH_ID, PROJECT_ID); + + // valid externalId but unknown SDLC system: 404 after the externalId was applied + mockMvc.perform(patch(requestUri) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"externalId\": \"REJECTED\", \"sdlcSystem\": {\"id\": 999}}")) + .andExpect(status().isNotFound()); + + mockMvc.perform(get(requestUri)) + .andExpect(status().isOk()) + .andExpect(jsonPath(EXTERNALID_JPATH, equalTo("PROJECTX"))); + } + + @Test + void givenProjectJsonWhenPostThenTimestampsMatchStoredPrecision() throws Exception { + final String jsonContent = String.format(PROJECT_JSON_TEMPLATE, "SECONDS", "Name", 1); + + mockMvc.perform(post(ProjectRestController.ENDPOINT) + .content(jsonContent) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.createdDate", matchesPattern(".*T\\d{2}:\\d{2}:\\d{2}Z"))) + .andExpect(jsonPath("$.lastModifiedDate", matchesPattern(".*T\\d{2}:\\d{2}:\\d{2}Z"))); + } + + @Test + void givenSdlcSystemWithoutIdWhenPostThenReturnStatusBadRequest() throws Exception { + mockMvc.perform(post(ProjectRestController.ENDPOINT) + .content("{\"externalId\": \"NOSYSTEMID\", \"sdlcSystem\": {}}") + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isBadRequest()); + } + + @Test + void givenSdlcSystemWithoutIdWhenPatchedThenReturnStatusBadRequest() throws Exception { + mockMvc.perform(patch(String.format(ENDPOINT_WITH_ID, 1)) + .content("{\"sdlcSystem\": {}}") + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isBadRequest()); + } + + @Test + void givenNegativeSdlcSystemIdWhenPostThenReturnStatusNotFound() throws Exception { + mockMvc.perform(post(ProjectRestController.ENDPOINT) + .content(String.format(PROJECT_JSON_TEMPLATE, "NEGATIVE", "Name", -5)) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isNotFound()); + } } diff --git a/src/test/java/com/company/resourceapi/controllers/UniqueConstraintViolationTest.java b/src/test/java/com/company/resourceapi/controllers/UniqueConstraintViolationTest.java new file mode 100644 index 0000000..45650cb --- /dev/null +++ b/src/test/java/com/company/resourceapi/controllers/UniqueConstraintViolationTest.java @@ -0,0 +1,83 @@ +package com.company.resourceapi.controllers; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowableOfType; + +import com.company.resourceapi.entities.Project; +import com.company.resourceapi.repositories.ProjectRepository; +import com.company.resourceapi.repositories.SdlcSystemRepository; +import jakarta.persistence.PersistenceException; +import java.sql.SQLException; +import org.hibernate.exception.ConstraintViolationException; +import org.hibernate.exception.ConstraintViolationException.ConstraintKind; +import org.hibernate.exception.DataException; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionTemplate; + +@SpringBootTest +@Transactional +class UniqueConstraintViolationTest { + + @Autowired + private ProjectRepository projectRepository; + + @Autowired + private SdlcSystemRepository sdlcSystemRepository; + + @Autowired + private PlatformTransactionManager transactionManager; + + @Test + void givenDuplicateProjectWhenSavedThenRecognisedAsUniqueConstraintViolation() { + // given: (SAMPLEPROJECT, 1) already exists in data.sql + var duplicate = new Project(); + duplicate.setExternalId("SAMPLEPROJECT"); + duplicate.setSdlcSystem(sdlcSystemRepository.findById(1L).orElseThrow()); + + // when + var e = catchThrowableOfType(DataIntegrityViolationException.class, + () -> projectRepository.saveAndFlush(duplicate)); + + // then + assertThat(ProjectRestController.isUniqueConstraintViolation(e)).isTrue(); + } + + @Test + void givenOtherIntegrityViolationThenNotRecognisedAsUniqueConstraintViolation() { + var e = new DataIntegrityViolationException("value too long", + new DataException("value too long", new SQLException())); + + assertThat(ProjectRestController.isUniqueConstraintViolation(e)).isFalse(); + } + + @Test + void givenWrappedUniqueViolationThenRecognisedAsUniqueConstraintViolation() { + // e.g. a commit-time failure wrapped by the persistence provider + var e = new DataIntegrityViolationException("could not commit", + new PersistenceException(new ConstraintViolationException( + "duplicate", new SQLException(), ConstraintKind.UNIQUE, "system_project"))); + + assertThat(ProjectRestController.isUniqueConstraintViolation(e)).isTrue(); + } + + @Test + void givenDuplicateDetectedAtCommitThenRecognisedAsUniqueConstraintViolation() { + // given: a separate transaction whose change only hits the unique key when it commits + var concurrentRequest = new TransactionTemplate(transactionManager); + concurrentRequest.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); + + // when: project 2 (PROJECTX, 2) is renamed to SAMPLEPROJECT, which project 3 already has in system 2 + var e = catchThrowableOfType(DataIntegrityViolationException.class, + () -> concurrentRequest.executeWithoutResult(status -> + projectRepository.findById(2L).orElseThrow().setExternalId("SAMPLEPROJECT"))); + + // then + assertThat(ProjectRestController.isUniqueConstraintViolation(e)).isTrue(); + } +} diff --git a/src/test/java/com/company/resourceapi/repositories/ProjectRepositoryTest.java b/src/test/java/com/company/resourceapi/repositories/ProjectRepositoryTest.java new file mode 100644 index 0000000..53250f8 --- /dev/null +++ b/src/test/java/com/company/resourceapi/repositories/ProjectRepositoryTest.java @@ -0,0 +1,31 @@ +package com.company.resourceapi.repositories; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Instant; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.transaction.annotation.Transactional; + +@SpringBootTest +@Transactional +class ProjectRepositoryTest { + + @Autowired + private ProjectRepository projectRepository; + + @Test + void givenChangedProjectWhenSavedThenLastModifiedDateIsUpdated() { + // given + var project = projectRepository.findById(2L).orElseThrow(); + project.setLastModifiedDate(Instant.EPOCH); + project.setName("Renamed"); + + // when + var saved = projectRepository.saveAndFlush(project); + + // then + assertThat(saved.getLastModifiedDate()).isAfter(Instant.EPOCH); + } +} diff --git a/src/test/java/com/company/resourceapi/repositories/SdlcSystemRepositoryTest.java b/src/test/java/com/company/resourceapi/repositories/SdlcSystemRepositoryTest.java new file mode 100644 index 0000000..9d57894 --- /dev/null +++ b/src/test/java/com/company/resourceapi/repositories/SdlcSystemRepositoryTest.java @@ -0,0 +1,31 @@ +package com.company.resourceapi.repositories; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.company.resourceapi.entities.SdlcSystem; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.transaction.annotation.Transactional; + +@SpringBootTest +@Transactional +class SdlcSystemRepositoryTest { + + @Autowired + private SdlcSystemRepository sdlcSystemRepository; + + @Test + void givenNewSdlcSystemWhenSavedThenAuditDatesArePopulated() { + // given + var sdlcSystem = new SdlcSystem(); + sdlcSystem.setBaseUrl("http://youtrack.company.com"); + + // when + var saved = sdlcSystemRepository.saveAndFlush(sdlcSystem); + + // then + assertThat(saved.getCreatedDate()).isNotNull(); + assertThat(saved.getLastModifiedDate()).isNotNull(); + } +} diff --git a/src/test/java/com/company/resourceapi/services/impl/ProjectServiceImplConcurrencyTest.java b/src/test/java/com/company/resourceapi/services/impl/ProjectServiceImplConcurrencyTest.java new file mode 100644 index 0000000..7b597f8 --- /dev/null +++ b/src/test/java/com/company/resourceapi/services/impl/ProjectServiceImplConcurrencyTest.java @@ -0,0 +1,62 @@ +package com.company.resourceapi.services.impl; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.company.resourceapi.entities.Project; +import com.company.resourceapi.repositories.ProjectRepository; +import com.company.resourceapi.repositories.SdlcSystemRepository; +import com.company.resourceapi.services.ProjectService; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.support.TransactionTemplate; + +@SpringBootTest +class ProjectServiceImplConcurrencyTest { + + @Autowired + private ProjectService projectService; + + @Autowired + private ProjectRepository projectRepository; + + @Autowired + private SdlcSystemRepository sdlcSystemRepository; + + @Autowired + private PlatformTransactionManager transactionManager; + + @Test + void givenConcurrentUpdatesOfDifferentFieldsThenBothChangesAreKept() throws Exception { + // given + var project = new Project(); + project.setExternalId("CONCURRENT"); + project.setName("Original"); + project.setSdlcSystem(sdlcSystemRepository.findById(3L).orElseThrow()); + long projectId = projectRepository.save(project).getId(); + + var otherRequest = new TransactionTemplate(transactionManager); + otherRequest.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); + + try { + // when: another request renames the project after this update has loaded it + projectService.updateProject(projectId, loaded -> { + otherRequest.executeWithoutResult(status -> { + var concurrent = projectRepository.findById(projectId).orElseThrow(); + concurrent.setName("Renamed concurrently"); + }); + loaded.setExternalId("CONCURRENT-EDITED"); + }); + + // then + var stored = projectRepository.findById(projectId).orElseThrow(); + assertThat(stored.getExternalId()).isEqualTo("CONCURRENT-EDITED"); + assertThat(stored.getName()).isEqualTo("Renamed concurrently"); + } finally { + // the context (and its in-memory database) is shared with other test classes + projectRepository.deleteById(projectId); + } + } +} diff --git a/src/test/java/com/company/resourceapi/services/impl/ProjectServiceImplTest.java b/src/test/java/com/company/resourceapi/services/impl/ProjectServiceImplTest.java index 863a9c3..85716a7 100755 --- a/src/test/java/com/company/resourceapi/services/impl/ProjectServiceImplTest.java +++ b/src/test/java/com/company/resourceapi/services/impl/ProjectServiceImplTest.java @@ -1,12 +1,20 @@ package com.company.resourceapi.services.impl; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.company.resourceapi.entities.Project; +import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.exceptions.ConflictException; import com.company.resourceapi.repositories.ProjectRepository; +import jakarta.validation.ConstraintViolationException; +import jakarta.validation.Validation; +import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -16,28 +24,55 @@ @ExtendWith(MockitoExtension.class) public class ProjectServiceImplTest { + private static final long PROJECT_ID = 1; + @Mock private ProjectRepository projectRepository; private ProjectServiceImpl projectService; + private Project storedProject; + @BeforeEach void setup() { - projectService = new ProjectServiceImpl(projectRepository); + projectService = new ProjectServiceImpl(projectRepository, + Validation.buildDefaultValidatorFactory().getValidator()); + + var sdlcSystem = new SdlcSystem(); + sdlcSystem.setId(1); + storedProject = new Project(); + storedProject.setId(PROJECT_ID); + storedProject.setExternalId("SAMPLEPROJECT"); + storedProject.setSdlcSystem(sdlcSystem); + when(projectRepository.findById(PROJECT_ID)).thenReturn(Optional.of(storedProject)); } @Test - void givenProjectWhenUpdateProjectThenSetModifiedLastDate() { - // given - Project updatedProject = mock(Project.class); - Project resultantProject = mock(Project.class); - when(projectRepository.save(updatedProject)).thenReturn(resultantProject); - + void givenNameOnlyUpdateWhenUpdateProjectThenSkipDuplicateCheck() throws Exception { // when - projectService.updateProject(updatedProject); + Project result = projectService.updateProject(PROJECT_ID, project -> project.setName("Renamed")); // then - verify(updatedProject).setLastModifiedDate(any()); - verify(projectRepository).save(updatedProject); + assertThat(result).isSameAs(storedProject); + assertThat(result.getName()).isEqualTo("Renamed"); + verify(projectRepository, never()) + .existsByExternalIdAndSdlcSystemIdAndIdNot(anyString(), anyLong(), anyLong()); + } + + @Test + void givenInvalidUpdateWhenUpdateProjectThenThrowConstraintViolation() { + assertThatThrownBy(() -> projectService.updateProject(PROJECT_ID, project -> project.setExternalId(" "))) + .isInstanceOf(ConstraintViolationException.class); + } + + @Test + void givenDuplicateExternalIdWhenUpdateProjectThenThrowConflict() { + // given + when(projectRepository.existsByExternalIdAndSdlcSystemIdAndIdNot("PROJECTX", 1, PROJECT_ID)) + .thenReturn(true); + + // when / then + assertThatThrownBy(() -> projectService.updateProject(PROJECT_ID, project -> project.setExternalId("PROJECTX"))) + .isInstanceOf(ConflictException.class); } }