laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.55k stars 11.03k forks source link

[5.1] Hydrating a model assumes it exists already? #9360

Closed cabloo closed 9 years ago

cabloo commented 9 years ago

I stumbled upon this issue while writing a Seeder. The code should be fairly self-explanatory. When I run this, nothing shows up in the database.

list($hdds, $other) = PartType::hydrate([
    ['name' => 'Hard Drives'],
    ['name' => 'Other'],
])->each(function($item) {
    $item->save();
});

$hdd_parts = Part::hydrate([
    [
        'name' => 'HDD 1',
        'billing_id' => 'HDD-1',
    ], [
        'name' => 'HDD 2',
        'billing_id' => 'HDD-2',
    ],
]);

$hdds->parts()->saveMany($hdd_parts->all());

After some digging through the Model source code, I found that the hydrate function ends up creating a new Model instance and sets the exists property to true. Then, when calling the save function, nothing is added to the database because it is assumed to exist there already.

As a temporary fix for my use case, I added this to my base Model. With this code added, everything shows up in the database as expected.

    public static function hydrate(array $items, $connection = null)
    {
        return parent::hydrate($items, $connection)->each(function($item) {
            $item->exists = false;
        });
    }

However, I think the correct solution would be to create each new Model instance without assuming it exists already.

lukasgeiter commented 9 years ago

That's intentional and totally how hydrate should work. It's specifically to hydrate the data from the db into a model.

cabloo commented 9 years ago

Ah, I understood it as turning raw arrays into models so that they could be stored in the database.

So I should just create a second function like hydrate that does this specifically for seeding, using the logic from my overridden hydrate function?

On Mon, Jun 22, 2015, 2:39 AM lukasgeiter notifications@github.com wrote:

That's intentional and totally how hydrate should work. It's specifically to hydrate the data from the db into a model.

— Reply to this email directly or view it on GitHub https://github.com/laravel/framework/issues/9360#issuecomment-114021187.

franzliedke commented 9 years ago

These sections of the docs should help you:

I think this ticket can be closed. :)

cabloo commented 9 years ago

Yes, thanks for your help. It's unfortunate that there's no fillMany or similar in Eloquent, I ended up renaming the hydrate function I wrote above to fillMany for this use case.