This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new bb81aa4e5d fixes #7700 (#7702)
bb81aa4e5d is described below
commit bb81aa4e5ddbee0ac7189eaeeaf9f42c6741ae99
Author: Bart Maertens <[email protected]>
AuthorDate: Fri Jul 31 11:13:22 2026 +0200
fixes #7700 (#7702)
* recognise renamed transforms and actions in the visual diff, fixes #7700
A renamed transform or action was reported as one that had been removed
and another that had been added, because the visual diff matches the two
versions on name alone and nothing else identifies it.
What is left of an entry once its name and position are taken out is now
used to pair up the names that exist on only one side. A pair found that
way is a rename and is reported as changed. The hops around it are looked
up under the new name as well, so they are no longer dragged into the
diff along with it.
The position is left out of that identity regardless of the
ignore-position option: whether a move counts as a change is a separate
question from whether these are the same transform.
A rename is only recognised when it is all that happened to the entry, a
move aside. Renaming and editing in one commit still reads as a removal
plus an addition.
* name the visual diff tabs after the file, #7700
Both versions of a file are read out of a git blob and so carry no
filename, which left PipelineMeta and WorkflowMeta falling back to the
name stored inside the file for the tab title. That is rarely what the
rest of the GUI calls the file: a pipeline saved as bogus-pipeline.hpl
but never renamed inside showed up as "New pipeline".
The title is taken from the path being compared instead. The filename is
deliberately not set on the metadata: these are throwaway views of an old
revision and should not look like the file itself.
---
.../modules/ROOT/pages/hop-gui/hop-gui-git.adoc | 4 +
.../main/java/org/apache/hop/git/GitGuiPlugin.java | 41 ++++--
.../src/main/java/org/apache/hop/git/HopDiff.java | 152 +++++++++++++++++++--
.../git/info/GitInfoExplorerFileTypeHandler.java | 41 ++++--
.../org/apache/hop/git/util/FileTypeUtils.java | 14 ++
.../test/java/org/apache/hop/git/HopDiffTest.java | 117 ++++++++++++++--
.../org/apache/hop/git/util/FileTypeUtilsTest.java | 41 ++++++
7 files changed, 366 insertions(+), 44 deletions(-)
diff --git a/docs/hop-user-manual/modules/ROOT/pages/hop-gui/hop-gui-git.adoc
b/docs/hop-user-manual/modules/ROOT/pages/hop-gui/hop-gui-git.adoc
index d39acd7dff..1c23bdf3c8 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/hop-gui/hop-gui-git.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/hop-gui/hop-gui-git.adoc
@@ -93,6 +93,10 @@ image::hop-gui/git-visual-diff.png[Git visual
diff,width="65%"]
A transform or action that was only moved on the canvas is not marked as
changed.
Uncheck "Ignore position in the visual diff" in the configuration options
below to have a move count as a change again.
+A transform or action that was renamed is marked as changed, rather than as
one that was removed and another that was added.
+The hops around it are left alone.
+This is recognised as long as the rename is all that happened to it, a move
aside: renaming and editing in the same commit still shows up as a removal plus
an addition, because there is nothing left to recognise it by.
+
== Git Configuration options
There are a few global configuration options available for the git GUI plugin.
diff --git
a/plugins/misc/git/src/main/java/org/apache/hop/git/GitGuiPlugin.java
b/plugins/misc/git/src/main/java/org/apache/hop/git/GitGuiPlugin.java
index 694c94f3f4..d60a046b64 100644
--- a/plugins/misc/git/src/main/java/org/apache/hop/git/GitGuiPlugin.java
+++ b/plugins/misc/git/src/main/java/org/apache/hop/git/GitGuiPlugin.java
@@ -53,6 +53,7 @@ import org.apache.hop.git.info.GitInfoExplorerFileTypeHandler;
import org.apache.hop.git.model.UIFile;
import org.apache.hop.git.model.UIGit;
import org.apache.hop.git.model.VCS;
+import org.apache.hop.git.util.FileTypeUtils;
import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.ui.core.dialog.EnterSelectionDialog;
@@ -1041,12 +1042,21 @@ public class GitGuiPlugin
new PipelineMeta(xmlStreamNew, hopGui.getMetadataProvider(),
hopGui.getVariables());
boolean ignorePosition =
GitConfigSingleton.getConfig().isIgnoringPositionInDiff();
+ Map<String, String> renamed =
+ HopDiff.detectTransformRenames(pipelineMetaOld, pipelineMetaNew);
+ Map<String, String> renamedBack =
+ HopDiff.detectTransformRenames(pipelineMetaNew, pipelineMetaOld);
+
+ pipelineMetaOld =
+ HopDiff.compareTransforms(
+ pipelineMetaOld, pipelineMetaNew, true, ignorePosition, renamed);
pipelineMetaOld =
- HopDiff.compareTransforms(pipelineMetaOld, pipelineMetaNew, true,
ignorePosition);
- pipelineMetaOld = HopDiff.comparePipelineHops(pipelineMetaOld,
pipelineMetaNew, true);
+ HopDiff.comparePipelineHops(pipelineMetaOld, pipelineMetaNew, true,
renamed);
+ pipelineMetaNew =
+ HopDiff.compareTransforms(
+ pipelineMetaNew, pipelineMetaOld, false, ignorePosition,
renamedBack);
pipelineMetaNew =
- HopDiff.compareTransforms(pipelineMetaNew, pipelineMetaOld, false,
ignorePosition);
- pipelineMetaNew = HopDiff.comparePipelineHops(pipelineMetaNew,
pipelineMetaOld, false);
+ HopDiff.comparePipelineHops(pipelineMetaNew, pipelineMetaOld, false,
renamedBack);
pipelineMetaOld.setPipelineVersion(CONST_GIT + commitIdOld);
pipelineMetaNew.setPipelineVersion(CONST_GIT + commitIdNew);
@@ -1056,7 +1066,7 @@ public class GitGuiPlugin
pipelineMetaOld.setName(
String.format(
CONST_S_S_S,
- pipelineMetaOld.getName(),
+ FileTypeUtils.getDiffName(filename, pipelineMetaOld.getName()),
git.getShortenedName(commitIdOld),
git.getShortenedName(commitIdNew)));
pipelineMetaOld.setNameSynchronizedWithFilename(false);
@@ -1064,7 +1074,7 @@ public class GitGuiPlugin
pipelineMetaNew.setName(
String.format(
CONST_S_S_S,
- pipelineMetaNew.getName(),
+ FileTypeUtils.getDiffName(filename, pipelineMetaNew.getName()),
git.getShortenedName(commitIdNew),
git.getShortenedName(commitIdOld)));
pipelineMetaNew.setNameSynchronizedWithFilename(false);
@@ -1094,12 +1104,19 @@ public class GitGuiPlugin
new WorkflowMeta(xmlStreamNew, hopGui.getMetadataProvider(),
hopGui.getVariables());
boolean ignorePosition =
GitConfigSingleton.getConfig().isIgnoringPositionInDiff();
+ Map<String, String> renamed =
HopDiff.detectActionRenames(workflowMetaOld, workflowMetaNew);
+ Map<String, String> renamedBack =
+ HopDiff.detectActionRenames(workflowMetaNew, workflowMetaOld);
+
+ workflowMetaOld =
+ HopDiff.compareActions(workflowMetaOld, workflowMetaNew, true,
ignorePosition, renamed);
workflowMetaOld =
- HopDiff.compareActions(workflowMetaOld, workflowMetaNew, true,
ignorePosition);
- workflowMetaOld = HopDiff.compareWorkflowHops(workflowMetaOld,
workflowMetaNew, true);
+ HopDiff.compareWorkflowHops(workflowMetaOld, workflowMetaNew, true,
renamed);
+ workflowMetaNew =
+ HopDiff.compareActions(
+ workflowMetaNew, workflowMetaOld, false, ignorePosition,
renamedBack);
workflowMetaNew =
- HopDiff.compareActions(workflowMetaNew, workflowMetaOld, false,
ignorePosition);
- workflowMetaNew = HopDiff.compareWorkflowHops(workflowMetaNew,
workflowMetaOld, false);
+ HopDiff.compareWorkflowHops(workflowMetaNew, workflowMetaOld, false,
renamedBack);
workflowMetaOld.setWorkflowVersion(CONST_GIT + commitIdOld);
workflowMetaNew.setWorkflowVersion(CONST_GIT + commitIdNew);
@@ -1109,7 +1126,7 @@ public class GitGuiPlugin
workflowMetaOld.setName(
String.format(
CONST_S_S_S,
- workflowMetaOld.getName(),
+ FileTypeUtils.getDiffName(filename, workflowMetaOld.getName()),
git.getShortenedName(commitIdOld),
git.getShortenedName(commitIdNew)));
workflowMetaOld.setNameSynchronizedWithFilename(false);
@@ -1117,7 +1134,7 @@ public class GitGuiPlugin
workflowMetaNew.setName(
String.format(
CONST_S_S_S,
- workflowMetaNew.getName(),
+ FileTypeUtils.getDiffName(filename, workflowMetaNew.getName()),
git.getShortenedName(commitIdNew),
git.getShortenedName(commitIdOld)));
workflowMetaNew.setNameSynchronizedWithFilename(false);
diff --git a/plugins/misc/git/src/main/java/org/apache/hop/git/HopDiff.java
b/plugins/misc/git/src/main/java/org/apache/hop/git/HopDiff.java
index 3fe70bb134..97bf60361f 100644
--- a/plugins/misc/git/src/main/java/org/apache/hop/git/HopDiff.java
+++ b/plugins/misc/git/src/main/java/org/apache/hop/git/HopDiff.java
@@ -18,10 +18,14 @@
package org.apache.hop.git;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import java.util.regex.Pattern;
import org.apache.hop.core.exception.HopException;
+import org.apache.hop.core.logging.LogChannel;
import org.apache.hop.pipeline.PipelineHopMeta;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.transform.TransformMeta;
@@ -46,6 +50,14 @@ public class HopDiff {
private static final Pattern POSITION_TAGS =
Pattern.compile("[ \\t]*<(xloc|yloc)>.*?</\\1>\\r?\\n?");
+ /**
+ * The name of the transform or action itself. Both TransformMeta and
ActionMeta write it as the
+ * first name tag of the element, ahead of anything the plugin serializes,
so only the first
+ * occurrence may be replaced -- a transform's own fields carry name tags of
their own.
+ */
+ private static final Pattern NAME_TAG =
+ Pattern.compile("[ \\t]*<name(\\s*/>|>.*?</name>)\\r?\\n?");
+
private HopDiff() {}
/**
@@ -64,18 +76,112 @@ public class HopDiff {
return xml1.equals(xml2);
}
+ /**
+ * What is left of a transform or action once the two things a rename is
allowed to alter -- its
+ * name and where it sits -- are taken out. Two entries carrying the same
remainder under
+ * different names are the same one, renamed.
+ *
+ * <p>The position goes regardless of the ignore-position option: that
option decides whether a
+ * move counts as a change, which is a different question from whether these
are the same
+ * transform.
+ */
+ private static String identity(String xml) {
+ return removePosition(NAME_TAG.matcher(xml).replaceFirst(""));
+ }
+
+ /**
+ * Pair the names that exist on only one side against each other. A name
present on both sides is
+ * matched by the regular comparison and is never a rename; of what remains,
two entries with the
+ * same identity are a rename. Each candidate is claimed once, so identical
transforms renamed in
+ * the same commit pair off one by one rather than all collapsing onto the
first.
+ *
+ * @return the names in the first version mapped to what they are called in
the second
+ */
+ private static Map<String, String> pairRenames(
+ Map<String, String> identities1, Map<String, String> identities2) {
+ Map<String, String> renames = new LinkedHashMap<>();
+ Set<String> claimed = new HashSet<>();
+
+ for (Map.Entry<String, String> entry : identities1.entrySet()) {
+ if (identities2.containsKey(entry.getKey())) {
+ continue;
+ }
+ for (Map.Entry<String, String> candidate : identities2.entrySet()) {
+ if (identities1.containsKey(candidate.getKey()) ||
claimed.contains(candidate.getKey())) {
+ continue;
+ }
+ if (entry.getValue().equals(candidate.getValue())) {
+ renames.put(entry.getKey(), candidate.getKey());
+ claimed.add(candidate.getKey());
+ break;
+ }
+ }
+ }
+ return renames;
+ }
+
+ private static String renamedTo(String name, Map<String, String> renames) {
+ return renames.getOrDefault(name, name);
+ }
+
+ /** Transforms renamed between the two versions, by name in the first
version. */
+ public static Map<String, String> detectTransformRenames(
+ PipelineMeta pipelineMeta1, PipelineMeta pipelineMeta2) {
+ return pairRenames(transformIdentities(pipelineMeta1),
transformIdentities(pipelineMeta2));
+ }
+
+ private static Map<String, String> transformIdentities(PipelineMeta
pipelineMeta) {
+ Map<String, String> identities = new LinkedHashMap<>();
+ for (TransformMeta transform : pipelineMeta.getTransforms()) {
+ // AttributeMap("Git") cannot affect the comparison: by the time the
second version is
+ // compared the first one already carries the status of the first pass.
+ //
+ Map<String, String> tmp = transform.getAttributesMap().remove(ATTR_GIT);
+ try {
+ identities.put(transform.getName(), identity(transform.getXml()));
+ } catch (HopException e) {
+ LogChannel.GENERAL.logError(
+ "Error serializing transform '" + transform.getName() + "' to
detect renames", e);
+ } finally {
+ transform.setAttributes(ATTR_GIT, tmp);
+ }
+ }
+ return identities;
+ }
+
+ /** Actions renamed between the two versions, by name in the first version.
*/
+ public static Map<String, String> detectActionRenames(
+ WorkflowMeta workflowMeta1, WorkflowMeta workflowMeta2) {
+ return pairRenames(actionIdentities(workflowMeta1),
actionIdentities(workflowMeta2));
+ }
+
+ private static Map<String, String> actionIdentities(WorkflowMeta
workflowMeta) {
+ Map<String, String> identities = new LinkedHashMap<>();
+ for (ActionMeta action : workflowMeta.getActions()) {
+ Map<String, String> tmp = action.getAttributesMap().remove(ATTR_GIT);
+ try {
+ identities.put(action.getName(), identity(action.getXml()));
+ } finally {
+ action.setAttributes(ATTR_GIT, tmp);
+ }
+ }
+ return identities;
+ }
+
public static PipelineMeta compareTransforms(
PipelineMeta pipelineMeta1,
PipelineMeta pipelineMeta2,
boolean isForward,
- boolean ignorePosition) {
+ boolean ignorePosition,
+ Map<String, String> renames) {
pipelineMeta1
.getTransforms()
.forEach(
transform -> {
+ String name = renamedTo(transform.getName(), renames);
Optional<TransformMeta> transform2 =
pipelineMeta2.getTransforms().stream()
- .filter(obj -> transform.getName().equals(obj.getName()))
+ .filter(obj -> name.equals(obj.getName()))
.findFirst();
String status = null;
if (transform2.isPresent()) {
@@ -109,15 +215,23 @@ public class HopDiff {
}
public static PipelineMeta comparePipelineHops(
- PipelineMeta pipelineMeta1, PipelineMeta pipelineMeta2, boolean
isForward) {
+ PipelineMeta pipelineMeta1,
+ PipelineMeta pipelineMeta2,
+ boolean isForward,
+ Map<String, String> renames) {
pipelineMeta1
.getPipelineHops()
.forEach(
hop -> {
+ // A hop is identified by the transforms it connects, so it has
to be looked up under
+ // what those are called in the other version. The status is
still stored under the
+ // name in this version: that is what the painter asks for.
+ //
String hopName = getPipelineHopName(hop);
+ String lookupName = getPipelineHopName(hop, renames);
Optional<PipelineHopMeta> hop2 =
pipelineMeta2.getPipelineHops().stream()
- .filter(otherHop ->
hopName.equals(getPipelineHopName(otherHop)))
+ .filter(otherHop ->
lookupName.equals(getPipelineHopName(otherHop)))
.findFirst();
String status = null;
if (hop2.isPresent()) {
@@ -139,16 +253,20 @@ public class HopDiff {
}
public static String getPipelineHopName(PipelineHopMeta hopMeta) {
+ return getPipelineHopName(hopMeta, Map.of());
+ }
+
+ private static String getPipelineHopName(PipelineHopMeta hopMeta,
Map<String, String> renames) {
String name = "";
TransformMeta from = hopMeta.getFromTransform();
if (from != null) {
- name += from.getName();
+ name += renamedTo(from.getName(), renames);
}
name += " - ";
TransformMeta to = hopMeta.getToTransform();
if (to != null) {
- name += to.getName();
+ name += renamedTo(to.getName(), renames);
}
return name;
}
@@ -157,14 +275,16 @@ public class HopDiff {
WorkflowMeta workflowMeta1,
WorkflowMeta workflowMeta2,
boolean isForward,
- boolean ignorePosition) {
+ boolean ignorePosition,
+ Map<String, String> renames) {
workflowMeta1
.getActions()
.forEach(
je -> {
+ String name = renamedTo(je.getName(), renames);
Optional<ActionMeta> je2 =
workflowMeta2.getActions().stream()
- .filter(obj -> je.getName().equals(obj.getName()))
+ .filter(obj -> name.equals(obj.getName()))
.findFirst();
String status = null;
if (je2.isPresent()) {
@@ -193,15 +313,19 @@ public class HopDiff {
}
public static WorkflowMeta compareWorkflowHops(
- WorkflowMeta workflowMeta1, WorkflowMeta workflowMeta2, boolean
isForward) {
+ WorkflowMeta workflowMeta1,
+ WorkflowMeta workflowMeta2,
+ boolean isForward,
+ Map<String, String> renames) {
workflowMeta1
.getWorkflowHops()
.forEach(
hop -> {
String hopName = getWorkflowHopName(hop);
+ String lookupName = getWorkflowHopName(hop, renames);
Optional<WorkflowHopMeta> hop2 =
workflowMeta2.getWorkflowHops().stream()
- .filter(otherHop ->
hopName.equals(getWorkflowHopName(otherHop)))
+ .filter(otherHop ->
lookupName.equals(getWorkflowHopName(otherHop)))
.findFirst();
String status = null;
if (hop2.isPresent()) {
@@ -223,15 +347,19 @@ public class HopDiff {
}
public static String getWorkflowHopName(WorkflowHopMeta hopMeta) {
+ return getWorkflowHopName(hopMeta, Map.of());
+ }
+
+ private static String getWorkflowHopName(WorkflowHopMeta hopMeta,
Map<String, String> renames) {
String name = "";
ActionMeta from = hopMeta.getFromAction();
if (from != null) {
- name += from.getName();
+ name += renamedTo(from.getName(), renames);
}
name += " - ";
ActionMeta to = hopMeta.getToAction();
if (to != null) {
- name += to.getName();
+ name += renamedTo(to.getName(), renames);
}
return name;
}
diff --git
a/plugins/misc/git/src/main/java/org/apache/hop/git/info/GitInfoExplorerFileTypeHandler.java
b/plugins/misc/git/src/main/java/org/apache/hop/git/info/GitInfoExplorerFileTypeHandler.java
index fa030118cf..efca243100 100644
---
a/plugins/misc/git/src/main/java/org/apache/hop/git/info/GitInfoExplorerFileTypeHandler.java
+++
b/plugins/misc/git/src/main/java/org/apache/hop/git/info/GitInfoExplorerFileTypeHandler.java
@@ -44,6 +44,7 @@ import org.apache.hop.git.model.UIFile;
import org.apache.hop.git.model.UIGit;
import org.apache.hop.git.model.VCS;
import org.apache.hop.git.model.revision.ObjectRevision;
+import org.apache.hop.git.util.FileTypeUtils;
import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.ui.core.PropsUi;
@@ -443,12 +444,21 @@ public class GitInfoExplorerFileTypeHandler extends
BaseExplorerFileTypeHandler
new PipelineMeta(xmlStreamNew, hopGui.getMetadataProvider(),
hopGui.getVariables());
boolean ignorePosition =
GitConfigSingleton.getConfig().isIgnoringPositionInDiff();
+ Map<String, String> renamed =
+ HopDiff.detectTransformRenames(pipelineMetaOld, pipelineMetaNew);
+ Map<String, String> renamedBack =
+ HopDiff.detectTransformRenames(pipelineMetaNew, pipelineMetaOld);
+
+ pipelineMetaOld =
+ HopDiff.compareTransforms(
+ pipelineMetaOld, pipelineMetaNew, true, ignorePosition, renamed);
pipelineMetaOld =
- HopDiff.compareTransforms(pipelineMetaOld, pipelineMetaNew, true,
ignorePosition);
- pipelineMetaOld = HopDiff.comparePipelineHops(pipelineMetaOld,
pipelineMetaNew, true);
+ HopDiff.comparePipelineHops(pipelineMetaOld, pipelineMetaNew, true,
renamed);
+ pipelineMetaNew =
+ HopDiff.compareTransforms(
+ pipelineMetaNew, pipelineMetaOld, false, ignorePosition,
renamedBack);
pipelineMetaNew =
- HopDiff.compareTransforms(pipelineMetaNew, pipelineMetaOld, false,
ignorePosition);
- pipelineMetaNew = HopDiff.comparePipelineHops(pipelineMetaNew,
pipelineMetaOld, false);
+ HopDiff.comparePipelineHops(pipelineMetaNew, pipelineMetaOld, false,
renamedBack);
pipelineMetaOld.setPipelineVersion(CONST_GIT + commitIdOld);
pipelineMetaNew.setPipelineVersion(CONST_GIT + commitIdNew);
@@ -458,7 +468,7 @@ public class GitInfoExplorerFileTypeHandler extends
BaseExplorerFileTypeHandler
pipelineMetaOld.setName(
String.format(
CONST_S_S_S,
- pipelineMetaOld.getName(),
+ FileTypeUtils.getDiffName(filename, pipelineMetaOld.getName()),
git.getShortenedName(commitIdOld),
git.getShortenedName(commitIdNew)));
pipelineMetaOld.setNameSynchronizedWithFilename(false);
@@ -466,7 +476,7 @@ public class GitInfoExplorerFileTypeHandler extends
BaseExplorerFileTypeHandler
pipelineMetaNew.setName(
String.format(
CONST_S_S_S,
- pipelineMetaNew.getName(),
+ FileTypeUtils.getDiffName(filename, pipelineMetaNew.getName()),
git.getShortenedName(commitIdNew),
git.getShortenedName(commitIdOld)));
pipelineMetaNew.setNameSynchronizedWithFilename(false);
@@ -495,12 +505,19 @@ public class GitInfoExplorerFileTypeHandler extends
BaseExplorerFileTypeHandler
new WorkflowMeta(xmlStreamNew, hopGui.getMetadataProvider(),
hopGui.getVariables());
boolean ignorePosition =
GitConfigSingleton.getConfig().isIgnoringPositionInDiff();
+ Map<String, String> renamed =
HopDiff.detectActionRenames(workflowMetaOld, workflowMetaNew);
+ Map<String, String> renamedBack =
+ HopDiff.detectActionRenames(workflowMetaNew, workflowMetaOld);
+
+ workflowMetaOld =
+ HopDiff.compareActions(workflowMetaOld, workflowMetaNew, true,
ignorePosition, renamed);
workflowMetaOld =
- HopDiff.compareActions(workflowMetaOld, workflowMetaNew, true,
ignorePosition);
- workflowMetaOld = HopDiff.compareWorkflowHops(workflowMetaOld,
workflowMetaNew, true);
+ HopDiff.compareWorkflowHops(workflowMetaOld, workflowMetaNew, true,
renamed);
+ workflowMetaNew =
+ HopDiff.compareActions(
+ workflowMetaNew, workflowMetaOld, false, ignorePosition,
renamedBack);
workflowMetaNew =
- HopDiff.compareActions(workflowMetaNew, workflowMetaOld, false,
ignorePosition);
- workflowMetaNew = HopDiff.compareWorkflowHops(workflowMetaNew,
workflowMetaOld, false);
+ HopDiff.compareWorkflowHops(workflowMetaNew, workflowMetaOld, false,
renamedBack);
workflowMetaOld.setWorkflowVersion(CONST_GIT + commitIdOld);
workflowMetaNew.setWorkflowVersion(CONST_GIT + commitIdNew);
@@ -510,7 +527,7 @@ public class GitInfoExplorerFileTypeHandler extends
BaseExplorerFileTypeHandler
workflowMetaOld.setName(
String.format(
CONST_S_S_S,
- workflowMetaOld.getName(),
+ FileTypeUtils.getDiffName(filename, workflowMetaOld.getName()),
git.getShortenedName(commitIdOld),
git.getShortenedName(commitIdNew)));
workflowMetaOld.setNameSynchronizedWithFilename(false);
@@ -518,7 +535,7 @@ public class GitInfoExplorerFileTypeHandler extends
BaseExplorerFileTypeHandler
workflowMetaNew.setName(
String.format(
CONST_S_S_S,
- workflowMetaNew.getName(),
+ FileTypeUtils.getDiffName(filename, workflowMetaNew.getName()),
git.getShortenedName(commitIdNew),
git.getShortenedName(commitIdOld)));
workflowMetaNew.setNameSynchronizedWithFilename(false);
diff --git
a/plugins/misc/git/src/main/java/org/apache/hop/git/util/FileTypeUtils.java
b/plugins/misc/git/src/main/java/org/apache/hop/git/util/FileTypeUtils.java
index 19d1b213be..895f354da5 100644
--- a/plugins/misc/git/src/main/java/org/apache/hop/git/util/FileTypeUtils.java
+++ b/plugins/misc/git/src/main/java/org/apache/hop/git/util/FileTypeUtils.java
@@ -17,6 +17,7 @@
package org.apache.hop.git.util;
+import org.apache.commons.io.FilenameUtils;
import org.apache.hop.core.exception.HopException;
import org.apache.hop.core.logging.LogChannel;
import org.apache.hop.ui.hopgui.HopGui;
@@ -47,6 +48,19 @@ public final class FileTypeUtils {
return perspective.getFileTypeImage(fileType);
}
+ /**
+ * What to call a file in the title of a diff. Both versions are read out of
a git blob and carry
+ * no filename of their own, which leaves PipelineMeta and WorkflowMeta
falling back to the name
+ * stored inside the file. That is not what the rest of the GUI calls the
file.
+ *
+ * @param path the file being compared, relative to the repository
+ * @param fallback what to use if the path holds nothing usable
+ */
+ public static String getDiffName(String path, String fallback) {
+ String baseName = FilenameUtils.getBaseName(path);
+ return baseName == null || baseName.isEmpty() ? fallback : baseName;
+ }
+
public static boolean isHopFileType(String fileName) {
if (fileName != null) {
try {
diff --git a/plugins/misc/git/src/test/java/org/apache/hop/git/HopDiffTest.java
b/plugins/misc/git/src/test/java/org/apache/hop/git/HopDiffTest.java
index 44ac074811..7c602b3a61 100644
--- a/plugins/misc/git/src/test/java/org/apache/hop/git/HopDiffTest.java
+++ b/plugins/misc/git/src/test/java/org/apache/hop/git/HopDiffTest.java
@@ -20,18 +20,24 @@ package org.apache.hop.git;
import static org.apache.hop.git.HopDiff.ADDED;
import static org.apache.hop.git.HopDiff.ATTR_GIT;
+import static org.apache.hop.git.HopDiff.ATTR_GIT_HOPS;
import static org.apache.hop.git.HopDiff.ATTR_STATUS;
import static org.apache.hop.git.HopDiff.CHANGED;
import static org.apache.hop.git.HopDiff.REMOVED;
import static org.apache.hop.git.HopDiff.UNCHANGED;
import static org.apache.hop.git.HopDiff.compareActions;
+import static org.apache.hop.git.HopDiff.comparePipelineHops;
import static org.apache.hop.git.HopDiff.compareTransforms;
+import static org.apache.hop.git.HopDiff.detectActionRenames;
+import static org.apache.hop.git.HopDiff.detectTransformRenames;
import static org.apache.hop.git.HopDiff.getPipelineHopName;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
import java.io.File;
import java.io.FileInputStream;
import java.io.InputStream;
+import java.util.Map;
import org.apache.hop.core.HopClientEnvironment;
import org.apache.hop.core.annotations.Action;
import org.apache.hop.core.annotations.Transform;
@@ -56,6 +62,8 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
class HopDiffTest {
+ private static final Map<String, String> NO_RENAMES = Map.of();
+
IHopMetadataProvider metadataProvider;
@BeforeEach
@@ -101,8 +109,10 @@ class HopDiffTest {
PipelineMeta pipelineMeta1 = loadPipeline("r1.hpl");
PipelineMeta pipelineMeta2 = loadPipeline("r2.hpl");
- PipelineMeta resultForward = compareTransforms(pipelineMeta1,
pipelineMeta2, true, false);
- PipelineMeta resultBackward = compareTransforms(pipelineMeta2,
pipelineMeta1, false, false);
+ PipelineMeta resultForward =
+ compareTransforms(pipelineMeta1, pipelineMeta2, true, false,
NO_RENAMES);
+ PipelineMeta resultBackward =
+ compareTransforms(pipelineMeta2, pipelineMeta1, false, false,
NO_RENAMES);
assertEquals(CHANGED, resultForward.getTransform(0).getAttribute(ATTR_GIT,
ATTR_STATUS));
assertEquals(UNCHANGED,
resultForward.getTransform(1).getAttribute(ATTR_GIT, ATTR_STATUS));
assertEquals(REMOVED, resultForward.getTransform(2).getAttribute(ATTR_GIT,
ATTR_STATUS));
@@ -119,14 +129,16 @@ class HopDiffTest {
@Test
void movedTransformIsChangedWithoutTheOption() throws Exception {
- PipelineMeta result = compareTransforms(loadPipeline("r1.hpl"),
movedPipeline(), true, false);
+ PipelineMeta result =
+ compareTransforms(loadPipeline("r1.hpl"), movedPipeline(), true,
false, NO_RENAMES);
assertEquals(CHANGED, result.getTransform(0).getAttribute(ATTR_GIT,
ATTR_STATUS));
}
@Test
void movedTransformIsUnchangedWithTheOption() throws Exception {
- PipelineMeta result = compareTransforms(loadPipeline("r1.hpl"),
movedPipeline(), true, true);
+ PipelineMeta result =
+ compareTransforms(loadPipeline("r1.hpl"), movedPipeline(), true, true,
NO_RENAMES);
assertEquals(UNCHANGED, result.getTransform(0).getAttribute(ATTR_GIT,
ATTR_STATUS));
}
@@ -136,8 +148,8 @@ class HopDiffTest {
WorkflowMeta jobMeta = loadWorkflow("r1.hwf");
WorkflowMeta jobMeta2 = loadWorkflow("r2.hwf");
- jobMeta = compareActions(jobMeta, jobMeta2, true, false);
- jobMeta2 = compareActions(jobMeta2, jobMeta, false, false);
+ jobMeta = compareActions(jobMeta, jobMeta2, true, false, NO_RENAMES);
+ jobMeta2 = compareActions(jobMeta2, jobMeta, false, false, NO_RENAMES);
assertEquals(CHANGED, jobMeta.getAction(0).getAttribute(ATTR_GIT,
ATTR_STATUS));
assertEquals(UNCHANGED, jobMeta.getAction(1).getAttribute(ATTR_GIT,
ATTR_STATUS));
assertEquals(REMOVED, jobMeta.getAction(2).getAttribute(ATTR_GIT,
ATTR_STATUS));
@@ -150,8 +162,8 @@ class HopDiffTest {
WorkflowMeta jobMeta = loadWorkflow("r1.hwf");
WorkflowMeta jobMeta2 = loadWorkflow("r2.hwf");
- jobMeta = compareActions(jobMeta, jobMeta2, true, true);
- jobMeta2 = compareActions(jobMeta2, jobMeta, false, true);
+ jobMeta = compareActions(jobMeta, jobMeta2, true, true, NO_RENAMES);
+ jobMeta2 = compareActions(jobMeta2, jobMeta, false, true, NO_RENAMES);
assertEquals("START", jobMeta.getAction(0).getName());
assertEquals(UNCHANGED, jobMeta.getAction(0).getAttribute(ATTR_GIT,
ATTR_STATUS));
assertEquals(UNCHANGED, jobMeta.getAction(1).getAttribute(ATTR_GIT,
ATTR_STATUS));
@@ -167,4 +179,93 @@ class HopDiffTest {
assertEquals(
"CSV file input - Add a checksum",
getPipelineHopName(pipelineMeta.getPipelineHop(0)));
}
+
+ /** r1 with "Add a checksum" renamed, and nothing else touched. */
+ private PipelineMeta renamedPipeline() throws Exception {
+ PipelineMeta pipelineMeta = loadPipeline("r1.hpl");
+ pipelineMeta.getTransform(1).setName("checksum renamed");
+ return pipelineMeta;
+ }
+
+ @Test
+ void renameIsDetected() throws Exception {
+ Map<String, String> renames =
detectTransformRenames(loadPipeline("r1.hpl"), renamedPipeline());
+
+ assertEquals(Map.of("Add a checksum", "checksum renamed"), renames);
+ }
+
+ @Test
+ void renamedTransformIsChangedRatherThanAddedAndRemoved() throws Exception {
+ PipelineMeta original = loadPipeline("r1.hpl");
+ PipelineMeta renamed = renamedPipeline();
+ Map<String, String> renames = detectTransformRenames(original, renamed);
+ Map<String, String> back = detectTransformRenames(renamed, original);
+
+ PipelineMeta forward = compareTransforms(original, renamed, true, true,
renames);
+ PipelineMeta backward = compareTransforms(renamed, original, false, true,
back);
+
+ assertEquals(CHANGED, forward.getTransform(1).getAttribute(ATTR_GIT,
ATTR_STATUS));
+ assertEquals(CHANGED, backward.getTransform(1).getAttribute(ATTR_GIT,
ATTR_STATUS));
+ // What sits around it is untouched.
+ assertEquals(UNCHANGED, forward.getTransform(0).getAttribute(ATTR_GIT,
ATTR_STATUS));
+ assertEquals(UNCHANGED, forward.getTransform(2).getAttribute(ATTR_GIT,
ATTR_STATUS));
+ }
+
+ /** Both hops touch the renamed transform, and neither of them actually
changed. */
+ @Test
+ void hopsOfARenamedTransformAreNotReported() throws Exception {
+ PipelineMeta original = loadPipeline("r1.hpl");
+ PipelineMeta renamed = renamedPipeline();
+ Map<String, String> renames = detectTransformRenames(original, renamed);
+
+ PipelineMeta forward = comparePipelineHops(original, renamed, true,
renames);
+
+ assertNull(forward.getAttribute(ATTR_GIT_HOPS, "CSV file input - Add a
checksum"));
+ assertNull(forward.getAttribute(ATTR_GIT_HOPS, "Add a checksum -
Calculator"));
+ }
+
+ @Test
+ void renameIsStillDetectedWhenTheTransformAlsoMoved() throws Exception {
+ PipelineMeta renamed = renamedPipeline();
+ TransformMeta transform = renamed.getTransform(1);
+ transform.setLocation(transform.getLocation().x + 80,
transform.getLocation().y + 40);
+
+ Map<String, String> renames =
detectTransformRenames(loadPipeline("r1.hpl"), renamed);
+
+ assertEquals(Map.of("Add a checksum", "checksum renamed"), renames);
+ }
+
+ @Test
+ void severalRenamesInOneCommitAreEachPairedUp() throws Exception {
+ PipelineMeta renamed = loadPipeline("r1.hpl");
+ renamed.getTransform(1).setName("checksum renamed");
+ renamed.getTransform(2).setName("calculator renamed");
+
+ Map<String, String> renames =
detectTransformRenames(loadPipeline("r1.hpl"), renamed);
+
+ assertEquals(
+ Map.of("Add a checksum", "checksum renamed", "Calculator", "calculator
renamed"), renames);
+ }
+
+ /** r1 drops Calculator and r2 adds Select values. Unrelated, so not a
rename. */
+ @Test
+ void aRealRemovalIsNotReadAsARename() throws Exception {
+ Map<String, String> renames =
+ detectTransformRenames(loadPipeline("r1.hpl"), loadPipeline("r2.hpl"));
+
+ assertEquals(Map.of(), renames);
+ }
+
+ @Test
+ void renamedActionIsChangedRatherThanAddedAndRemoved() throws Exception {
+ WorkflowMeta original = loadWorkflow("r1.hwf");
+ WorkflowMeta renamed = loadWorkflow("r1.hwf");
+ renamed.getAction(1).setName("workflow renamed");
+
+ Map<String, String> renames = detectActionRenames(original, renamed);
+ WorkflowMeta forward = compareActions(original, renamed, true, true,
renames);
+
+ assertEquals(Map.of("Workflow", "workflow renamed"), renames);
+ assertEquals(CHANGED, forward.getAction(1).getAttribute(ATTR_GIT,
ATTR_STATUS));
+ }
}
diff --git
a/plugins/misc/git/src/test/java/org/apache/hop/git/util/FileTypeUtilsTest.java
b/plugins/misc/git/src/test/java/org/apache/hop/git/util/FileTypeUtilsTest.java
new file mode 100644
index 0000000000..1e06264791
--- /dev/null
+++
b/plugins/misc/git/src/test/java/org/apache/hop/git/util/FileTypeUtilsTest.java
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.hop.git.util;
+
+import static org.apache.hop.git.util.FileTypeUtils.getDiffName;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.junit.jupiter.api.Test;
+
+class FileTypeUtilsTest {
+
+ @Test
+ void theFileIsNamedAfterItsPathRatherThanWhatIsStoredInside() {
+ assertEquals("bogus-pipeline", getDiffName("code/bogus-pipeline.hpl", "New
pipeline"));
+ assertEquals("main", getDiffName("main.hwf", "New workflow"));
+ assertEquals("dotted.name", getDiffName("a/b/dotted.name.hpl", "New
pipeline"));
+ }
+
+ @Test
+ void aPathWithNothingUsableInItFallsBack() {
+ assertEquals("New pipeline", getDiffName(null, "New pipeline"));
+ assertEquals("New pipeline", getDiffName("", "New pipeline"));
+ assertEquals("New pipeline", getDiffName("code/", "New pipeline"));
+ }
+}
