Changed the move comics REST API [comixed#378]

* Made the endpoint auditable. * Changed the return type to be an ApiResponse. * Deleted the previous response class. * Annotated the endpoint to enforce admin-only access. * Small refactorings to the related task classes.
mcpierce · Aug 25, 2020 · 377f9c5 · 377f9c5
1 parent 56b3ba2
commit 377f9c5
Show file tree

Hide file tree

Showing 8 changed files with 285 additions and 132 deletions.
diff --git a/comixed-rest-api/src/main/java/org/comixedproject/controller/library/LibraryController.java b/comixed-rest-api/src/main/java/org/comixedproject/controller/library/LibraryController.java
@@ -23,10 +23,12 @@
 import java.util.Date;
 import java.util.List;
 import lombok.extern.log4j.Log4j2;
+import org.comixedproject.auditlog.AuditableEndpoint;
 import org.comixedproject.model.archives.ArchiveType;
 import org.comixedproject.model.comic.Comic;
 import org.comixedproject.model.library.ReadingList;
 import org.comixedproject.model.net.*;
+import org.comixedproject.model.net.library.MoveComicsRequest;
 import org.comixedproject.model.user.LastReadDate;
 import org.comixedproject.service.comic.ComicService;
 import org.comixedproject.service.library.LibraryException;
@@ -41,6 +43,7 @@
 import org.springframework.beans.factory.ObjectFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.MediaType;
+import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.bind.annotation.*;
 
 @RestController
@@ -185,24 +188,43 @@ public ClearImageCacheResponse clearImageCache() {
     return new ClearImageCacheResponse(true);
   }
 
+  /**
+   * Consolidates the library, moving all comics under the specified parent directory and using
+   * given naming rules. Will delete comics marked for deletion as well.
+   *
+   * @param request the request body
+   * @return the response body
+   */
   @PostMapping(
       value = "/library/move",
       produces = MediaType.APPLICATION_JSON_VALUE,
       consumes = MediaType.APPLICATION_JSON_VALUE)
-  public MoveComicsResponse moveComics(@RequestBody() MoveComicsRequest request) {
-    String targetDirectory = request.getTargetDirectory();
-    String renamingRule = request.getRenamingRule();
-    Boolean deletePhysicalFiles = request.getDeletePhysicalFiles();
+  @PreAuthorize("hasRole('ADMIN')")
+  @AuditableEndpoint
+  public ApiResponse<Void> moveComics(@RequestBody() MoveComicsRequest request) {
+    final ApiResponse<Void> response = new ApiResponse<>();
 
-    log.info("Moving comics: targetDirectory={}", targetDirectory);
-    log.info("             : renamingRule={}", renamingRule);
+    try {
+      String targetDirectory = request.getTargetDirectory();
+      String renamingRule = request.getRenamingRule();
+      Boolean deletePhysicalFiles = request.getDeletePhysicalFiles();
 
-    final MoveComicsWorkerTask task = this.moveComicsWorkerTaskObjectFactory.getObject();
-    task.setDirectory(targetDirectory);
-    task.setRenamingRule(renamingRule);
+      log.info("Moving comics: targetDirectory={}", targetDirectory);
+      log.info("             : renamingRule={}", renamingRule);
 
-    this.taskManager.runTask(task);
+      final MoveComicsWorkerTask task = this.moveComicsWorkerTaskObjectFactory.getObject();
+      task.setDirectory(targetDirectory);
+      task.setRenamingRule(renamingRule);
+
+      this.taskManager.runTask(task);
+
+      response.setSuccess(true);
+    } catch (Exception error) {
+      response.setSuccess(false);
+      response.setError(error.getMessage());
+      response.setThrowable(error);
+    }
 
-    return new MoveComicsResponse(true);
+    return response;
   }
 }
diff --git a/comixed-rest-api/src/main/java/org/comixedproject/model/net/MoveComicsResponse.java b/comixed-rest-api/src/main/java/org/comixedproject/model/net/MoveComicsResponse.java
diff --git a/...dproject/model/net/MoveComicsRequest.java → .../model/net/library/MoveComicsRequest.java b/...dproject/model/net/MoveComicsRequest.java → .../model/net/library/MoveComicsRequest.java
@@ -16,7 +16,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses>
  */
 
-package org.comixedproject.model.net;
+package org.comixedproject.model.net.library;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 

diff --git a/...d-rest-api/src/test/java/org/comixedproject/controller/library/LibraryControllerTest.java b/...d-rest-api/src/test/java/org/comixedproject/controller/library/LibraryControllerTest.java
@@ -28,6 +28,7 @@
 import org.comixedproject.model.comic.Comic;
 import org.comixedproject.model.library.ReadingList;
 import org.comixedproject.model.net.*;
+import org.comixedproject.model.net.library.MoveComicsRequest;
 import org.comixedproject.model.user.ComiXedUser;
 import org.comixedproject.model.user.LastReadDate;
 import org.comixedproject.service.comic.ComicService;
@@ -289,7 +290,7 @@ public void testMoveLibrary() {
     Mockito.when(moveComicsWorkerTaskObjectFactory.getObject()).thenReturn(moveComicsWorkerTask);
     Mockito.doNothing().when(taskManager).runTask(Mockito.any(WorkerTask.class));
 
-    MoveComicsResponse result =
+    final ApiResponse<Void> result =
         libraryController.moveComics(
             new MoveComicsRequest(
                 TEST_DELETE_PHYSICAL_FILES, TEST_DESTINATION_DIRECTORY, TEST_RENAMING_RULE));

diff --git a/comixed-tasks/src/main/java/org/comixedproject/task/encoders/MoveComicWorkerTaskEncoder.java b/comixed-tasks/src/main/java/org/comixedproject/task/encoders/MoveComicWorkerTaskEncoder.java
@@ -18,6 +18,8 @@
 
 package org.comixedproject.task.encoders;
 
+import lombok.Getter;
+import lombok.Setter;
 import lombok.extern.log4j.Log4j2;
 import org.comixedproject.model.comic.Comic;
 import org.comixedproject.model.tasks.Task;
@@ -45,9 +47,9 @@ public class MoveComicWorkerTaskEncoder extends AbstractWorkerTaskEncoder<MoveCo
   @Autowired private ObjectFactory<MoveComicWorkerTask> moveComicWorkerTaskObjectFactory;
   @Autowired private TaskService taskService;
 
-  private Comic comic;
-  private String directory;
-  private String renamingRule;
+  @Getter @Setter private Comic comic;
+  @Getter @Setter private String directory;
+  @Getter @Setter private String renamingRule;
 
   @Override
   public Task encode() {
@@ -79,31 +81,4 @@ public MoveComicWorkerTask decode(Task task) {
 
     return result;
   }
-
-  /**
-   * Sets the comic to be moved.
-   *
-   * @param comic the comic
-   */
-  public void setComic(Comic comic) {
-    this.comic = comic;
-  }
-
-  /**
-   * Sets the root directory into which the comic is to be moved.
-   *
-   * @param directory the directory
-   */
-  public void setDirectory(String directory) {
-    this.directory = directory;
-  }
-
-  /**
-   * Sets the renaming rule to be applied.
-   *
-   * @param renamingRule the renaming rule
-   */
-  public void setRenamingRule(String renamingRule) {
-    this.renamingRule = renamingRule;
-  }
 }
diff --git a/comixed-tasks/src/main/java/org/comixedproject/task/model/MoveComicWorkerTask.java b/comixed-tasks/src/main/java/org/comixedproject/task/model/MoveComicWorkerTask.java
@@ -20,6 +20,9 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.text.SimpleDateFormat;
+import lombok.Getter;
+import lombok.Setter;
 import lombok.extern.log4j.Log4j2;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.FilenameUtils;
@@ -30,6 +33,7 @@
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Component;
+import org.springframework.util.StringUtils;
 
 /**
  * <code>MoveComicWorkerTask</code> handles moving a single comic file to a new location, creating
@@ -41,19 +45,14 @@
 @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE)
 @Log4j2
 public class MoveComicWorkerTask extends AbstractWorkerTask {
-  @Autowired private ComicService comicService;
-
-  private Comic comic;
-  private String destination;
-  private String renamingRule;
+  static final SimpleDateFormat dateFormat = new SimpleDateFormat("MMM yyyy");
+  private static final String UNKNOWN_VALUE = "Unknown";
 
-  public void setComic(Comic comic) {
-    this.comic = comic;
-  }
+  @Autowired private ComicService comicService;
 
-  public void setDirectory(String destination) {
-    this.destination = destination;
-  }
+  @Getter @Setter private Comic comic;
+  @Getter @Setter private String directory;
+  @Getter @Setter private String renamingRule;
 
   @Override
   public void startTask() throws WorkerTaskException {
@@ -63,25 +62,25 @@ public void startTask() throws WorkerTaskException {
     } catch (IOException error) {
       throw new WorkerTaskException("Could not rename original comic file", error);
     }
-    File destFile = new File(this.getRelativeDestination(), getRelativeComicFilename());
+    File destFile = new File(this.getRelativeDirectory(), getRelativeComicFilename());
     String defaultExtension = FilenameUtils.getExtension(comic.getFilename());
     destFile =
         new File(
             ComicFileUtils.findAvailableFilename(destFile.getAbsolutePath(), 0, defaultExtension));
 
     // if the source and target are the same, then skip the file
     if (destFile.equals(sourceFile)) {
-      log.debug("Source and target are the same: " + destFile.getAbsolutePath());
+      log.debug("Source and target are the same: {}", destFile.getAbsolutePath());
       return;
     }
 
     // create the directory if it doesn't exist
     if (!destFile.getParentFile().exists()) {
-      log.debug("Creating directory: " + destFile.getParentFile().getAbsolutePath());
+      log.debug("Creating directory: {}", destFile.getParentFile().getAbsolutePath());
       destFile.getParentFile().mkdirs();
     }
     try {
-      log.debug("Moving comic: " + this.comic.getFilename() + " -> " + this.destination);
+      log.debug("Moving comic: {} -> {}", this.comic.getFilename(), this.directory);
 
       FileUtils.moveFile(sourceFile, destFile);
 
@@ -103,8 +102,8 @@ private File renameOriginalFile() throws IOException {
     return result;
   }
 
-  private String getRelativeDestination() {
-    StringBuffer result = new StringBuffer(this.destination);
+  private String getRelativeDirectory() {
+    StringBuilder result = new StringBuilder(this.directory);
 
     this.addDirectory(result, this.comic.getPublisher());
     this.addDirectory(result, this.comic.getSeries());
@@ -113,23 +112,51 @@ private String getRelativeDestination() {
     return result.toString();
   }
 
-  private String getRelativeComicFilename() {
-    StringBuffer result = new StringBuffer();
+  String getRelativeComicFilename() {
+    if (StringUtils.isEmpty(this.renamingRule)) {
+      log.debug(
+          "No renaming rules: using original filename: {}",
+          FilenameUtils.getBaseName(this.comic.getFilename()));
+      return FilenameUtils.getBaseName(this.comic.getFilename());
+    }
 
-    result.append(comic.getSeries() != null ? comic.getSeries() : "Unknown");
-    result.append(" v" + (comic.getVolume() != null ? comic.getVolume() : "Unknown"));
-    result.append(" #" + (comic.getIssueNumber() != null ? comic.getIssueNumber() : "0000"));
+    log.debug("Generating relative filename based on renaming rule: {}", this.renamingRule);
+
+    final String publisher =
+        StringUtils.isEmpty(this.comic.getPublisher()) ? UNKNOWN_VALUE : this.comic.getPublisher();
+    final String series =
+        StringUtils.isEmpty(this.comic.getSeries()) ? UNKNOWN_VALUE : this.comic.getSeries();
+    final String volume =
+        StringUtils.isEmpty(this.comic.getVolume()) ? UNKNOWN_VALUE : this.comic.getVolume();
+    final String issueNumber =
+        StringUtils.isEmpty(this.comic.getIssueNumber())
+            ? UNKNOWN_VALUE
+            : this.comic.getIssueNumber();
+    final String coverDate =
+        this.comic.getCoverDate() != null
+            ? dateFormat.format(this.comic.getCoverDate())
+            : "No Cover Date";
+
+    final String result =
+        this.renamingRule
+            .replace("$PUBLISHER", publisher)
+            .replace("$SERIES", series)
+            .replace("$VOLUME", volume)
+            .replace("$ISSUE", issueNumber)
+            .replace("$COVERDATE", coverDate);
+
+    log.debug("Relative comic filename: {}", result);
 
-    return result.toString();
+    return result;
   }
 
-  private void addDirectory(StringBuffer result, String value) {
+  private void addDirectory(StringBuilder result, String value) {
     result.append(File.separator);
 
     if ((value != null) && !value.isEmpty()) {
       result.append(value);
     } else {
-      result.append("Unknown");
+      result.append(UNKNOWN_VALUE);
     }
   }
 
@@ -144,17 +171,8 @@ protected String createDescription() {
         .append(" from=")
         .append(FileUtils.getFile(this.comic.getFilename()).getAbsolutePath())
         .append(" to=")
-        .append(this.destination);
+        .append(this.directory);
 
     return result.toString();
   }
-
-  /**
-   * Sets the renaming rule to use.
-   *
-   * @param renamingRule the renaming rule
-   */
-  public void setRenamingRule(String renamingRule) {
-    this.renamingRule = renamingRule;
-  }
 }
diff --git a/comixed-tasks/src/main/java/org/comixedproject/task/model/MoveComicsWorkerTask.java b/comixed-tasks/src/main/java/org/comixedproject/task/model/MoveComicsWorkerTask.java
@@ -19,6 +19,8 @@
 package org.comixedproject.task.model;
 
 import java.util.List;
+import lombok.Getter;
+import lombok.Setter;
 import lombok.extern.log4j.Log4j2;
 import org.comixedproject.model.comic.Comic;
 import org.comixedproject.service.comic.ComicService;
@@ -48,8 +50,8 @@ public class MoveComicsWorkerTask extends AbstractWorkerTask {
   @Autowired private TaskService taskService;
   @Autowired private ComicService comicService;
 
-  private String directory;
-  private String renamingRule;
+  @Getter @Setter private String directory;
+  @Getter @Setter private String renamingRule;
 
   @Override
   protected String createDescription() {
@@ -77,22 +79,4 @@ public void startTask() throws WorkerTaskException {
       done = comics.size() < (MAX_COMIC_PAGE + 1);
     }
   }
-
-  /**
-   * Sets the directory into which the comics will be moved.
-   *
-   * @param directory the directory
-   */
-  public void setDirectory(String directory) {
-    this.directory = directory;
-  }
-
-  /**
-   * Sets the renaming rule to be used.
-   *
-   * @param renamingRule the renaming rule
-   */
-  public void setRenamingRule(String renamingRule) {
-    this.renamingRule = renamingRule;
-  }
 }