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

Reorder with rename test #277

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

dantleech
Copy link
Contributor

This is a failing test, is it valid behavior?

@dbu
Copy link
Member

dbu commented Apr 17, 2013

i think renames are only put into phpcr on flush. so the var_dumps can be expected to return the old name.

but i fear the reordering is taking shortcuts with phpcr to decide how to order things. what the right thing is depends on the order in which the UoW is telling phpcr about the move and the orderBefore. semantically i would expect the dm to handle that case properly. so depending on the execution order it either needs to update pending reorders when a move is detected maybe?

@dbu dbu added the id label Feb 15, 2014
@dbu
Copy link
Member

dbu commented Mar 7, 2014

maybe #449 could fix this, or at least will have an influence on this test. want to check on the #449 branch?

@dbu
Copy link
Member

dbu commented Mar 14, 2014

i just investigated this a bit. from the way the children collection works, it is clear what happens. we ask phpcr for the list of children, but the rename is not put into the phpcr layer before the flush. if we want this to work, we would need to build a pending move table in the unit of work where explicit moves and move by assignment are tracked, and adjust the children mapping to be aware of that. even then, if you first would load the parent and then move the child, the children collection would still not see the move. i am afraid of the complexity we run into with such things. we have to figure out what the sane use cases are and stick to those.

$this->dm->clear();

$parent = $this->dm->find(null, '/functional/source');
$this->assertSame(array('second', 'first', 'third', 'fourth'), $this->getChildrenNames($parent->getChildren()));
Copy link
Member

Choose a reason for hiding this comment

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

i would expect the array to be 'second', 'renamed', 'third', 'forth' - that was the point of renaming... however, we get 'renamed' first as the reorder does not work, the child will still have the old name 'first' at the point when reorder is calculated.

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.

3 participants