Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.0] Single entity flush #1139

Closed
wants to merge 1 commit into from

Conversation

goetas
Copy link
Member

@goetas goetas commented Sep 18, 2014

The current flush behavior seems to be inconsistent or not well documented.

If i have understood the documentation in the correct way, flush($entity) should flush the passed entity (and its related associations/entities), and leave untouched the remaining object graph.

The current behavior seems to be: flush all new objects, flush all $entity "updates", clear all "todo" stored into current UnitOfWork state.

@goetas
Copy link
Member Author

goetas commented Sep 18, 2014

Implemented!
There is a possible BC (or a previous wrong test...) here f4b9fca#diff-c8b2f0edff9d7ec04a82332c76136cdaR1147

@beberlei
Copy link
Member

Why does it need changing? The behavior is documented this way. I am not sure this is a problem to flush the new entities as well, however your solution requires yet another state array inside UoW, making the management much more complex.

@goetas
Copy link
Member Author

goetas commented Sep 18, 2014

I have this usecase:

class City {
    public $id; // pk
    public $name;
}

// 1st case
$rome = new City();
$rome->name = "rome";
$em->persist($rome);

$milan = new City();
$milan->name = "milan";
$em->persist($milan);

$em->flush($rome);
// "milan" is flushed to the database... why?

// this case is in contradiction with previous one
$rome = findByName('rome');
$rome->name = "rome1";
$em->persist($rome);

$milan = findByName('milan');
$milan->name = "mailan1";
$em->persist($milan);

$em->flush($rome);
// now, "milan" is not flushed to the database... 

2nd case

class City {
    public $id; // pk
    public $name;
}
class Building {
    public $id;  // pk
    public $name;
    public $city;  // not null
}
function findOrCreateCity($name){
    $city  = findCityByName($name);
    if ($city){
        $city = new City();
        $city->name = $name;
        $em->persist($city);
        $em->flush($city);
    }
    return $city;
}

$building = new Building();
$building->name = 'Palace';

$em->persist($building);
// note, $building->city has NOT NULL constraint, 
// and $building has been already persisted, but not flushed
$building->city = findOrCreateCity('rome');
// note, flush() call is not present here

Building::$city can not be null, and this causes a database FK check failure.

This happens when findOrCreateCity calls flush on $city, but this causes the flush of $building too.
Looking just at the findOrCreateCity function, it seems good and safe, but due to flush behavior, all new entities are always flushed.

@goetas
Copy link
Member Author

goetas commented Sep 18, 2014

The added complexity to UoW of course is a bad thing, and i will try to find a better solution, but this does not change my point of view in flush behavior.

@goetas
Copy link
Member Author

goetas commented Sep 18, 2014

Looking at this test, https://github.com/doctrine/doctrine2/blob/master/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php#L1056

$userC has been (implicitly) flushed at line 1075, but later at line 1079 does not happen the same thing.

@Ocramius Ocramius changed the title Single entity flush [3.0] [RFC] Single entity flush Jan 13, 2015
@Ocramius
Copy link
Member

Here are my thoughts on this:

  • yes, current behavior is inconsistent
  • the single entity flush was never meant to guarantee insulation of the flush operation
  • the single entity flush is a mere performance optimization, it shouldn't change behavior of the UnitOfWork

I think we should just drop this API completely for 3.x: instead, what we could eventually do is providing some API for manually populating UnitOfWork changesets.

@goetas
Copy link
Member Author

goetas commented Sep 27, 2016

Hi 😄

I have rebased this on top of the latest master.

Any chance to fix this flush inconsistency in 3.0 ?

@goetas goetas changed the title [3.0] [RFC] Single entity flush [3.0] Single entity flush Sep 27, 2016
@sstok
Copy link

sstok commented Nov 6, 2016

Related to #6118

@Ocramius Ocramius self-assigned this Jun 21, 2017
@Ocramius
Copy link
Member

After much discussion, EntityManager#flush($entity) is going to disappear in ORM 3.x, replaced by EntityManager#flush(), so this patch is no longer valid.

@goetas
Copy link
Member Author

goetas commented Jun 21, 2017

Is there going to be a way to flush a single object, or the feature has been just removed?

@Ocramius
Copy link
Member

@goetas just removed - see #6118 for further info. See #5550 about possible future development of direct unit-of-work-less operations

@goetas
Copy link
Member Author

goetas commented Jun 21, 2017

Nevermind, just checked the related tickets

@goetas
Copy link
Member Author

goetas commented Jun 21, 2017

Arg.... You are too fast :))

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

Successfully merging this pull request may close these issues.

4 participants