Attachment attributes instantiated even when excluded from select statement

[rossbearman]

Since an upgrade from 2.7 to 3.2, we've been encountering an issue with the new method for attaching the attributes to an entity. If we use a restrictive select on a query to retrieve a subset of attributes, as shown below, any attachments will also be tagged on to the end as soon as the objects are booted and there doesn't seem to be any way to exclude them.

In 2.7 Product::select(['name', 'sku', 'price'])->get() would only retrieve those three attributes for the entity as expected. Since the update (and presumably the switch to utilising model events) any attachment attributes will also be initialised.

This causes issues with code that serialises or converts entities as it can get stuck in a recursive loop following the attachment's $instance property back to the entity itself. We encountered this issue with Laravel Datatabes after updating Paperclip. Specifically, this function in the Datatables package gets stuck recursively looping between the entity, an attachment and the $instance reference back to the entity.

Using removeFileAttributes() after retrieving the entities would likely work, but this isn't possible with packages that only take in a query object and handles the retrieval itself.

[czim]

I've been poking around in the logic and I haven't found a sensible fix for this yet. Checking for the availability of the necessary keys (say, image_file_name) before merging an attachment does not work, because the attachment should still be available for 'normal' new model instances.

I've found no other way to detect the difference between a model hydrated with omitted attributes and a full model that simply has null values the same attributes, for instance.

These kinds of issues make me feel that the Stapler/Paperclip approach to attachments is just not right -- it's too finicky, too reliant on Eloquent's inner workings (that are liable to change).

I'll think on this some more, but for now I don't see a clear way out.

Edit: perhaps in your case it may be a good work-around to handle serialization of the models manually by implementing the Serializable interface. That way you could leave out any attachments on serialization, and just re-initialize them as needed?

[rossbearman]

Hi czim,

Thanks for taking the time to look into this. Unfortunately I don't think the workaround would work in our situation, at least not without considerable work.

I think we may just have to figure out a different way of providing the data to Datatables that doesn't trigger that convertToArray method, as it doesn't play nicely with the attachments.

[czim]

Perhaps a (temporary) addition to the hidden attributes? That'd also omit the attachments from a toArray. I don't know your use-case exactly, so just a random thought.

I think Paperclip needs to be rethought. It also needs an overhaul to make use of newer PHP features anyway.

[rossbearman]

That suggestion has made me think of an ugly, but workable solution for this case. As I can guarantee that certain attributes on this entity won't be null unless they're being called from a restrictive select, I should be able to override toArray on the entity and have it strip out the attachments in that situation.

Thanks again for your time on this issue, and Paperclip in general.

[rossbearman]

This is the solution I went with in the end, overriding the toArray() method on the entity, which works fine for our use case. This would likely be appropriate in most instances where someone is running into issues with toArray() on a Paperclip enabled entity, as currently calling toArray() will output the entire IoC container to the array along with a copy of the entity itself, which is probably never going to be what you want.

I wonder if a method that either replaces the normal file attribute object with a simple text representation (file path/URL, size, etc), or replaces the functionality of the attributesToArray() method in Model, dynamically replacing the file attributes for that call, would be an appropriate addition to PaperclipTrait?

This is the code I'm using, for anyone that stumbles upon this in future:

    /**
     * Convert the model instance to an array, excluding file attributes.
     *
     * @return array
     */
    public function toArray()
    {
        $this->removeFileAttributes();

        $array = array_merge($this->attributesToArray(), $this->relationsToArray());

        $this->mergeFileAttributes();

        return $array;
    }

[nevedimko]

Same problem for me. I use Datatables as well and I got an infinite loop in function convertToArray. I had to add attachment to $hidden array of Model.

<?php

namespace App\Models\Traits;

use Czim\Paperclip\Model\PaperclipTrait as BasePaperclipTrait;

trait PaperclipTrait
{
    use BasePaperclipTrait {
        hasAttachedFile as hasAttachedFileOrig;
    }

    /**
     * Add a new file attachment type to the list of available attachments.
     *
     * @param string $name
     * @param array  $options
     */
    public function hasAttachedFile($name, array $options = [])
    {
        $this->hasAttachedFileOrig($name, $options);

        $this->makeHidden($name);
    }
}