spatie / laravel-medialibrary

Associate files with Eloquent models
https://spatie.be/docs/laravel-medialibrary
MIT License
5.78k stars 1.08k forks source link

Multiple Calls to registerMediaCollections Result in Duplicate Entries in mediaCollections Array #3598

Closed RasmusGodske closed 6 months ago

RasmusGodske commented 6 months ago

Issue Overview: When using methods like getMediaCollection or getRegisteredMediaCollections from the InteractsWithMedia trait, registerMediaCollections is invoked multiple times. This behavior leads to multiple identical entries being added to the mediaCollections array, potentially causing unexpected behavior when interacting with media collections.

Steps to Reproduce:

  1. Define a MediaCollection in a model using the registerMediaCollections method:

    public function registerMediaCollections(): void
    {
    $this->addMediaCollection('documents');
    }
  2. Call getRegisteredMediaCollections() multiple times on the same model instance:

    $model->getRegisteredMediaCollections();
    $model->getRegisteredMediaCollections();
  3. Observe that the documents collection is registered multiple times in the mediaCollections array.

Expected Behavior: registerMediaCollections should ensure that it only adds unique media collections, or it should only be called once per instance to prevent duplicate registrations.

Actual Behavior: Every call to getRegisteredMediaCollections() or similar methods results in repeated registration of the same media collections.

Proposed Solutions:

Own temporary solution:

I got around the issue with this quick and dirty solution:

public function createMediaCollection(string $collection_name): MediaCollection
    {
        $mediaCollection = MediaCollection::create($collection_name);

        $has_been_reigstered = false;

        foreach ($this->mediaCollections as $registeredMediaCollection) {
            if ($registeredMediaCollection->name === $mediaCollection->name) {
                $has_been_reigstered = true;
                break;
            }
        }

        if (! $has_been_reigstered) {
            $this->mediaCollections[] = $mediaCollection;
        }

        return $mediaCollection;
    }
timvandijck commented 6 months ago

Thank you for the extensive bug report, I was able to replicate it in a test and fix the issue.

The mediaCollections was a regular indexed array. I now made it a key, value array with the name being the key. This should ensure that even when the register method is called no duplicates will be added.

The fix should be in the next release of the package.