Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the ability to overlay one PDF over another #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Added
- Added the ability to merge multiple images into a single PDF
- Added the ability to load PDFs from byte arrays
- Added the ability to overlay one PDF over another

### Changed
- Using lists for :imports
Expand Down
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ Clojure PDF manipulation library & wrapper for [PDFBox](http://pdfbox.apache.org
(pdf/merge-pdfs :input ["test/pdfs/clojure-1.pdf" "test/pdfs/clojure-2.pdf"] :output "foo.pdf")
```

### Overlay one PDF over another

```clojure
(require '[pdfboxing.overlay :as pdf-overlay])
(pdf-overlay/overlay-pdf
{:input-file-path "test/pdfs/clojure-1.pdf"
:overlay-file-path "test/pdfs/clojure-2.pdf"
:output-file-path "test/pdfs/foo.pdf"})
```

### Merge multiple images into single PDF

You can use either `merge-images-from-path` for providing images in
Expand Down
47 changes: 47 additions & 0 deletions src/pdfboxing/overlay.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
(ns pdfboxing.overlay
(:import
(org.apache.pdfbox.multipdf Overlay)))

(defn overlay-pdf
"overlay one PDF over another"
[{:keys
[input-file-path
input-pdf
overlay-file-path
overlay-pdf
first-page-overlay-pdf
first-page-overlay-file-path
last-page-overlay-file-path
last-page-overlay-pdf
all-page-overlay-file-path
all-page-overlay-pdf
odd-page-overlay-file-path
odd-page-overlay-pdf
even-page-overlay-file-path
even-page-overlay-pdf
^String output-file-path]}]

(let [overlay (Overlay.)]
(.setInputFile overlay input-file-path)
(.setInputPDF overlay input-pdf)

(.setDefaultOverlayFile overlay overlay-file-path)
(.setDefaultOverlayPDF overlay overlay-pdf)

(.setFirstPageOverlayFile overlay first-page-overlay-file-path)
(.setFirstPageOverlayPDF overlay first-page-overlay-pdf)

(.setLastPageOverlayFile overlay last-page-overlay-file-path)
(.setLastPageOverlayPDF overlay last-page-overlay-pdf)

(.setAllPagesOverlayFile overlay all-page-overlay-file-path)
(.setAllPagesOverlayPDF overlay all-page-overlay-pdf)

(.setOddPageOverlayFile overlay odd-page-overlay-file-path)
(.setOddPageOverlayPDF overlay odd-page-overlay-pdf)

(.setEvenPageOverlayFile overlay even-page-overlay-file-path)
(.setEvenPageOverlayPDF overlay even-page-overlay-pdf)

(-> overlay (.overlay {}) (.save output-file-path))
Copy link

@PavlosMelissinos PavlosMelissinos Jan 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for hijacking the thread, I thought I'd offer some tips... Hope it's ok 🙂

So yeah, this is dotemacs' call but you might want to avoid saving to disk since that's a side-effect (and side-effects make writing proper tests harder, among other things)

I'd also argue that returning a byte array would be more useful to the end-user. Every user of your function is interested in the output, one way or the other, but how many will be interested in storing it locally right away?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem. Thank you for the feedback. I agree a byte array would be more useful in general but would mean its different from the other functions for example draw, form and merge all save to disk.
I'm happy to have it either way. @dotemacs is there reason to save to disk here or can I change this to return a byte array?

(.close overlay)))
27 changes: 27 additions & 0 deletions test/pdfboxing/overlay_test.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
(ns pdfboxing.overlay-test
(:require
[clojure.java.io :as io]
[clojure.test :refer [deftest is]]
[pdfboxing.common :as common]
[pdfboxing.overlay :refer [overlay-pdf]]))

(def output-file-path "test/pdfs/foo.pdf")

(deftest pdf-file-merge
(let [_
(overlay-pdf
{:input-file-path "test/pdfs/clojure-1.pdf"
:overlay-file-path "test/pdfs/clojure-2.pdf"
:output-file-path output-file-path})

overlayed-pdf-file (.exists (io/as-file output-file-path))]
(is (true? overlayed-pdf-file))
(is (true? (common/is-pdf? output-file-path)))))

;; clean up
(defn clean-up [file]
(if (.exists (io/as-file file))
(io/delete-file file)))

(deftest cleaner
(clean-up "test/pdfs/foo.pdf"))
Comment on lines +10 to +27
Copy link

@PavlosMelissinos PavlosMelissinos Jan 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more feedback here as well!

  1. Keep in mind that not all test runners are deterministic! I don't know about clj test-runner but others like kaocha allow you to randomize the test run sequence to catch side-effects. Therefore you should not make assumptions about the order tests run. Consider cleaning up the temporary file in the deftest it was created ;).
  2. What if the output file already exists for some reason unrelated to these tests? Your tests will overwrite that and then delete it. That shouldn't happen. As an alternative, take a look at this: https://stackoverflow.com/a/57845906

You could also consider the following function:

(defn with-temp-pdf [prefix fn]
  (let [file (java.io.File/createTempFile prefix ".pdf")]
    (fn (str file))
    (if (.exists file)
      (io/delete-file file))

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good points! I followed this pattern from the tests in merge.
I'll update once the byte array vs output file is finalised.