[
https://issues.apache.org/jira/browse/SLING-4753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15769366#comment-15769366
]
Felix Meschberger edited comment on SLING-4753 at 12/22/16 7:37 AM:
--------------------------------------------------------------------
Hmm, I am a bit concerned, yet, of course, late in the game.
IIRC we explicitly did *not* commit the changes to the Tenant before calling
the customers to abort/roll-back the creation in case of problems. This change
of course prevents that from happening.
I am also a bit concerned with the reasoning behind this change: AEM's
PageManager.createPage method presumably clearing any changes before acting. I
am not sure, we really want to support such questionable behaviour.
Also, while I agree that is not clearly spec-ed that changes are not persisted
*before* calling the customizers, there is some indication in that:
* It is clearly stated in the TenantManager.setup method that changes are
persisted *before* the method returns
* It is clearly stated with the TenantCustomer that ResourceResolver.commit
must not be called
Particularly from the second statement we could deduce, that also
ResourceResolver.revert should not be called, hence calling Session.refresh is
equally unexpected and undesired.
Plus, even if the original creation is persisted, the behaviour could even
revert any changes that a previous customizer applied. I really think, we
should fix the problem not the symptom.
As such, the longer I think about it, the longer I have the impression, that
this change should be reverted.
Last but not least: Do we really close issues before a release has been cut ?
/cc [~marett], [~amitgupt]
was (Author: fmeschbe):
Hmm, I am a bit concerned, yet, of course, late in the game.
IIRC we explicitly did *not* commit the changes to the Tenant before calling
the customers to abort/roll-back the creation in case of problems. This change
of course prevents that from happening.
I am also a bit concerned with the reasoning behind this change: AEM's
PageManager.createPage method presumably clearing any changes before acting. I
am not sure, we really want to support such questionable behaviour.
Also, while I agree that is not clearly spec-ed that changes are not persisted
*before* calling the customizers, there is some indication in that:
* It is clearly stated in the TenantManager.setup method that changes are
persisted *before* the method returns
* It is clearly stated with the TenantCustomer that ResourceResolver.commit
must not be called
Particularly from the second statement we could deduce, that also
ResourceResolver.revert should not be called, hence calling Session.refresh is
equally unexpected and undesired.
As such, the longer I think about it, the longer I have the impression, that
this change should be reverted.
Last but not least: Do we really close issues before a release has been cut ?
/cc [~marett], [~amitgupt]
> Commit the Resource Resolver before passing it to Tenant Customizers for
> setting up their own customizations
> ------------------------------------------------------------------------------------------------------------
>
> Key: SLING-4753
> URL: https://issues.apache.org/jira/browse/SLING-4753
> Project: Sling
> Issue Type: Bug
> Components: Extensions
> Affects Versions: Tenant 1.1.0
> Reporter: Agraj Mangal
> Assignee: Amit Gupta
> Fix For: Tenant 1.1.0
>
>
> We should commit the Resource Resolver after creating the Tenant Resource and
> before passing it on to the Tenant Customizers.
> One possible issue is that one of the Tenant Customizers calls some APIs like
> PageManager##createPage that does a session.refresh() and rollbacks all the
> un-committed changes on the resolver so far. That could also include the
> tenant resource itself.
> Ideally the TenantCustomizers should not call commit on the resolver and let
> TenantProvider commit the changes, but it would be a good protection against
> all such cases where we could prevent the tenant resource from getting
> modified if the TenantCustomizer failed and tried to refresh the session.
> We are experiencing this issue in
> https://jira.corp.adobe.com/browse/MAC-25410
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
