Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Turn persist_once on by default
  • Loading branch information
@Seldaek
Seldaek committed Jul 29, 2013
1 parent 2ed4a52 commit 6c5da4d
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
4 changes: 2 additions & 2 deletions README.md
View file
Expand Up @@ -65,8 +65,8 @@ with the following keys:
to null to disable (defaults to 1)
- logger: a callable or Psr\Log\LoggerInterface object that will receive progress
information during the loading of the fixtures
- persist_once: only persist objects once if multiple files are passed, by default
objects are persisted after each file
- persist_once: if set to false, objects are persisted after every file is parsed,
by default everything is persisted once at the end

### Detailed Usage ###

Expand Down
4 changes: 2 additions & 2 deletions src/Nelmio/Alice/Fixtures.php
View file
Expand Up @@ -30,7 +30,7 @@ public function __construct($container, array $defaultOptions = array(), array $
'providers' => array(),
'seed' => 1,
'logger' => null,
'persist_once' => false,
'persist_once' => true,
);
$this->defaultOptions = array_merge($defaults, $defaultOptions);
$this->processors = $processors;
Expand All @@ -47,7 +47,7 @@ public function __construct($container, array $defaultOptions = array(), array $
* - seed: a seed to make sure faker generates data consistently across
* runs, set to null to disable
* - logger: a callable or Psr\Log\LoggerInterface object that will receive progress information
* - persist_once: only persist objects once if multiple files are passsed
* - persist_once: if set to false, objects are persisted after every file is parsed and not once at the end
* @param array $processors optional array of ProcessorInterface instances
*/
public static function load($files, $container, array $options = array(), array $processors = array())
Expand Down

9 comments on commit 6c5da4d

@paulandrieux
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change the default behavior of Alice, it would be great to throw an help message if an error like
"Entity of type My\Bundle\Entity\Foo has identity through a foreign entity My\Bundle\Entity\Bar, however t his entity has no identity itself." happens.

@Seldaek
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify how and why you get this error? I don't really see how it makes a difference to persist once or several times, but I'm probably missing something.

@paulandrieux
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I defined a first file with this structure:

My\Bundle\Entity\Foo:
    foo1:
        title: hello
        description: world

and a second file:

My\Bundle\Entity\Bar
    bar1:
        title: bar1
        foo: @foo1

Before your default option change, a persist was performed between this two files, and the relation was successfully persisted.
Now, the error "Entity of type My\Bundle\Entity\Foo has identity through a foreign entity My\Bundle\Entity\Bar, however t his entity has no identity itself." is thrown because Bar references a non persisted Foo object, and FK cannot be set.

I would be surprised that this bug was isolated, references are one of the great feature of Alice ;)

@Seldaek
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird though, I thought doctrine could deal with that fine by saving the Foo object first. Any thoughts on this @marekkalnik? Seems like you told me the fix was to allow cross-file references, and now it seems it actually prevents them. I'm confused :)

@marekkalnik
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check it out in few hours. I have tested and everything seemed to be ok. I'll get back to you.

@marekkalnik
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I had to work late and I couldn't test it. May I suggest that you change the behavior to "false" to avoid regression and I'll continue to test it? I gues that it is related to the way Doctrine handles PostgresSQL ids (I tested on a Postgres project, and the ids are automatically injected from sequences), but I have had no time to re-test it and make sure it's not an error in my code.

@Seldaek
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, no worries. I guess I'll just revert and tag a new release, then we can see about this later if it's fixable.

@Seldaek
Copy link
Member Author

@Seldaek Seldaek commented on 6c5da4d Aug 1, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.5.1 tagged /cc @baldurrensch

@marekkalnik
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I've tested a bit, and I can't seem to reproduce the error. I guess it depends on Doctrine version (I'm using 2.3). As for the feature itself turning persist_once on or off does not change anything, so I guess the "fix" was coincidentally somewhere in my code. Turns out it became just a minor performance improvement for those that use newer doctrine versions :(

Please sign in to comment.