feat: implement idempotent soft delete for Books (#4)

* Add TDD for success and custom ResourceNotFound exception

* Add custom ResourceNotFoundException class

* Add deleteBookById() with idempotency; Update tests to use findById()

* Fix wrong path for jacoco coverage report

* build(jacoco): print clickable HTML report link to console

* Improve not-found error; tidy comments; update Javadoc

Author: codesungrape

README.md

@@ -74,7 +74,7 @@ JaCoCo generates unit test and integration test coverage reports in XML and HTML
 
 ### Coverage report location
 
-`build/reports/jacoco/test/jacocoTestReport.html`
+`build/reports/jacoco/test/html/index.html`
 
 ----------
 

build.gradle

@@ -72,6 +72,16 @@ jacocoTestReport {
         html.required = true // For human-readable reports
         csv.required = false
     }
+
+    doLast {
+      // We extract the file path into a variable
+      def reportFile = reports.html.entryPoint
+
+      println "\n========================================================="
+      // Wrap the path in quotes to handle the space in "BookAPI 2" better
+      println "JaCoCo Report: file://${reportFile}"
+      println "=========================================================\n"
+    }
 }
 
 // ENFORCE 100% coverage with branch coverage

src/main/java/com/codesungrape/hmcts/bookapi/exception/ResourceNotFoundException.java

@@ -0,0 +1,13 @@
+package com.codesungrape.hmcts.bookapi.exception;
+
+/**
+ * Custom exception to signal that a requested resource (Book, Reservation, etc.)
+ * could not be found, typically mapping to HTTP 404.
+ */
+
+public class ResourceNotFoundException extends RuntimeException {
+
+    public ResourceNotFoundException(String message) {
+        super(message);
+    }
+}

src/main/java/com/codesungrape/hmcts/bookapi/service/BookService.java

@@ -2,18 +2,22 @@
 
 import com.codesungrape.hmcts.bookapi.dto.BookRequest;
 import com.codesungrape.hmcts.bookapi.entity.Book;
+import com.codesungrape.hmcts.bookapi.exception.ResourceNotFoundException;
 import com.codesungrape.hmcts.bookapi.repository.BookRepository;
+import jakarta.transaction.Transactional;
 import lombok.RequiredArgsConstructor;
 import org.springframework.stereotype.Service;
 
+import java.util.UUID;
+
 /**
  * Service layer responsible for all business logic related to the Book resource.
  */
 @Service
 @RequiredArgsConstructor // Lombok creates constructor for dependency injection
 public class BookService {
 
-    // Create a field to store the repo
+    // Create a field to store the repo in this scope to be accessed and used/reused by methods below
     private final BookRepository bookRepository;
 
     /**
@@ -24,13 +28,14 @@ public class BookService {
      * @throws NullPointerException     if request is null
      * @throws IllegalArgumentException if title is null or blank
      */
+    @Transactional // Required: This method modifies data
     public Book createBook(BookRequest request) {
         // Validation check for business rules (e.g., uniqueness, if required)
         if (request == null) {
             throw new NullPointerException("BookRequest cannot be null");
         }
 
-        // TODO: Leaving this here for now as i haven't implemented the Controller Layer yet
+        // TODO: Remove manual validation once Controller validation (@Valid) is implemented.
         // The service layer is duplicating validation that already exists in the
         // BookRequest DTO with @notblank annotations. Since the DTO has validation
         // constraints, this manual check is redundant when Spring's validation
@@ -54,4 +59,26 @@ public Book createBook(BookRequest request) {
 
         return savedBook;
     }
+
+    /**
+     * Performs a soft delete on a Book entity by marking it as deleted.
+     * This operation is idempotent - repeated calls will not trigger additional database writes.
+     *
+     * @param bookId The UUID of the book to soft delete
+     * @throws ResourceNotFoundException if no book exists with the given ID
+     */
+    @Transactional // Required: This method modifies data
+    public void deleteBookById(UUID bookId) {
+
+        Book book = bookRepository.findById(bookId)
+            .orElseThrow(() -> new ResourceNotFoundException(String.format(
+                "Book not found with id: %s", bookId
+            )));
+
+        // Idempotent way to mark soft-delete and save
+        if (!book.isDeleted()) {
+            book.setDeleted(true);
+            bookRepository.save(book);
+        }
+    }
 }

src/test/java/com/codesungrape/hmcts/bookapi/BookServiceTest.java

@@ -2,6 +2,7 @@
 
 import com.codesungrape.hmcts.bookapi.dto.BookRequest;
 import com.codesungrape.hmcts.bookapi.entity.Book;
+import com.codesungrape.hmcts.bookapi.exception.ResourceNotFoundException;
 import com.codesungrape.hmcts.bookapi.repository.BookRepository;
 import com.codesungrape.hmcts.bookapi.service.BookService;
 import org.junit.jupiter.api.BeforeEach;
@@ -14,12 +15,14 @@
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
+import java.util.Optional;
 import java.util.UUID;
 import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.when;
@@ -53,6 +56,10 @@ class BookServiceTest {
     private Book persistedBook;
     private UUID testId;
 
+    // --------------------------------------
+    // Parameter Sources
+    // --------------------------------------
+
     // Provide test data, static method: can be called without creating an object.
     private static Stream<Arguments> provideLongFieldTestCases() {
         UUID testId = UUID.randomUUID();
@@ -84,8 +91,9 @@ private static Stream<Arguments> provideLongFieldTestCases() {
         );
     }
 
-    // --------- TESTS ------------
-
+    // --------------------------------------
+    // Tests
+    // --------------------------------------
     @BeforeEach
     void setUp() {
         testId = UUID.randomUUID();
@@ -109,6 +117,9 @@ void setUp() {
                 .build();
     }
 
+    // --------------------------------------
+    // Tests: createBook
+    // --------------------------------------
     @Test
     void testCreateBook_Success() {
 
@@ -129,13 +140,6 @@ void testCreateBook_Success() {
         verify(testBookRepository, times(1)).save(any(Book.class));
     }
 
-    // CoPilot feedback:
-    // This test will fail because BookRequest uses @value from Lombok with @notblank validation.
-    // The @notblank constraint on the title field means that creating a BookRequest with a null
-    // title should trigger validation failure at the DTO level, not allow the object to be
-    // created. Either the test expectations are incorrect, or the DTO validation is not being
-    // applied. The same issue affects tests on lines 105-116, 119-127, and 130-138.
-
     @Test
     void testCreateBook_NullRequest_ThrowsException() {
         // Act & Assert
@@ -192,7 +196,6 @@ void testCreateBook_BlankTitle_ThrowsException() {
         );
     }
 
-    // --------- Repository failures
     @Test
     void testCreateBook_RepositoryFailure_ThrowsException() {
         // Arrange
@@ -213,7 +216,10 @@ void testCreateBook_RepositoryFailure_ThrowsException() {
     @ParameterizedTest(name = "{0}") // Display the test name
     @MethodSource("provideLongFieldTestCases")
     void testCreateBook_VeryLongFields_Success(
-        String testName, BookRequest request, Book expectedBook) {
+        String testName,
+        BookRequest request,
+        Book expectedBook
+    ) {
 
         // Arrange
         when(testBookRepository.save(any(Book.class))).thenReturn(expectedBook);
@@ -260,4 +266,66 @@ void testCreateBook_SpecialCharactersInTitle_Success() {
         // Did the service perform the correct action on its dependency?
         verify(testBookRepository, times(1)).save(any(Book.class));
     }
+
+    // --------------------------------------------------------------------------------------------
+    // Tests: deleteBookById(UUID)
+    // -------------------------------------------------------------------------------------------
+
+    @Test
+    void testDelete_Book_ShouldThrowException_WhenIdNotFound() {
+
+        // Arrange: As goal is to test what happens when the resource doesn't exist,
+        // we intentionally simulate DB returning NO result
+        when(testBookRepository.findById(testId)).thenReturn(Optional.empty());
+
+        // ACT and ASSERT: throw ResourceNotFoundException when calling the delete method.
+        assertThrows(
+            // custom exception to reflect business rules vs technical problem
+            ResourceNotFoundException.class,
+            () -> testBookService.deleteBookById(testId)
+        );
+
+        // Assert: ensure the save method was NEVER called.
+        // proves delete business logic halts immediately when the resource isn't found.
+        verify(testBookRepository, never()).save(any(Book.class));
+    }
+
+    @Test
+    void testDeleteBookById_Success() {
+
+        // Arrange:
+        persistedBook.setDeleted(false); // ensure starting state
+
+        when(testBookRepository.findById(testId))
+            .thenReturn(Optional.of(persistedBook));
+
+        when(testBookRepository.save(any(Book.class)))
+            .thenReturn(persistedBook);
+
+        // Act: call the service method we are testing
+        testBookService.deleteBookById(testId);
+
+        // Assert: the entity was marked deleted
+        assertTrue(persistedBook.isDeleted());
+
+        // Assert: repository methods were called correctly
+        verify(testBookRepository, times(1)).findById(testId);
+        verify(testBookRepository, times(1)).save(persistedBook);
+    }
+
+    @Test
+    void testDeleteBookById_ShouldDoNothing_WhenAlreadyDeleted() {
+
+        // Arrange:
+        persistedBook.setDeleted(true); // ensure starting state
+        when(testBookRepository.findById(testId))
+            .thenReturn(Optional.of(persistedBook));
+
+        // Act: call the service method we are testing
+        testBookService.deleteBookById(testId);
+
+        // Assert
+        // Verify save was NEVER called (the if condition was false, so the if block was skipped)
+        verify(testBookRepository, never()).save(any(Book.class));
+    }
 }