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

Migration commits repository transaction wrong user #76

Closed
wizhippo opened this issue Oct 9, 2016 · 9 comments
Closed

Migration commits repository transaction wrong user #76

wizhippo opened this issue Oct 9, 2016 · 9 comments
Assignees
Labels
Milestone

Comments

@wizhippo
Copy link
Contributor

wizhippo commented Oct 9, 2016

On https://github.com/kaliop-uk/ezmigrationbundle/blob/master/Core/MigrationService.php#L221 this can cause an exception as the commit may need read access to objects. At this point the user could be anonymous and not have read rights as the RepositoryExecuter resets the user.

This can be replicated by running the following migration on a clean install

https://gist.github.com/wizhippo/d549e03a6615f05122763f259ebfd383

Guess repository transactions are user sensitive.

In this case during the commit the PublishVersion signal dispatches a cache clear which in turn tries to load the content info. Since we are now anonymous it fails.

@wizhippo wizhippo changed the title Migration commits repositry transaction wrong user Migration commits repository transaction wrong user Oct 9, 2016
@gggeek
Copy link
Member

gggeek commented Oct 10, 2016

Thx for the test case - I think the pointer to line 22 in MigrationService might be wrong :-)

@gggeek gggeek added the bug label Oct 10, 2016
@wizhippo
Copy link
Contributor Author

Corrected. Should have been line 221

@gggeek
Copy link
Member

gggeek commented Oct 11, 2016

Your snippet does work in 5.4 and lower!

It seems like the concept of what a 'repository transaction' is, and what is hooked up to it, might have changed across versions of eZ5/eZ6.
As if not having support for nested transactions was not bad enough...

The good news is: you can use the existing flag to omit transactions, and your migration will succeed.

The bad news is: if we want to fix this in a better way, we would need to set the repository-user per-migration, in the migration service, and stop doing it per-step, in the step executors. It will be hard to achieve that without any BC breackage. I wonder which release would be best suited to introduce this change (3 or maybe 4 ?)

@gggeek
Copy link
Member

gggeek commented Oct 11, 2016

Btw, I created an issue in eZ: https://jira.ez.no/browse/EZP-26407

@gggeek
Copy link
Member

gggeek commented Oct 13, 2016

ps: "it seems" that the above scenario might apply as well when there are legacy workflows involved, on an ez 5.4 kernel.
So definitely something to tackle...

@wizhippo
Copy link
Contributor Author

Thanks for looking into this more and posting the issue on jira

@gggeek
Copy link
Member

gggeek commented Oct 14, 2016

Most likely related to #78

@gggeek
Copy link
Member

gggeek commented Oct 15, 2016

I opted for an ugly but minor change: 'become admin' just around the call to repo->commit. This should fix the symptoms for the moment.
For the future: make it possible to choose the 'admin' user to use

@gggeek gggeek added this to the 2.4 milestone Oct 15, 2016
@gggeek gggeek self-assigned this Oct 15, 2016
@gggeek
Copy link
Member

gggeek commented Oct 16, 2016

Fixed in 2.4 & 3.0beta2

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

No branches or pull requests

2 participants