Merge pull request #3 from DEXPRO-Solutions-GmbH/fix/landscape-portra…

…it-check

Fix unwanted scaling on landscape A4

.github/workflows/maven.yml

@@ -21,6 +21,17 @@ jobs:
         java-version: '17'
         distribution: 'temurin'
         cache: maven
+
+    - name: Install deps with Maven
+      run: mvn install --file pom.xml
+
+    - name: Test with Maven
+      run: mvn test --file pom.xml
+
+    - name: Upload Test Report
+      uses: ScaCap/action-surefire-report@v1.7.2
+      if: success() || failure()
+
     - name: Build with Maven
       run: mvn -B package --file pom.xml
 

pom.xml

@@ -6,7 +6,7 @@
 
     <groupId>one.squeeze</groupId>
     <artifactId>pdftools</artifactId>
-    <version>0.1.0</version>
+    <version>0.1.1</version>
 
     <properties>
         <maven.compiler.source>8</maven.compiler.source>
@@ -39,6 +39,12 @@
                     </execution>
                 </executions>
             </plugin>
+
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>2.22.2</version>
+            </plugin>
         </plugins>
     </build>
 
@@ -54,5 +60,11 @@
             <artifactId>picocli</artifactId>
             <version>4.7.5</version>
         </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>5.6.2</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>

src/main/java/one/squeeze/pdftools/app/scale/IScaler.java

@@ -0,0 +1,12 @@
+package one.squeeze.pdftools.app.scale;
+
+import org.apache.pdfbox.pdmodel.PDDocument;
+import org.apache.pdfbox.pdmodel.PDPage;
+
+import java.io.IOException;
+
+public interface IScaler {
+
+    public void scale(PDDocument pdf, PDPage page) throws IOException;
+
+}

src/main/java/one/squeeze/pdftools/app/scale/NoopScaler.java

@@ -0,0 +1,11 @@
+package one.squeeze.pdftools.app.scale;
+
+import org.apache.pdfbox.pdmodel.PDDocument;
+import org.apache.pdfbox.pdmodel.PDPage;
+
+import java.io.IOException;
+
+public class NoopScaler implements IScaler {
+    @Override
+    public void scale(PDDocument pdf, PDPage page) throws IOException {}
+}

src/main/java/one/squeeze/pdftools/app/scale/Scaler.java

@@ -0,0 +1,45 @@
+package one.squeeze.pdftools.app.scale;
+
+import org.apache.pdfbox.pdmodel.PDDocument;
+import org.apache.pdfbox.pdmodel.PDPage;
+import org.apache.pdfbox.pdmodel.PDPageContentStream;
+import org.apache.pdfbox.pdmodel.common.PDRectangle;
+import org.apache.pdfbox.util.Matrix;
+
+import java.io.IOException;
+
+public class Scaler implements IScaler {
+
+    private final float factor;
+
+    private final PDRectangle targetMediaBox;
+
+    public Scaler(float factor, PDRectangle targetMediaBox) {
+        if (factor <= 0 || factor == 1) {
+            throw new IllegalArgumentException("Invalid factor: " + factor);
+        }
+        if (targetMediaBox == null) {
+            throw new IllegalArgumentException("Null targetMediaBox argument");
+        }
+
+        this.factor = factor;
+        this.targetMediaBox = targetMediaBox;
+    }
+
+    @Override
+    public void scale(PDDocument pdf, PDPage page) throws IOException {
+        PDPageContentStream contentStream = new PDPageContentStream(pdf, page, PDPageContentStream.AppendMode.PREPEND, false);
+        contentStream.transform(Matrix.getScaleInstance(factor, factor));
+        contentStream.close();
+
+        page.setMediaBox(targetMediaBox);
+    }
+
+    public float getFactor() {
+        return factor;
+    }
+
+    public PDRectangle getTargetMediaBox() {
+        return targetMediaBox;
+    }
+}

src/main/java/one/squeeze/pdftools/cli/cmds/FixPDFCommand.java

@@ -1,16 +1,16 @@
 package one.squeeze.pdftools.cli.cmds;
 
 
+import one.squeeze.pdftools.app.scale.IScaler;
+import one.squeeze.pdftools.app.scale.NoopScaler;
+import one.squeeze.pdftools.app.scale.Scaler;
 import org.apache.pdfbox.Loader;
 import org.apache.pdfbox.pdmodel.PDDocument;
 import org.apache.pdfbox.pdmodel.PDPage;
-import org.apache.pdfbox.pdmodel.PDPageContentStream;
 import org.apache.pdfbox.pdmodel.PDPageTree;
 import org.apache.pdfbox.pdmodel.common.PDRectangle;
-import org.apache.pdfbox.util.Matrix;
 import picocli.CommandLine;
 
-import java.awt.print.PrinterJob;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -50,25 +50,44 @@ public Integer call() throws Exception {
 
     private static void fix(File input, File output) throws IOException {
         try (PDDocument pdf = Loader.loadPDF(input)) {
-            PrinterJob job = PrinterJob.getPrinterJob();
             PDPageTree tree = pdf.getPages();
 
             for (PDPage page : tree) {
-                if (page.getMediaBox().getWidth() > MAX_WIDTH || page.getMediaBox().getHeight() > MAX_HEIGHT) {
-                    float fWidth = MAX_WIDTH / page.getMediaBox().getWidth();
-                    float fHeight = MAX_HEIGHT / page.getMediaBox().getHeight();
+                IScaler scaler = buildScaler(page);
+                scaler.scale(pdf, page);
+            }
 
-                    float factor = Math.min(fWidth, fHeight);
+            pdf.save(output);
+        }
+    }
 
-                    PDPageContentStream contentStream = new PDPageContentStream(pdf, page, PDPageContentStream.AppendMode.PREPEND, false);
-                    contentStream.transform(Matrix.getScaleInstance(factor, factor));
-                    contentStream.close();
+    public static IScaler buildScaler(PDPage page) {
+        PDRectangle mediaBox = page.getMediaBox();
+        boolean isPortrait = mediaBox.getWidth() < mediaBox.getHeight();
+        boolean shouldScale = false;
 
-                    page.setMediaBox(PDRectangle.A4);
-                }
-            }
+        if (isPortrait) {
+            shouldScale = mediaBox.getWidth() > MAX_WIDTH || mediaBox.getHeight() > MAX_HEIGHT;
+        } else {
+            shouldScale = mediaBox.getWidth() > MAX_HEIGHT || mediaBox.getHeight() > MAX_WIDTH;
+        }
+        if (!shouldScale) {
+            return new NoopScaler();
+        }
 
-            pdf.save(output);
+        // Calculate scale factors. Depending on the orientation this requires division of height or width.
+        float fWidth = 1;
+        float fHeight = 1;
+        if (isPortrait) {
+            fWidth = MAX_WIDTH / page.getMediaBox().getWidth();
+            fHeight = MAX_HEIGHT / page.getMediaBox().getHeight();
+        } else {
+            fWidth = MAX_HEIGHT / page.getMediaBox().getWidth();
+            fHeight = MAX_WIDTH / page.getMediaBox().getHeight();
         }
+
+        float factor = Math.min(fWidth, fHeight);
+
+        return new Scaler(factor, PDRectangle.A4);
     }
 }

src/test/java/one/squeeze/pdftools/cli/cmds/FixPDFCommandTest.java

@@ -0,0 +1,95 @@
+package one.squeeze.pdftools.cli.cmds;
+
+import one.squeeze.pdftools.app.scale.IScaler;
+import one.squeeze.pdftools.app.scale.NoopScaler;
+import one.squeeze.pdftools.app.scale.Scaler;
+import org.apache.pdfbox.pdmodel.PDPage;
+import org.apache.pdfbox.pdmodel.common.PDRectangle;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class FixPDFCommandTest {
+
+
+    /**
+     * This constant is used to compare the expected output of factor calculation against 1.
+     * This is a nasty workarround, it would be better to improve the assertions using this constant.
+     * Either way, this currently works fine.
+     */
+    public static final double TEN_PERCENT = 0.10000000149011612;
+
+    @Test
+    void testBuildScaler_NoopOnA4Portrait() {
+        PDPage page = new PDPage();
+        PDRectangle box = new PDRectangle();
+        box.setUpperRightX(595);
+        box.setUpperRightY(841);
+        page.setMediaBox(box);
+
+        IScaler scaler = FixPDFCommand.buildScaler(page);
+        assertTrue(scaler instanceof NoopScaler);
+    }
+
+    @Test
+    void testBuildScaler_NoopOnA4Landscape() {
+        PDPage page = new PDPage();
+        PDRectangle box = new PDRectangle();
+        box.setUpperRightX(841);
+        box.setUpperRightY(595);
+        page.setMediaBox(box);
+
+        IScaler scaler = FixPDFCommand.buildScaler(page);
+        assertTrue(scaler instanceof NoopScaler);
+    }
+
+    @Test
+    void testBuildScaler_NoopOnSmallerThanA4Portrait() {
+        PDPage page = new PDPage();
+        PDRectangle box = new PDRectangle();
+        box.setUpperRightX((float) 595 / 2);
+        box.setUpperRightY((float) 841 / 2);
+        page.setMediaBox(box);
+
+        IScaler scaler = FixPDFCommand.buildScaler(page);
+        assertTrue(scaler instanceof NoopScaler);
+    }
+
+    @Test
+    void testBuildScaler_NoopOnSmallerThanA4Landscape() {
+        PDPage page = new PDPage();
+        PDRectangle box = new PDRectangle();
+        box.setUpperRightX((float) 841 / 2);
+        box.setUpperRightY((float) 595 / 2);
+        page.setMediaBox(box);
+
+        IScaler scaler = FixPDFCommand.buildScaler(page);
+        assertTrue(scaler instanceof NoopScaler);
+    }
+
+    @Test
+    void testBuildScaler_ScalesLargePortrait() {
+        PDPage page = new PDPage();
+        PDRectangle box = new PDRectangle();
+        box.setUpperRightX(595 * 10);
+        box.setUpperRightY(841 * 10);
+        page.setMediaBox(box);
+
+        Scaler scaler = (Scaler) FixPDFCommand.buildScaler(page);
+        assertEquals(TEN_PERCENT, scaler.getFactor());
+        assertEquals(PDRectangle.A4, scaler.getTargetMediaBox());
+    }
+
+    @Test
+    void testBuildScaler_ScalesLargeLandscape() {
+        PDPage page = new PDPage();
+        PDRectangle box = new PDRectangle();
+        box.setUpperRightX(841 * 10);
+        box.setUpperRightY(595 * 10);
+        page.setMediaBox(box);
+
+        Scaler scaler = (Scaler) FixPDFCommand.buildScaler(page);
+        assertEquals(TEN_PERCENT, scaler.getFactor());
+        assertEquals(PDRectangle.A4, scaler.getTargetMediaBox());
+    }
+}