Add imagick extension support by danloveg · Pull Request #2194 · artefactual/atom

danloveg · 2025-10-06T21:09:35Z

Installs the imagick extension in the application image.
Adds a new sfImagickAdapter that can be used with the sfThumbnail class.
~~Adds deprecation warnings to methods using pdfinfo, convert, or identify with sfImageMagickAdapter. The old adapter still works, but the new adapter is used if imagick is installed.~~ (Removed sfImageMagickAdapter)
Refactored exploding multi-page assets to use imagick if it's installed.

Things that are not complete:

I have not created a policy.xml file, I wanted some feedback on this PR before looking into that.
This PR does not update any of the deployment documentation or any Ansible scripts.
~~Unit tests.~~ (Tests added)
Copyright block at the top of the new adapter? I added a new file to a plugin that was not developed at Artefactual, not sure what sort of copyright case that falls under. I'd guess the regular AtoM copyright block but I'm not sure whether the licenses are compatible. (Added the default AtoM copyright block based on CODE_OF_CONDUCT.md)

Also of note, I did not implement the fancy thumbnail method that sfImageMagickAdapter does since I couldn't see any place where those custom methods were being used.

lib/model/QubitDigitalObject.php

sbreker

Hi Daniel - thank you so much for this code contribution! I have added some additional feedback regarding the memcache change, and the logic to fall back to using ImageMagick via exec(). Please let me know if there is anything I should be considering with respect to these requests. Interested to hear your thoughts.

Dockerfile

sbreker · 2025-10-29T22:50:59Z

lib/model/QubitDigitalObject.php

                $pages = sfImageMagickAdapter::getPdfinfoPageCount($filename);
-            } else {
-                $command = 'identify '.$filename;
+            } elseif (sfImageMagickAdapter::isImageMagickAvailable()) {


I've been thinking about the logic to fall back to using ImageMagick in the event that the Imagick PHP extension is not present. I think that while this may provide some convenience with respect to AtoM upgrade requirements, I think it is more important to completely remove the original way of calling ImageMagick with exec() calls and rely solely on your new work to introduce Imagick. Removing the calls to exec() entirely will avoid triggering alerts in future security scans and remove the risk associated with these calls. In this case, if we remove all dependency on sfImageMagickAdapter.class.php, then we might be able to delete sfImageMagickAdapter.class.php entirely (I have not checked if there are other locations where it is referenced - but removing it would have implications for the PDF page count methods in there).

That would certainly simplify things in the code. One issue I can see with that change is that we'll want to ensure the upgrade instructions are up to date for whichever version of AtoM this ends up in since php8.3-imagick will be a dependency requirement ~~, and imagemagick will no longer be required.~~

The imagick adapter handles PDF page counts without the use of Poppler (i.e., pdfinfo), having spent some time with this part of the code, I think completely removing sfImageMagickAdapter shouldn't be an issue at all.

I made some commits for what it would look like without sfImageMagickAdapter, let me know what you think.

If needed, I can revert this commit range to bring it back:

d29c313..5438fc5

lib/model/QubitDigitalObject.php

sbreker · 2025-12-19T01:10:11Z

lib/model/om/BaseDigitalObject.php

    try
    {
-      return call_user_func_array('QubitObject::__get', $args);
+      return parent::__get(...$args);


I think this file is generated by Symfony. Was this change the result of running a php symfony commands?

I can't remember why I made that change initially, but things are still working with that change reverted.

I take that back - I remember why I made that change now. It's because if you change it back to how it was, this is how the unit test output looks:

lib/model/QubitDigitalObject.php

plugins/sfThumbnailPlugin/lib/sfThumbnail.class.php

lib/model/QubitDigitalObject.php

plugins/sfThumbnailPlugin/lib/sfImagickAdapter.class.php

This is based on the sfImageMagickAdapter. I opted not to use pdfinfo so that there is no exec() going on in this new adapter.

This has to be set *before* reading an image.

I needed to remove it from three places: 1. From the digital object derivatives setting action. imagick handles PDF page extraction, not pdfinfo any more. So I changed the error message to reflect that. 2. From the QubitDigitalObject class. I was using the sfImageMagickAdapter as a fallback for sfImagickAdapter, so I removed those fallback cases. 3. From the lime unit tests in the sfThumbnailPlugin. The unit tests do run successfully in the sfThumbnailPlugin class, but it's tedious to run them. In case you want to run them, you need to run these commands in the Docker container to install gd. apk add libpng-dev libjpeg-turbo-dev docker-php-ext-configure gd --with-jpeg docker-php-ext-install gd This is because the tests *require* you to have gd installed, and gd is not installed in the base AtoM image. And then to run the tests, you invoke the test file directly: php plugins/sfThumbnailPlugin/test/unit/sfThumbnailTest.php You'll see this when the tests run: # Looks like everything went fine. # Looks like everything went fine.

It's no longer needed! Also, added a note to the README of the plugin since the way the plugin works has changed a lot.

apps/qubit/modules/settings/templates/digitalObjectDerivativesSuccess.php

anvit · 2026-01-21T20:59:10Z

plugins/sfThumbnailPlugin/lib/sfImagickAdapter.class.php

+ */
+class sfImagickAdapter
+{
+    protected const string PDF_EXTRACT_BACKGROUND = 'white';


I would suggest omitting the type here since this would break in PHP 8.1 and PHP 8.2, which our documentation still lists as being supported in AtoM 2.10.x

Good catch, I removed the types in a36230a

anvit · 2026-01-21T20:59:51Z

lib/model/QubitDigitalObject.php

+    public const string MULTI_PAGE_ASSET_EXTRACT_BACKGROUND = 'white';
+    public const string MULTI_PAGE_ASSET_EXTRACT_FORMAT = 'jpeg';
+    public const int MULTI_PAGE_ASSET_EXTRACT_QUALITY = 100;
+    public const int MULTI_PAGE_ASSET_EXTRACT_RESOLUTION = 300;  // This controls the DPI


We should omit the types here as well since this would break in PHP 8.1 and PHP 8.2.

Removed in a36230a

sarah-mason added Community Pull requests that have been contributed from community members outside Artefactual work-in-progress labels Oct 7, 2025

sbreker reviewed Oct 27, 2025

View reviewed changes

lib/model/QubitDigitalObject.php Outdated Show resolved Hide resolved

sbreker reviewed Oct 27, 2025

View reviewed changes

lib/model/QubitDigitalObject.php Outdated Show resolved Hide resolved

sbreker reviewed Oct 27, 2025

View reviewed changes

lib/model/QubitDigitalObject.php Outdated Show resolved Hide resolved

sbreker requested changes Oct 29, 2025

View reviewed changes

sbreker reviewed Dec 19, 2025

View reviewed changes

lib/model/QubitDigitalObject.php Show resolved Hide resolved

sbreker reviewed Dec 19, 2025

View reviewed changes

lib/model/QubitDigitalObject.php Show resolved Hide resolved

sbreker reviewed Dec 19, 2025

View reviewed changes

plugins/sfThumbnailPlugin/lib/sfThumbnail.class.php Outdated Show resolved Hide resolved

sbreker reviewed Dec 19, 2025

View reviewed changes

lib/model/QubitDigitalObject.php Outdated Show resolved Hide resolved

sbreker reviewed Dec 19, 2025

View reviewed changes

plugins/sfThumbnailPlugin/lib/sfImagickAdapter.class.php Show resolved Hide resolved

danloveg added 18 commits December 22, 2025 11:44

Install imagick==3.8.0 in app image

274284e

Create sfImagickAdapter class

0134ffb

This is based on the sfImageMagickAdapter. I opted not to use pdfinfo so that there is no exec() going on in this new adapter.

Fix logic when requested page > page count

84b8286

Add deprecation warning to sfImageMagickAdapter

936152a

Enable sfImagickAdapter

805d8df

Re-write ad hoc multi-page logic with Imagick

11cb9cd

Simplify getting MIME type

d531651

Fix incorrect page error issue

b2f3d97

Add error checking in loadFile

c7ada99

Use constants instead of magic strings/nums

509db25

Fix setting resolution

5015b72

This has to be set *before* reading an image.

Add AtoM copyright block

2d546f9

Check image magick availability for fallback cases

42483ae

Change 'white' to constant in adapter

e056770

Revert changes to memcache installation

18a2d17

Add missing cleanup command

719dbed

Remove the sfImageMagickAdapter.class.php file

a77b974

It's no longer needed! Also, added a note to the README of the plugin since the way the plugin works has changed a lot.

danloveg added 6 commits December 22, 2025 11:44

Update comments in QubitDigitalObject

eba011e

Add thumbnail tests for QubitDigitalObject

a29a839

Simplify logic in setPageCount()

37e751b

Add more tests for thumbnails and page counts

5b950a2

Remove PDF tests that policy.xml is preventing

6224535

Revert changes to BaseDigitalObject.php

49302a5

danloveg force-pushed the dev/2163-switch-to-imagick-library branch from f413650 to 49302a5 Compare December 22, 2025 20:19

danloveg added 3 commits December 22, 2025 14:39

Add functions to check for image extensions

4a92bf2

Adjust extension priority in plugin

d6de81e

Use pathinfo instead of regex for filename

c721411

danloveg marked this pull request as ready for review December 22, 2025 22:20

melaniekung removed the work-in-progress label Jan 15, 2026

sbreker reviewed Jan 20, 2026

View reviewed changes

apps/qubit/modules/settings/templates/digitalObjectDerivativesSuccess.php Outdated Show resolved Hide resolved

Capitalize Imagick in message

a8b0610

anvit requested changes Feb 18, 2026

View reviewed changes

Remove types, fix error message

a36230a

Conversation

danloveg commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sbreker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danloveg Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

danloveg commented Oct 6, 2025 •

edited

Loading

danloveg Nov 4, 2025 •

edited

Loading