Merge pull request #481 from beatngu13/feature/refactor

Refactor
beatngu13 · Jan 4, 2024 · 6e2ceec · 6e2ceec
2 parents 3e735c1 + 41742fd
commit 6e2ceec
Show file tree

Hide file tree

Showing 11 changed files with 114 additions and 86 deletions.
diff --git a/scripts/start-hotfix.sh b/scripts/start-hotfix.sh
@@ -2,4 +2,4 @@
 
 set -x
 
-./mvnw -B gitflow:hotfix-start
+./mvnw -B gitflow:hotfix-start -DhotfixBranch="$1"
diff --git a/src/main/java/com/github/beatngu13/pdfzoomwizard/ui/MainViewController.java b/src/main/java/com/github/beatngu13/pdfzoomwizard/ui/MainViewController.java
@@ -21,6 +21,8 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.File;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 
 /**
  * Provides a JavaFX-based Wizard UI.
@@ -31,6 +33,10 @@ public class MainViewController {
 
 	private static final Logger logger = LoggerFactory.getLogger(MainViewController.class);
 
+	/**
+	 * <code>ExecutorService</code> for running {@link Wizard} tasks.
+	 */
+	private final ExecutorService executorService = Executors.newSingleThreadExecutor();
 	/**
 	 * Provides the last directory for {@link #directoryChooser} and
 	 * {@link #fileChooser}.
@@ -204,11 +210,12 @@ private boolean logAndShow(String msg) {
 	 * @return Confirmation message for directory/file to be overwritten/copied.
 	 */
 	private String getConfirmationMessage() {
-		var prefix = multipleMode
-				? "All files in '" + root.getAbsolutePath() + "' and its enclosing subdirectories will be "
-				: "File '" + root.getAbsolutePath() + "' will be ";
+		var prefix = (multipleMode
+				? "All files in '%s' and its enclosing subdirectories will be "
+				: "File '%s' will be ")
+				.formatted(root.getAbsolutePath());
 		var infix = copyCheckBox.isSelected() ? "copied." : "overwritten.";
-		var suffix = "\n\nAre you sure to proceed?";
+		var suffix = System.lineSeparator().repeat(2) + "Are you sure to proceed?";
 		return prefix + infix + suffix;
 	}
 
@@ -219,11 +226,9 @@ private String getConfirmationMessage() {
 	private void run() {
 		var filenameInfix = copyCheckBox.isSelected() ? copyTextField.getText() : null;
 		var wizard = new Wizard(root, filenameInfix, zoomChoiceBox.getValue());
-		var thread = new Thread(wizard);
 		// Can't be bound because infoText is also set within here.
 		wizard.messageProperty().addListener((observable, oldValue, newValue) -> infoText.setText(newValue));
-		thread.setDaemon(true);
-		thread.start();
+		executorService.submit(wizard);
 	}
 
 }
diff --git a/src/test/java/com/github/beatngu13/pdfzoomwizard/TestUtil.java b/src/test/java/com/github/beatngu13/pdfzoomwizard/TestUtil.java
diff --git a/src/test/java/com/github/beatngu13/pdfzoomwizard/core/WizardIT.java b/src/test/java/com/github/beatngu13/pdfzoomwizard/core/WizardIT.java
@@ -1,10 +1,6 @@
 package com.github.beatngu13.pdfzoomwizard.core;
 
-import com.github.beatngu13.pdfzoomwizard.TestUtil;
-import com.itextpdf.kernel.pdf.PdfObject;
 import org.approvaltests.Approvals;
-import org.approvaltests.namer.NamedEnvironment;
-import org.approvaltests.namer.NamerFactory;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
@@ -17,7 +13,6 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
-import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
@@ -40,32 +35,29 @@ void setUp(@TempDir Path temp) throws Exception {
 		}
 
 		@ParameterizedTest
-		@EnumSource(Zoom.class)
+		@EnumSource
 		void zoom_should_be_applied_properly(Zoom zoom) {
-			var zoomName = TestUtil.toStringNormalized(zoom);
-			try (NamedEnvironment env = NamerFactory.withParameters(zoomName)) {
-				new Wizard(pdf, null, zoom).call();
-				List<PdfObject> pdfObjects = TestUtil.getAllPdfObjects(pdf);
-				Approvals.verify(pdfObjects);
-			}
+			var zoomName = zoom.name().toLowerCase();
+			new Wizard(pdf, null, zoom).call();
+			var bookmarks = WizardITUtil.getBookmarks(pdf);
+			Approvals.verifyAll(bookmarks.toArray(), Object::toString, Approvals.NAMES.withParameters(zoomName));
 		}
 
 		@Test
 		void should_overwrite_pdf_if_infix_is_null() {
 			new Wizard(pdf, null, Zoom.INHERIT_ZOOM).call();
-			assertThat(pdf.getParentFile().listFiles())
-					.extracting(File::getName)
-					.containsExactly(pdfName);
+			assertThat(pdf.getParentFile())
+					.isDirectoryContaining(file -> file.getName().equals(pdfName));
 		}
 
 		@Test
 		void should_copy_pdf_if_infix_is_not_null() {
 			var pdfInfix = "-infix";
 			new Wizard(pdf, pdfInfix, Zoom.INHERIT_ZOOM).call();
 			var pdfCopyName = pdfPrefix + pdfInfix + pdfSuffix;
-			assertThat(pdf.getParentFile().listFiles())
-					.extracting(File::getName)
-					.containsExactlyInAnyOrder(pdfName, pdfCopyName);
+			assertThat(pdf.getParentFile())
+					.isDirectoryContaining(file -> file.getName().equals(pdfName))
+					.isDirectoryContaining(file -> file.getName().equals(pdfCopyName));
 		}
 
 	}

diff --git a/src/test/java/com/github/beatngu13/pdfzoomwizard/core/WizardITUtil.java b/src/test/java/com/github/beatngu13/pdfzoomwizard/core/WizardITUtil.java
@@ -0,0 +1,59 @@
+package com.github.beatngu13.pdfzoomwizard.core;
+
+import com.itextpdf.kernel.pdf.PdfDocument;
+import com.itextpdf.kernel.pdf.PdfOutline;
+import com.itextpdf.kernel.pdf.PdfReader;
+import com.itextpdf.kernel.pdf.navigation.PdfDestination;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.List;
+import java.util.stream.Stream;
+
+final class WizardITUtil {
+
+	record Bookmark(String title, String data) {
+	}
+
+	private WizardITUtil() {
+	}
+
+	static List<Bookmark> getBookmarks(File pdf) {
+		return streamOutlines(pdf)
+				.map(WizardITUtil::toBookmark)
+				.toList();
+	}
+
+	private static Stream<PdfOutline> streamOutlines(File pdf) {
+		try (PdfDocument doc = new PdfDocument(new PdfReader(pdf))) {
+			PdfOutline outlines = doc.getOutlines(true);
+			if (outlines == null) {
+				return Stream.empty();
+			}
+			return outlines
+					.getAllChildren()
+					.stream()
+					.flatMap(WizardITUtil::streamOutlines);
+		} catch (IOException e) {
+			throw new UncheckedIOException(e);
+		}
+	}
+
+	private static Stream<PdfOutline> streamOutlines(PdfOutline outline) {
+		Stream<PdfOutline> allChildren = outline.getAllChildren()
+				.stream()
+				.flatMap(WizardITUtil::streamOutlines);
+		return Stream.concat(Stream.of(outline), allChildren);
+	}
+
+	private static Bookmark toBookmark(PdfOutline outline) {
+		String title = outline.getTitle();
+		PdfDestination destination = outline.getDestination();
+		String data = destination == null
+				? "No destination"
+				: destination.getPdfObject().toString();
+		return new Bookmark(title, data);
+	}
+
+}
diff --git a/src/test/java/com/github/beatngu13/pdfzoomwizard/core/ZoomTest.java b/src/test/java/com/github/beatngu13/pdfzoomwizard/core/ZoomTest.java
@@ -12,12 +12,12 @@
 class ZoomTest {
 
 	@ParameterizedTest
-	@MethodSource("args")
+	@MethodSource
 	void to_string_should_be_formatted(Zoom zoom, String formatted) {
 		assertThat(zoom).hasToString(formatted);
 	}
 
-	static Stream<Arguments> args() {
+	static Stream<Arguments> to_string_should_be_formatted() {
 		return Stream.of(
 				arguments(Zoom.ACTUAL_SIZE, "Actual size"),
 				arguments(Zoom.FIT_PAGE, "Fit page"),

diff --git a/...zoomwizard/core/WizardIT.WithPdf.zoom_should_be_applied_properly.actual_size.approved.txt b/...zoomwizard/core/WizardIT.WithPdf.zoom_should_be_applied_properly.actual_size.approved.txt
@@ -1 +1,6 @@
-[[42 0 R /XYZ 742 null 1 ], [1 0 R /XYZ 731 null 1 ], [3 0 R /XYZ 731 null 1 ], [5 0 R /XYZ 731 null 1 ], [7 0 R /XYZ 731 null 1 ], [11 0 R /XYZ 731 null 1 ]]
+Bookmark[title=Working with Bookmarks , data=[42 0 R /XYZ 742 null 1 ]]
+Bookmark[title=Creating New Bookmarks  , data=[1 0 R /XYZ 731 null 1 ]]
+Bookmark[title=Creating a Bookmark Hierarchy  , data=[3 0 R /XYZ 731 null 1 ]]
+Bookmark[title=Editing Bookmarks  , data=[5 0 R /XYZ 731 null 1 ]]
+Bookmark[title=Deleting Bookmarks  , data=[7 0 R /XYZ 731 null 1 ]]
+Bookmark[title=Setting Documents Properties for Bookmarks , data=[11 0 R /XYZ 731 null 1 ]]
diff --git a/...pdfzoomwizard/core/WizardIT.WithPdf.zoom_should_be_applied_properly.fit_page.approved.txt b/...pdfzoomwizard/core/WizardIT.WithPdf.zoom_should_be_applied_properly.fit_page.approved.txt
@@ -1 +1,6 @@
-[[42 0 R /Fit ], [1 0 R /Fit ], [3 0 R /Fit ], [5 0 R /Fit ], [7 0 R /Fit ], [11 0 R /Fit ]]
+Bookmark[title=Working with Bookmarks , data=[42 0 R /Fit ]]
+Bookmark[title=Creating New Bookmarks  , data=[1 0 R /Fit ]]
+Bookmark[title=Creating a Bookmark Hierarchy  , data=[3 0 R /Fit ]]
+Bookmark[title=Editing Bookmarks  , data=[5 0 R /Fit ]]
+Bookmark[title=Deleting Bookmarks  , data=[7 0 R /Fit ]]
+Bookmark[title=Setting Documents Properties for Bookmarks , data=[11 0 R /Fit ]]
diff --git a/...zoomwizard/core/WizardIT.WithPdf.zoom_should_be_applied_properly.fit_visible.approved.txt b/...zoomwizard/core/WizardIT.WithPdf.zoom_should_be_applied_properly.fit_visible.approved.txt
@@ -1 +1,6 @@
-[[42 0 R /FitBH 742 ], [1 0 R /FitBH 731 ], [3 0 R /FitBH 731 ], [5 0 R /FitBH 731 ], [7 0 R /FitBH 731 ], [11 0 R /FitBH 731 ]]
+Bookmark[title=Working with Bookmarks , data=[42 0 R /FitBH 742 ]]
+Bookmark[title=Creating New Bookmarks  , data=[1 0 R /FitBH 731 ]]
+Bookmark[title=Creating a Bookmark Hierarchy  , data=[3 0 R /FitBH 731 ]]
+Bookmark[title=Editing Bookmarks  , data=[5 0 R /FitBH 731 ]]
+Bookmark[title=Deleting Bookmarks  , data=[7 0 R /FitBH 731 ]]
+Bookmark[title=Setting Documents Properties for Bookmarks , data=[11 0 R /FitBH 731 ]]
diff --git a/...dfzoomwizard/core/WizardIT.WithPdf.zoom_should_be_applied_properly.fit_width.approved.txt b/...dfzoomwizard/core/WizardIT.WithPdf.zoom_should_be_applied_properly.fit_width.approved.txt
@@ -1 +1,6 @@
-[[42 0 R /FitH 742 ], [1 0 R /FitH 731 ], [3 0 R /FitH 731 ], [5 0 R /FitH 731 ], [7 0 R /FitH 731 ], [11 0 R /FitH 731 ]]
+Bookmark[title=Working with Bookmarks , data=[42 0 R /FitH 742 ]]
+Bookmark[title=Creating New Bookmarks  , data=[1 0 R /FitH 731 ]]
+Bookmark[title=Creating a Bookmark Hierarchy  , data=[3 0 R /FitH 731 ]]
+Bookmark[title=Editing Bookmarks  , data=[5 0 R /FitH 731 ]]
+Bookmark[title=Deleting Bookmarks  , data=[7 0 R /FitH 731 ]]
+Bookmark[title=Setting Documents Properties for Bookmarks , data=[11 0 R /FitH 731 ]]
diff --git a/...oomwizard/core/WizardIT.WithPdf.zoom_should_be_applied_properly.inherit_zoom.approved.txt b/...oomwizard/core/WizardIT.WithPdf.zoom_should_be_applied_properly.inherit_zoom.approved.txt
@@ -1 +1,6 @@
-[[42 0 R /XYZ 742 null null ], [1 0 R /XYZ 731 null null ], [3 0 R /XYZ 731 null null ], [5 0 R /XYZ 731 null null ], [7 0 R /XYZ 731 null null ], [11 0 R /XYZ 731 null null ]]
+Bookmark[title=Working with Bookmarks , data=[42 0 R /XYZ 742 null null ]]
+Bookmark[title=Creating New Bookmarks  , data=[1 0 R /XYZ 731 null null ]]
+Bookmark[title=Creating a Bookmark Hierarchy  , data=[3 0 R /XYZ 731 null null ]]
+Bookmark[title=Editing Bookmarks  , data=[5 0 R /XYZ 731 null null ]]
+Bookmark[title=Deleting Bookmarks  , data=[7 0 R /XYZ 731 null null ]]
+Bookmark[title=Setting Documents Properties for Bookmarks , data=[11 0 R /XYZ 731 null null ]]