Add imagick extension support #2194

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.
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

sbreker · 2025-10-29T23:02:25Z

lib/model/QubitDigitalObject.php

+        $pages = null;
+
+        if ($this->canThumbnail()) {
+            if (extension_loaded('imagick')) {


This gets checked a number of times to ensure if imagick is available. Perhaps this should be removed to be a method in sfImagickAdapter like sfImageMagickAdapter does. I think Imagick also requires ImageMagick to be installed, so perhaps a a method like sfImagickAdapter::isImagickAvailable() could encapsulate the logic to check that both Imagick and ImageMagick are present. Let me know what you think.

The two places I have this are in those methods that fall back to using sfImageMagickAdapter when the extension isn't loaded - if we completely remove the old adapter, then there'll be no need to call extension_loaded('imagick').

And Imagick doesn't require ImageMagick to be installed, the ImageMagick C code is directly compiled into the imagick extension when it's built. See here, the MagickWand C API is included and that's how the C gets compiled into the resulting PHP extension. The only thing that's needed is the imagemagick-dev package for building the extension. In Ubuntu, that won't be needed since there is already a pre-built package available: php8.3-imagick.

EDIT: You do still need imagemagick installed, I misunderstood how it works! The extension fails to be loaded if imagemagick is not installed.

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:

sbreker · 2025-12-19T01:13:08Z

lib/model/QubitDigitalObject.php

-            $command .= $path;
-            $command .= ' '.$filenameMinusExtension.'_%02d.'.self::THUMB_EXTENSION;
-            exec($command, $output, $status);
+            $imagick = new Imagick();


We should check that extension_loaded('imagick') is true before and issue some kind of error if it is not.

This function will only attempt to instantiate an Imagick instance now if the imagick extension is loaded: 4a92bf2

sbreker · 2025-12-19T01:13:31Z

lib/model/QubitDigitalObject.php

-                exec($command, $output, $status);
-                $pages = count($output);
-            }
+        $im = new Imagick();


We should check that extension_loaded('imagick') is true before and issue some kind of error if it is not.

This function will only attempt to instantiate an Imagick instance now if the imagick extension is loaded: 4a92bf2

sbreker · 2025-12-19T01:19:25Z

plugins/sfThumbnailPlugin/lib/sfThumbnail.class.php

        if (!$adapterClass) {
            if (extension_loaded('gd')) {
                $adapterClass = 'sfGDAdapter';
+            } elseif (extension_loaded('imagick')) {


Above in QubitDigitalObject.php we preferred Imagick over 'gd' but here we prefer 'gd' which could result in strange behaviour. It might be better to try loading first using Imagick here .

See:
#2194 (comment)

Fixed in d6de81e

sbreker · 2025-12-19T01:30:37Z

lib/model/QubitDigitalObject.php

                $path = $this->getAbsolutePath();
            }

            $filenameMinusExtension = preg_replace('/\.[a-zA-Z]{2,3}$/', '', $path);


Unrelated to your change but this parsing of extensions seems brittle. We should consider changing to use pathinfo($path, PATHINFO_DIRNAME|PATHINFO_FILENAME|etc).

I changed this line to use pathinfo: c721411

sbreker · 2025-12-19T01:44:18Z

plugins/sfThumbnailPlugin/lib/sfImagickAdapter.class.php

+    public function freeSource()
+    {
+        if (null !== $this->source) {
+            $this->source->clear();


We should perhaps also $this->source = null after calling clear() to ensure the PHP clears the memory.

Fixed in d6de81e

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.

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

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

Add imagick extension support #2194

Are you sure you want to change the base?

Add imagick extension support #2194

Uh oh!

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!

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!

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!

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!

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!

3 participants

danloveg commented Oct 6, 2025 •

edited

Loading

danloveg Nov 4, 2025 •

edited

Loading

danloveg Nov 4, 2025 •

edited

Loading