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

Fixed move bug #163

Closed
wants to merge 2 commits into from
Closed

Fixed move bug #163

wants to merge 2 commits into from

Conversation

dantleech
Copy link
Contributor

The UUID of a node is indexed by the nodes path. This commit fixes
an issue where this index is not removed, making the transport believe
that there is still a node existing at this path.

@wjzijderveld
Copy link

I found the same bug: #158
I also created a PR to test this with the phpcr-api-tests: phpcr/phpcr-api-tests#122

@dbu
Copy link
Member

dbu commented Jan 4, 2014

this has the same issue as #158 i guess, it does not handle moved children. cc20a28

i am kind of surprised why we have a path to uuid cache in the transport AND a uuid to path cache in the ObjectManager. do we really need both?

@dbu
Copy link
Member

dbu commented Jan 4, 2014

i merged #165 - @dantleech can you add a test about a child of the moved node and then call the method i added to transport to invalidate the cache and see if that fixes the problem?

@dbu
Copy link
Member

dbu commented Jan 4, 2014

this might be related to jackalope/jackalope#191

@dbu
Copy link
Member

dbu commented Jan 4, 2014

@dantleech can you check if jackalope/jackalope#200 matters for this or not?

@lsmith77
Copy link
Member

lsmith77 commented Jan 7, 2014

needs a rebase

The UUID of a node is indexed by the nodes path. This commit fixes
an issue where this index is not removed, making the transport believe
that there is still a node existing at this path.
- Added child test
- Really bizzare -- why does the child rename based on the parent?
@dantleech
Copy link
Contributor Author

Updated. I added a test for the child - but something very strange happens, as in the test case:

  • Create a node called /topic1
  • Add a child to that node called /topic1child
  • Move /topic1 to /topic2
  • Save the session

Now the following path is in the database: /topic2/topic2child -- havn't got time to investigate further today.

@lsmith77
Copy link
Member

lsmith77 commented Jan 8, 2014

@dbu
Copy link
Member

dbu commented Jan 8, 2014

ah, interesting. anybody on it right now or shall i have a look?

@dantleech
Copy link
Contributor Author

I'm working all day so feel free :)

@dbu dbu mentioned this pull request Jan 8, 2014
@dbu
Copy link
Member

dbu commented Jan 8, 2014

continued in #168 - thanks lukas, your tip made it simple to fix.

@dbu dbu closed this Jan 8, 2014
@dbu
Copy link
Member

dbu commented Jan 8, 2014

btw, i quickly checked if we might have a similar problem in phpcr-odm, but we only use str_replace in places where it makes sense.

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.

4 participants