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

Tests to show getAllLinearVersions() being 'stuck' on first value seen #84

Merged
merged 1 commit into from Feb 26, 2013
Merged

Tests to show getAllLinearVersions() being 'stuck' on first value seen #84

merged 1 commit into from Feb 26, 2013

Conversation

fazy
Copy link

@fazy fazy commented Feb 26, 2013

Using Jackalope-Jackrabbit it seems there is a problem with the version history response: after the first call to getAllLinearVersions(), subsequent calls always return the same value as the first call, even if the history has now changed.

The same problem doesn't seem to happen with getAllVersions(). I've created test cases (using new nodes for each) to show the working, getAllVersions(), and failing, getAllLinearVersions(), cases.

See https://groups.google.com/d/msg/symfony-cmf-devs/--3J2hbMsa8/bupTUgqeb-wJ

@dbu
Copy link
Member

dbu commented Feb 26, 2013

ok this is quite obviously wrong in jackalope: https://github.com/jackalope/jackalope/blob/master/src/Jackalope/Version/VersionHistory.php#L62 - we just cache and do never invalidate. i wonder how we would best invalidate. we should invalidate on every checkin/checkpoint operation. the tricky part is that if versioning is cascaded to children, so its not straight forward what to invalidate.

@fazy
Copy link
Author

fazy commented Feb 26, 2013

I think the difference between handling linearVersions and versions is the notifyHistoryChanged() method here: https://github.com/jackalope/jackalope/blob/master/src/Jackalope/Version/VersionHistory.php#L243

Jackalope\Version\VersionManager calls that method from checkin() and restore(). Can it be the same for linear versions?

@dbu
Copy link
Member

dbu commented Feb 26, 2013

ah so we already do invalidate! yes indeed, there is no reason not to kill the cache of linear versions as well, that should fix it. can you try and do a PR on jackalope if it fixes the problem?

btw the existing code does not handle the situation when creating versions is cascaded by the node type. for this we would need another method similar to ObjectManager::getCachedNode that gets all potential children of that node, by looping through the array and comparing the path string. but we do not need to fix that in the same moment, can also create an issue and fix it when somebody finds time to do that.

@fazy
Copy link
Author

fazy commented Feb 26, 2013

Ok, PR sent - jackalope/jackalope#148

I haven't looked at the cascading side of things.

dbu added a commit that referenced this pull request Feb 26, 2013
Tests to show getAllLinearVersions() being 'stuck' on first value seen
@dbu dbu merged commit d02e0ce into phpcr:master Feb 26, 2013
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.

None yet

2 participants