create a copy utility and remove the need for clone and copy constructors in transforms/actions - #8031
Merged
Conversation
…ata. fixes apache#8022 Object.clone() is shallow, so a cloned ITransformMeta or IAction keeps sharing every list, map and nested value object with the original. The pipeline editor relies on cloned snapshots in two places and the sharing broke both: - editTransform() decides whether a dialog changed anything by comparing the XML of a snapshot taken before the dialog opened against one taken after OK. The snapshot shared the live lists, so dialogs that refill a list in place - Add constants, the Data Grid Meta tab, Insert/Update, Update, Table output and around twenty more - serialized identically and their edits were silently not marked as changed. The reported workaround, moving the transform afterwards, worked because setLocation() sets the flag directly. - The same snapshot is stored as the undo entry, so undo either did nothing or rolled back only the properties that survived the shallow copy, leaving the transform in a state the user never had. BaseTransformMeta.clone() and ActionBase.clone() now deep copy the state described by the @HopMetadataProperty annotations, which is exactly the state that gets persisted, compared and restored. Live references - other transforms, metadata providers, caches - stay shared, as the javadoc on clone() intends. One generic implementation makes the hand written copies redundant: 211 clone() overrides and 122 copy constructors are removed. The 12 overrides that remain each do something the generic copy cannot: a covariant return required by ICsvInputAwareMeta, null defaulting, unannotated runtime state, or a copy constructor that normalizes a value. This also fixes AvroOutputMeta, CalculatorMeta and MergeJoinMeta, whose hand written clone() silently dropped most of their fields, and removes the assignment in ActionMail.clone() that wrote the copy's list back onto the original. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Transform and action dialogs are not modal, so the canvas stays live while one is open. Every dialog captures the changed flag when it is constructed and restores it in cancel(). A transform's entire dirty state was a single boolean on the inner ITransformMeta, and TransformMeta.setLocation() wrote to that same boolean, so dragging a transform while its dialog was open and then pressing Cancel discarded the move: the transform stayed at its new position while the pipeline reported no changes, and the move was lost on close. TransformMeta and ActionMeta now keep their own flag for the state they own - position, number of copies, row distribution - and hasChanged() reports the two together. A dialog's Cancel reaches only the transform's own settings, so it can still roll back an edit in progress without touching what the canvas did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please add a meaningful description for your change here
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean install apache-rat:checkto make sure basic checks pass. A more thorough check will be performed on your pull request automatically.git rebase -i.addresses #123), if applicable.To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.