[MAGNOLIA-5857] Provide a NodeVisitorTask and emphasize QueryTask as "unsafe" Created: 29/Jul/14  Updated: 15/Dec/21  Resolved: 30/Jul/14

Status: Closed
Project: Magnolia
Component/s: updatemechanism
Affects Version/s: 5.3.1
Fix Version/s: 5.2.8, 5.3.2

Type: Improvement Priority: Major
Reporter: Mikaël Geljić Assignee: Mikaël Geljić
Resolution: Fixed Votes: 0
Labels: query, upgrade
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File warn-example-install.png    
Issue Links:
Cloners
is cloned by MAGNOLIA-5859 Further prevent unsafe usages of Quer... Closed
Problem/Incident
causes MAGNOLIA-8255 QueryTask warning might be more subtle Open
dependency
is depended upon by MAGNOLIA-2595 Update tasks : streamline class names... Closed
is depended upon by MGNLDATA-257 Cannot start companyApp after update ... Closed
is depended upon by MGNLDATA-261 Migration diffs when migrating from 4... 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)
Date of First Response:

 Description   

Background
As part of work on migration diffs, it occurred that some upgrade tasks — those that have been implemented as QueryTasks, are very likely to cause incomplete upgrade issues.

1. In particular, this is the case when a module has a first delta that creates/modifies a node structure, and then a second delta which executes a QueryTask. There is no guarantee at all that the query returns non-persisted changes in that workspace.

2. We don't save sessions between deltas, only between modules.

Lights, Camera, Action!
1. We introduce a NodeVisitorTask, which in a similar fashion as the QueryTask enables to specify matching criteria and #operateOnNode().

  • We call for replacing QueryTask implementations with it.

2. We make it way more explicit that the QueryTask can only operate safely on persisted content.

  • We log a warning whenever executing a QueryTask.
  • In 5.4 we deprecate current constructor, and add a new one with a boolean flag in the following spirit: "I read about the limitations and I know what I'm doing". If that boolean is false (default), we will fire a TaskExecutionException and interrupt the upgrade (see followup ticket MAGNOLIA-5859).


 Comments   
Comment by Mikaël Geljić [ 30/Jul/14 ]

Attached an example outcome (data module) of the QueryTask emphasis on the install screen.

InstallContextImpl also logs to the console by default, so in addition we get this:

2014-07-30 11:43:28,147 WARN  info.magnolia.module.InstallContextImpl           : > ChangeAvailabilityRuleClassesTask is a QueryTask. Mind that JCR queries may NOT account for unsaved changes in current session, e.g. from previous update tasks. To avoid incomplete updates, please use a NodeVisitorTask.
2014-07-30 11:43:28,165 WARN  info.magnolia.module.InstallContextImpl           : > MigrateAvailabilityRulesTask is a QueryTask. Mind that JCR queries may NOT account for unsaved changes in current session, e.g. from previous update tasks. To avoid incomplete updates, please use a NodeVisitorTask.
2014-07-30 11:43:28,170 WARN  info.magnolia.module.InstallContextImpl           : > MigrateJcrPropertiesToContentConnectorTask is a QueryTask. Mind that JCR queries may NOT account for unsaved changes in current session, e.g. from previous update tasks. To avoid incomplete updates, please use a NodeVisitorTask.
2014-07-30 11:43:28,223 WARN  info.magnolia.module.InstallContextImpl           : > MoveActionNodeTypeRestrictionToAvailabilityTask is a QueryTask. Mind that JCR queries may NOT account for unsaved changes in current session, e.g. from previous update tasks. To avoid incomplete updates, please use a NodeVisitorTask.
2014-07-30 11:43:28,378 WARN  info.magnolia.module.InstallContextImpl           : > MigrateEditDataActionSubAppMappingConfiguration is a QueryTask. Mind that JCR queries may NOT account for unsaved changes in current session, e.g. from previous update tasks. To avoid incomplete updates, please use a NodeVisitorTask.
Comment by Roman Kovařík [ 21/Aug/14 ]
private static final String QUERY_SCOPE_WARNING = "%s is a QueryTask. "
            + "Mind that JCR queries may NOT account for unsaved changes in current session, e.g. from previous update tasks. "
            + "To avoid incomplete updates, please use a NodeVisitorTask.";

This outputs warning after installation on any executed QueryTask. We would need to replace usage of all QueryTask's in all modules to get rid of this even if we know that's safe. Moreover, some modules depends on lower versions of core where the NodeVisitorTask is not available.

Generated at Mon Feb 12 04:09:02 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.