At modmore we have 3 extras that use file uploads one way or another: ContentBlocks, Redactor and MoreGallery. The file upload utilities in these extras have all been built with the Media Sources API in MODX, allowing them to support different media source configurations, remote sources like Amazon S3 and plugins that fire on upload.

Unfortunately, some of those plugins change the name of the uploaded file (for sanitisation or SEO purposes) without the media sources exposing that to the upload, and for a long time we've been unable of supporting plugins like FileSluggy and filetranslit.

The Problem

In our code, we call $source->uploadObjectsToContainer() with a prepared files array, and the path we determined it should go to inside the media source. The media source, whether that's the file or Amazon S3 one, then takes care of the actual upload and returns a true (successful upload) or false (something went wrong).

As the upload happens, the OnFileManagerUpload event is fired by MODX, and plugins like FileSluggy are executed. If they determine the filename does not meet their requirements (for example it contains dashes while underscores are preferred, or it contains special characters), the plugin calls $source->renameObject() with a new file name. This is where the problem happens, as our code doesn't get a message that the file name was changed after upload. So we try to show an image with a name that no longer exists. Broken image.

For a long time we were unable of figuring out a workaround, until some guys from Sterc walked into modmore hq to check on the issue. Following a bit of a discussion and some source code digging, an epiphany happened and we finally found a fix. Since then we've updated our extras to fix the compatibility. As there might be other people out there (today or in six months from now) that need to deal with the same problem, I'd like to present you our solution.

The Solution

As with all issues that take ages to solve, the solution is pretty simple: listen for renamed files.

In the same fashion that MODX fires an event on upload in $source->uploadObjectsToContainer(), it also fires an event when a file is renamed through $source->renameObject(). We can listen for this event in a plugin, store the updated file name, and check if any file renames happened during the upload.

Here's how we fixed this, taking ContentBlocks as the example.

Step 1: Add a place to store changed filenames

The most obvious place to store changed file names is on the service class. Pretty much all MODX Extras have one with the same name as the package. As a service class, we can also use $modx->getService() to always get the same instance - rather than creating a new copy every time - so we can store our changed file name there temporarily.

For this purpose we added a simple $renames array.

<?php

class ContentBlocks {
    public $renames = array();
    
    public function __construct(modX $modx, array $config = array()) {
        // ...
    }
}

If you're feeling fancy, you might add a getter and setter for it, but we just made it public instead.

After adding it to the class we can start interacting with it, however it's important that we share the same instance across the request. In our case, we had a direct instantiation in assets/components/contentblocks/connector.php like this:

<?php
// ...
$corePath = $modx->getOption('contentblocks.core_path', null, $modx->getOption('core_path') . 'components/contentblocks/');
require_once $corePath.'model/contentblocks/contentblocks.class.php';
$modx->contentblocks = new ContentBlocks($modx);
// ...

Elsewhere in the code, such as in our plugin, we were using $modx->getService. The getService call would return a new instance because the earlier instance wasn't created properly, meaning we would be unable of accessing the file paths added to $contentblocks->renames.

The fix for this was luckily easy, we just had to switch the connector to also use getService.

<?php
// ...
$corePath = $modx->getOption('contentblocks.core_path', null, $modx->getOption('core_path') . 'components/contentblocks/');
$modx->getService('contentblocks', 'ContentBlocks', $corePath . 'model/contentblocks/');
// ...

Step 2: Log the file name changes

The next step is to listen to the OnFileManagerFileRename system event in MODX, and to log the new file name to our $contentblocks->renames array. To do this, we updated our existing plugin to also listen to that event and simply added the following to the code:

<?php
$corePath = $modx->getOption('contentblocks.core_path', null, $modx->getOption('core_path') . 'components/contentblocks/');
$contentblocks = $modx->getService('contentblocks', 'ContentBlocks', $corePath . 'model/contentblocks/');

switch ($modx->event->name) {
    case 'OnFileManagerFileRename':
        $contentblocks->renames[] = $path;
        break;
        
    // ...
}

$path contains the new path to the file. This way, whenever a file is renamed, ContentBlocks is made aware of it.

Step 3: Updating the Upload Code

The third, and final, step is to update the upload code to check for file renames. Here's what that looks like for ContentBlocks:

<?php

class ContentBlocksImageUploadProcessor extends ContentBlocksImageProcessor {
    /**
     * @return bool
     */
    public function process() {
        // ... bunch of preprocessing here
        
        /**
         * Do the upload
         */
        $this->contentblocks->renames = array();
        $uploaded = $this->source->uploadObjectsToContainer($this->path, $_FILES);
        if (!$uploaded) {
            $errors = $this->source->getErrors();
            $errors = implode('<br />', $errors);
            return $this->failure($errors);
        }
        /**
         * Check if the file has been renamed by a plugin like FileSluggy
         */
        $newFileName = reset($this->contentblocks->renames);
        if (!empty($newFileName)) {
            $baseMediaPath = $this->source->getBasePath() . $this->path;
            $newFileName = substr($newFileName, strlen($baseMediaPath));
            $_FILES['file']['name'] = $newFileName;
        }
        
        // ... and more processing here
    }
}

Obviously this is just a small snippet of the upload processor, and this exact logic might vary greatly between extras. Even though we've been standardising our code, even between our own extras there are big differences in how uploads are handled.

The basic gist of how this works is to ensure the renames array is empty ($this->contentblocks->renames = array()) before calling $this->source->uploadObjectsToContainer(), and then after the upload see if any file names were changed.

In ContentBlocks, we refer to the information in the $_FILES superglobal further down in the processor to get the full file url, so we just need to update that with the new file name. If your code has something like an url variable that you return, you'll need to fix that based on the path from $this->contentblocks->renames.

Conclusion

Sometimes all it takes to get past a longstanding issue is to discuss it with others, and a bit of creativity.