[MGNLACTIVATION-99] Unpublishing right after publication can result in exception Created: 18/Aug/14  Updated: 18/Aug/15  Resolved: 26/Jan/15

Status: Closed
Project: Activation
Component/s: None
Affects Version/s: 5.0
Fix Version/s: 5.3.2

Type: Bug Priority: Major
Reporter: Daniel Lipp Assignee: Evzen Fochr
Resolution: Fixed Votes: 0
Labels: asynch-issue, publishing, support, unizh, uzh
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: JPEG File Screen Shot 2014-08-18 at 13.28.18.jpg    
Issue Links:
dependency
is depended upon by MGNLXAA-88 Unpublishing right after publication ... Closed
relation
is related to MGNLUI-3245 The action "unpublish" should only be... Closed
is related to MGNLUI-3107 Publishing deletion right after delet... Closed
is related to MGNLUI-3230 Publishing and long running task defects Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

Publishing a page tree might take some time. If this is the case and you're quick enough to trigger an unpublication of the same page tree, you might get an exception because Magnolia tries to deactivate something that wasn't properly activated yet (tested with ee 5.2.7 and 5.3.2). We should check how to prevent running into this situation. e.g. introduce a kind of dependencies between asynchronously executed action (only start second when the first is indeed finished).

Solution of this problem needs public API changes.
Proposed solution: Node need to know if it is part of recursive activation (only sub nodes, not parent) and if yes, check activation status of node on author. If node on Author is deactivated we should quietly skip activation of this node, because we know over recursiveActivation correctly activated it (no exception and break of recursion) and somebody come after and deactivated it. For it you will need to change Syndicator interface.



 Comments   
Comment by Daniel Lipp [ 18/Aug/14 ]

MGNLUI-3106 would happen less frequently once 3104 has been fixed.

Comment by Evzen Fochr [ 27/Nov/14 ]

This is first working implementation. I'll continue in separate branch to implement that commands that are on independent part of tree wont be blocked in queue and be executed simultaneously.

Comment by Roman Kovařík [ 10/Dec/14 ]

There are three branches, which one should be reviewed?

Comment by Jan Haderka [ 10/Dec/14 ]

The code:

  • When checking skipPath you check simply for startsWith. This is common mistake that will result in path that is superset of skipped path to be skipped as well, e.g. if skipPath=/skip/me it will skip also node at path /skip/meNOT
    The test:
  • the TestSimpleSydicator is not really generic "testSS". More explicit name would help later to make anyone tempted to reuse it understand what it does. Something like ThrowExceptionWhileActivatingA_B_ANodeSyndicator
  • since in your fix you are manipulating parent nodes of activated node, you need extra test to cover also the case when activating node just under the root (since root node that will be obtained as parent tends to behave differently in certain corner cases)
  • test for names that are superset of failing name is called for (see above comment regarding code issue)
Comment by Jan Haderka [ 10/Dec/14 ]

Make sure any changes you do are maximally localised - do not change what this class is passing onto the syndicator.

Comment by Jan Haderka [ 17/Dec/14 ]

Could you please describe here why the issue was reopened? What was the problem it caused or what was not fixed by the original fix?

Comment by Evzen Fochr [ 17/Dec/14 ]

When deactivated parent node of node under activation was more then one level up from currently activated node, error handing was incorrectly reporting one error per each level.

Comment by Jan Haderka [ 17/Dec/14 ]

Seems little inefficient to retrieve session and node to get the parent every iteration of the loop and throw it away immediately. I would suggest to either keep those so you don't have to obtain it again and again. Specially since you do the same twice in each run (first in the while condition check and secondly inside of the loop.

Comment by Jan Haderka [ 12/Jan/15 ]

Copy paste error? If you had a unit test for newly added if condition, you would have noticed that it can't pass.

Comment by Jan Haderka [ 12/Jan/15 ]

Don't forget to squash it all when merging into master.

Comment by Espen Jervidalo [ 21/Jan/15 ]

Bundle is still pointing to the old version.

Comment by Evzen Fochr [ 26/Jan/15 ]

https://git.magnolia-cms.com/gitweb/?p=modules/activation.git;a=commit;h=c602567c856b521053aa0d4729d16ea5c277065b

Generated at Sun Feb 11 22:59:40 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.