Repository: dotemacs/pdfboxing (clojure)

Issue:
Less strict `merge-pdfs`
## Expected Behavior
I have a couple of `input-streams` containing PDFs that I want to merge into an `output-stream`.

The current `merge-pdfs` function requires that all [inputs are files](https://github.com/dotemacs/pdfboxing/blob/ac810b22aafc32412b6450ef6b7be251ffad53e0/src/pdfboxing/merge.clj#L40), but PDFBox supports [adding `InputStreams`](https://pdfbox.apache.org/docs/2.0.1/javadocs/org/apache/pdfbox/multipdf/PDFMergerUtility.html#addSource(java.io.InputStream)).

Same with the output, `merge-pdfs` requires that the [output is a filename](https://github.com/dotemacs/pdfboxing/blob/ac810b22aafc32412b6450ef6b7be251ffad53e0/src/pdfboxing/merge.clj#L41), but PDFBox supports both [filenames](https://pdfbox.apache.org/docs/2.0.1/javadocs/org/apache/pdfbox/multipdf/PDFMergerUtility.html#setDestinationFileName(java.lang.String)) and [`OutputStreams`](https://pdfbox.apache.org/docs/2.0.1/javadocs/org/apache/pdfbox/multipdf/PDFMergerUtility.html#setDestinationStream(java.io.OutputStream)).

## Potential Solution

To solve this in my codebase I've updated `merge-pdfs` to be less strict:
```clojure
(defn merge-pdfs [& {:keys [input output]}]
  (let [merger (PDFMergerUtility.)]
    (doseq [source input]
      (.addSource merger source))
    (cond
      (instance? java.io.OutputStream output)
      (.setDestinationStream merger output)

      :else
      (.setDestinationFileName merger output))
    (.mergeDocuments merger)))
```

But I wanted your feedback before submitting a PR.
Is this something you'd be interested in?

Pull request:
## Description of your pull request

Releases the restrictions on `merge-pdfs` to allow supplying more types supported by [PDFMergerUtility](https://pdfbox.apache.org/docs/2.0.1/javadocs/org/apache/pdfbox/multipdf/PDFMergerUtility.html) under the hood.

## Why should it be considered?

PDFMergerUtility supports [strings](https://pdfbox.apache.org/docs/2.0.1/javadocs/org/apache/pdfbox/multipdf/PDFMergerUtility.html#addSource(java.lang.String)), [files](https://pdfbox.apache.org/docs/2.0.1/javadocs/org/apache/pdfbox/multipdf/PDFMergerUtility.html#addSource(java.io.File)) and [input streams](https://pdfbox.apache.org/docs/2.0.1/javadocs/org/apache/pdfbox/multipdf/PDFMergerUtility.html#addSource(java.io.InputStream)) as sources.
It also supports [file names](https://pdfbox.apache.org/docs/2.0.1/javadocs/org/apache/pdfbox/multipdf/PDFMergerUtility.html#setDestinationFileName(java.lang.String)) and [output streams](https://pdfbox.apache.org/docs/2.0.1/javadocs/org/apache/pdfbox/multipdf/PDFMergerUtility.html#setDestinationStream(java.io.OutputStream)) as outputs.

These features are useful because it doesn't require the user to realise the PDF as a file on disk.

## Note

I've tried to keep the changes as minimal as possible, but personally I'm not sure of the use of `arg-check` checking that input sources are pdfs when presumably PDFBox will do that as well.

Fixes #66

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 85243bd..1115d7b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@
 - Imports in `pdfboxing.common` comply with clj-kondo linting
 - Made `pdfboxing.form/set-fields` more efficient [#64](https://github.com/dotemacs/pdfboxing/pull/64)
 - Made `pdfboxing.form/get-fields` more robust by handling nested fields [#65](https://github.com/dotemacs/pdfboxing/pull/65)
+- Made `pdfboxing.merge/merge-pdfs` more robust by allowing more input and output types [#67](https://github.com/dotemacs/pdfboxing/pull/67)
 
 ## 0.1.15.1-SNAPSHOT
 
diff --git a/src/pdfboxing/merge.clj b/src/pdfboxing/merge.clj
index 1b90326..5aad241 100644
--- a/src/pdfboxing/merge.clj
+++ b/src/pdfboxing/merge.clj
@@ -1,6 +1,6 @@
 (ns pdfboxing.merge
   (:require [pdfboxing.common :as common])
-  (:import (java.io File FileInputStream)
+  (:import (java.io InputStream OutputStream)
            (org.apache.pdfbox.multipdf PDFMergerUtility)
            (org.apache.pdfbox.pdmodel PDDocument
                                       PDPage
@@ -15,13 +15,19 @@
 (defn check-if-present
   "Check if the input & output file names where supplied"
   [input output]
-  (when (some true? (map empty? [input output]))
+  (when (or (nil? input) (nil? output))
     (throw-exception "argument can't be empty")))
 
+(defn input-stream-or-pdf?
+  "Checks if the file supplied is a PDF or an InputStream"
+  [file]
+  (or (instance? InputStream file)
+      (common/is-pdf? file)))
+
 (defn check-for-pdfs
   "Check if all the files supplied are actual PDFs."
   [files]
-  (if (some false? (map common/is-pdf? files))
+  (if (some false? (map input-stream-or-pdf? files))
     (throw-exception "the files supplied need to be PDFs")
     true))
 
@@ -36,9 +42,14 @@
   [& {:keys [output input]}]
   {:pre [(arg-check output input)]}
   (let [merger (PDFMergerUtility.)]
-    (doseq [f input]
-      (.addSource merger (FileInputStream. (File. f))))
-    (.setDestinationFileName merger output)
+    (doseq [source input]
+      (.addSource merger source))
+    (cond
+      (instance? OutputStream output)
+      (.setDestinationStream merger output)
+
+      :else
+      (.setDestinationFileName merger output))
     (.mergeDocuments merger)))
 
 (defn- add-image-to-page


Test patch:
diff --git a/test/pdfboxing/merge_test.clj b/test/pdfboxing/merge_test.clj
index 945ac51..55a197b 100644
--- a/test/pdfboxing/merge_test.clj
+++ b/test/pdfboxing/merge_test.clj
@@ -6,16 +6,22 @@
 
 (deftest input-output-argument-check
   (is (thrown? IllegalArgumentException (arg-check)))
+  (is (thrown? IllegalArgumentException (arg-check nil)))
   (is (thrown? IllegalArgumentException (arg-check "input")))
+  (is (thrown? IllegalArgumentException (arg-check nil nil)))
   (is (thrown? IllegalArgumentException (arg-check "input" "output")))
-  (is (true? (arg-check "output" ["test/pdfs/clojure-1.pdf" "test/pdfs/clojure-2.pdf"]))))
+  (is (true? (arg-check "output" ["test/pdfs/clojure-1.pdf" "test/pdfs/clojure-2.pdf"])))
+  (with-open [output (io/output-stream "test/pdfs/foobar.pdf")
+              input-1 (io/input-stream "test/pdfs/clojure-1.pdf")
+              input-2 (io/input-stream "test/pdfs/clojure-2.pdf")]
+    (is (true? (arg-check output [input-1 input-2])))))
 
 
 (deftest pdf-merge-exit-status
         (is (nil? (merge-pdfs :output "test/pdfs/foo.pdf"
                               :input ["test/pdfs/clojure-1.pdf" "test/pdfs/clojure-2.pdf"]))))
 
-(deftest pdf-file-merge
+(deftest pdf-file-merge-paths
   (let [file "test/pdfs/foo.pdf"
         merging-outcome (merge-pdfs :output file
                                     :input ["test/pdfs/clojure-1.pdf" "test/pdfs/clojure-2.pdf"])
@@ -23,10 +29,32 @@
     (is (true? merged-pdf-file))
     (is (true? (common/is-pdf? file)))))
 
+(deftest pdf-file-merge-streams
+  (let [output-path "test/pdfs/bar.pdf"]
+    (with-open [output (io/output-stream output-path)
+                input-1 (io/input-stream "test/pdfs/clojure-1.pdf")
+                input-2 (io/input-stream "test/pdfs/clojure-2.pdf")]
+      (let [merging-outcome (merge-pdfs :output output
+                                        :input [input-1 input-2])
+            merged-pdf-file (.exists (io/as-file output-path))]
+        (is (true? merged-pdf-file))
+        (is (true? (common/is-pdf? output-path)))))))
+
+(deftest pdf-file-merge-files
+  (let [file "test/pdfs/foo.pdf"
+        merging-outcome (merge-pdfs :output file
+                                    :input [(io/as-file "test/pdfs/clojure-1.pdf")
+                                            (io/as-file "test/pdfs/clojure-2.pdf")])
+        merged-pdf-file (.exists (io/as-file file))]
+    (is (true? merged-pdf-file))
+    (is (true? (common/is-pdf? file)))))
+
 ;; clean up
 (defn clean-up [file]
   (if (.exists (io/as-file file))
     (io/delete-file file)))
 
 (deftest cleaner
-  (clean-up "test/pdfs/foo.pdf"))
+  (clean-up "test/pdfs/foo.pdf")
+  (clean-up "test/pdfs/bar.pdf")
+  (clean-up "test/pdfs/foobar.pdf"))