Create general module migration tasks (MGNLMIGRATION-55)

[MGNLMIGRATION-64] Create RenameNodesByMapTask Created: 21/Sep/12  Updated: 06/Dec/12  Resolved: 06/Dec/12

Status: Closed
Project: Migration 4.4 to 4.5 (closed)
Component/s: None
Affects Version/s: None
Fix Version/s: 1.2

Type: Sub-task Priority: Neutral
Reporter: Jozef Chocholacek Assignee: Jozef Chocholacek
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Template:
Date of First Response:

 Description   

A general Task which renames nodes according to a specification submitted in a Map<String,String>. In such map, key is absolute path of node to rename, value is either new name of the node (if the parent path stays the same), or an absolute path where the node has to be moved.



 Comments   
Comment by Jozef Chocholacek [ 05/Oct/12 ]

src/main/java/info/magnolia/migration/task/general/RenameNodesByMapTask.java

Comment by Jan Haderka [ 05/Dec/12 ]
  72         if (map==null) {
  73             log.warn("The map is empty, skipping.");
  74             return;
  75         }

so there is nothing to rename. I don't think it's worth of warning (specially since if map is set but empty you do not print anything. I would suggest to change this to debug level log only.

  83         if (from==null || to==null) {
  84             log.debug("When renaming nodes, 'from' nor 'to' mut not be null.");
  85             return;
  86         }

mut is a must really.

 134                     } catch (Exception ex) {
 135                         // TODO JCh: this should be handled better... maybe even rollback?
  • two things here - TODO and catching generic Exception. Both should be avoided. As for the comment in TODO - this is an install task so it should not make any permanent changes if it fails.

which brings me to another point:

 144             // save the change
 145             session.save();

If some other task fails and installation/update will be reexecuted later, this task has already made it's changes and persisted them. So as such there should be big warning about it in javadoc and comment next to save describing why we need to persist change immediately.

Comment by Jozef Chocholacek [ 06/Dec/12 ]

Code fixed.

Generated at Mon Feb 12 10:16:49 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.