Skip to content

Conversation

@cosmastech
Copy link
Contributor

@cosmastech cosmastech commented Oct 30, 2025

It's really, really easy to write inefficient tests by leveraging Order::factory()->count(100)->create() not realizing that it's going to run 100 inserts. I just fixed this in a number of tests this week.

Here we add a Factory@insert() method that allows performing a mass insert of models.

Note that this does not return anything (it's akin to the Eloquent model's insert() method), and similarly does not emit Model events. This is only really useful if you need to insert lot of some model in your database, but don't actually need those models handy in the test. Think of testing pagination or how a resource returns a count.

@cosmastech cosmastech marked this pull request as draft October 30, 2025 22:23
@browner12
Copy link
Contributor

Is there a limitation that prevents you from returning the collection?

If you can get this to work that'd be great. I think there is some nuance and complications though with multiple inserts and things like model events not triggering that you'd have to deal with. Probably part of the same reason we don't have a save() method on the Eloquent/Collection yet.

@cosmastech
Copy link
Contributor Author

cosmastech commented Oct 30, 2025

Is there a limitation that prevents you from returning the collection?

If you can get this to work that'd be great. I think there is some nuance and complications though with multiple inserts and things like model events not triggering that you'd have to deal with. Probably part of the same reason we don't have a save() method on the Eloquent/Collection yet.

Generated IDs won't be available, for instance. We could return the collection, but it would be like a sharp knife falling in a kitchen.

It is probably worth noting in the docblock that model events aren't fired, which is by design (because they require the single insert approach)

@cosmastech cosmastech marked this pull request as ready for review October 31, 2025 01:34
@cosmastech cosmastech changed the title [12.x] Factory@insert() [12.x] Factory@insert() Oct 31, 2025
@shaedrich
Copy link
Contributor

You could have gone with returning an Eloquent collection just like createMany(). This method could be named something like massCreateMany(), however, this wouldn't follow the createMany*() convention of the other related methods.

@taylorotwell taylorotwell merged commit a91b059 into laravel:12.x Oct 31, 2025
65 of 66 checks passed
@cosmastech cosmastech deleted the factory-insert branch October 31, 2025 17:20
@jasonmccreary
Copy link
Contributor

@shaedrich, I think void is appropriate here. It avoids any testing footguns where you might try to use the result in a later assertion.

@tpetry
Copy link
Contributor

tpetry commented Nov 9, 2025

@cosmastech I think there's a bug somewhere, hadn't enough time yet to look exactly why its happening. But all Eloquent hidden attributes are stripped so the factory is failing for a basic User model.

The implementation is using toArray() so that bug makes sense. As hidden attributes are stripped.

Confirmed, did a hacky test. Maybe there's a better implementation than this:

$query->fillAndInsert(
    $madeCollection->withoutAppends()->setHidden([])->toArray()
);

@tpetry
Copy link
Contributor

tpetry commented Nov 9, 2025

@cosmastech Question about the implementation. The insert() approach does not save relationships.

User::factory(10)->has(Article::factory()->count(3))->insert();

Is that an intended? Is it possible to fallback to single-row inserts for relationships? Or better, use multi-row inserts too?

@cosmastech
Copy link
Contributor Author

@cosmastech Question about the implementation. The insert() approach does not save relationships.

User::factory(10)->has(Article::factory()->count(3))->insert();

Is that an intended? Is it possible to fallback to single-row inserts for relationships? Or better, use multi-row inserts too?

I think it was intended, but not mentioned, that afterCreate or attaching relationships won't work. This is because this method doesn't know the rows to attach to. We could pull records based on maybe created_at timestamp (if there is one), or pull the highest auto-inc ID in the users and grab the 10 records, and then attach them. But it makes a lot of assumptions about the DB structure (it needs an auto-incrementing ID or a timestamp, time wasn't frozen or anything).

Is there any other way that you can envision where this would be possible?

I do not feel that falling back to single-row inserts is the right approach. I think it's better to have a function which operates consistently.

@cosmastech
Copy link
Contributor Author

cosmastech commented Nov 9, 2025

@cosmastech I think there's a bug somewhere, hadn't enough time yet to look exactly why its happening. But all Eloquent hidden attributes are stripped so the factory is failing for a basic User model.

The implementation is using toArray() so that bug makes sense. As hidden attributes are stripped.

Confirmed, did a hacky test. Maybe there's a better implementation than this:

$query->fillAndInsert(
    $madeCollection->withoutAppends()->setHidden([])->toArray()
);

I think this was fixed via #57670. Added a test in a PR here. #57722

@zepfietje
Copy link
Contributor

Looks like this is currently broken for array cast json columns, @cosmastech. The entire stored array gets wrapped in double quotes.

@cosmastech
Copy link
Contributor Author

Looks like this is currently broken for array cast json columns, @cosmastech. The entire stored array gets wrapped in double quotes.

@zepfietje can you give me a quick example of the model you built and the factory it fails for? 🙏

@zepfietje
Copy link
Contributor

Sure! Minimal reproduction:

<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
    /**
     * Run the migrations.
     */
    public function up(): void
    {
        Schema::create('foos', function (Blueprint $table) {
            $table->id();
            $table->json('bar');
            $table->timestamps();
        });
    }
};
<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;

class Foo extends Model
{
    /** @use HasFactory<\Database\Factories\FooFactory> */
    use HasFactory;

    protected function casts(): array
    {
        return [
            'bar' => 'array',
        ];
    }
}
<?php

namespace Database\Factories;

use Illuminate\Database\Eloquent\Factories\Factory;

/**
 * @extends \Illuminate\Database\Eloquent\Factories\Factory<\App\Models\Foo>
 */
class FooFactory extends Factory
{
    /**
     * Define the model's default state.
     *
     * @return array<string, mixed>
     */
    public function definition(): array
    {
        return [
            'bar' => ['bar'],
        ];
    }
}

Running \App\Models\Foo::factory()->create() yields ["bar"] in the database, while \App\Models\Foo::factory()->insert() gives "[\"bar\"]".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants