diff --git a/src/main/java/org/openrewrite/java/spring/ChangeSpringPropertyKey.java b/src/main/java/org/openrewrite/java/spring/ChangeSpringPropertyKey.java index 7c4b7e802..016b682ba 100644 --- a/src/main/java/org/openrewrite/java/spring/ChangeSpringPropertyKey.java +++ b/src/main/java/org/openrewrite/java/spring/ChangeSpringPropertyKey.java @@ -31,15 +31,20 @@ import org.openrewrite.properties.search.FindProperties; import org.openrewrite.properties.tree.Properties; import org.openrewrite.yaml.ChangePropertyKey; +import org.openrewrite.yaml.MergeYamlVisitor; import org.openrewrite.yaml.UnfoldProperties; import org.openrewrite.yaml.YamlIsoVisitor; import org.openrewrite.yaml.tree.Yaml; +import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.regex.Matcher; import java.util.regex.Pattern; +import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static java.util.regex.Pattern.quote; @@ -81,8 +86,6 @@ public class ChangeSpringPropertyKey extends Recipe { @Override public TreeVisitor getVisitor() { - ChangePropertyKey yamlChangePropertyKey = - new ChangePropertyKey(oldPropertyKey, newPropertyKey, true, except, null); org.openrewrite.properties.ChangePropertyKey propertiesChangePropertyKey = new org.openrewrite.properties.ChangePropertyKey(oldPropertyKey, newPropertyKey, true, false); org.openrewrite.properties.ChangePropertyKey subpropertiesChangePropertyKey = @@ -99,10 +102,15 @@ public TreeVisitor getVisitor() { @Override public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { if (tree instanceof Yaml.Documents) { - boolean nested = isWrittenAsNestedMappings((Yaml.Documents) tree); - Tree newTree = yamlChangePropertyKey.getVisitor().visit(tree, ctx); - if (newTree != tree && nested) { - newTree = unfoldNewPropertyKey.getVisitor().visit(newTree, ctx); + Yaml.Documents documents = (Yaml.Documents) tree; + boolean nested = isWrittenAsNestedMappings(documents); + Tree newTree = new ChangePropertyKey(oldPropertyKey, newPropertyKey, true, excludedSubkeys(documents), null) + .getVisitor().visit(tree, ctx); + if (newTree != tree) { + if (nested) { + newTree = unfoldNewPropertyKey.getVisitor().visit(newTree, ctx); + } + newTree = new MergeRelocatedSectionsVisitor().visit(newTree, ctx); } tree = newTree; } else if (tree instanceof Properties.File) { @@ -132,22 +140,87 @@ private boolean isWrittenAsNestedMappings(Yaml.Documents documents) { @Override public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, AtomicBoolean found) { if (!found.get() && !entry.getKey().getValue().contains(".") && - NameCaseConvention.matchesGlobRelaxedBinding(propertyPath(entry), oldPropertyKey)) { + NameCaseConvention.matchesGlobRelaxedBinding(propertyPath(entry, getCursor()), oldPropertyKey)) { found.set(true); } return super.visitMappingEntry(entry, found); } + }.reduce(documents, new AtomicBoolean()).get(); + } - private String propertyPath(Yaml.Mapping.Entry entry) { - StringBuilder path = new StringBuilder(entry.getKey().getValue()); - for (Cursor c = getCursor().getParent(); c != null; c = c.getParent()) { - if (c.getValue() instanceof Yaml.Mapping.Entry) { - path.insert(0, ((Yaml.Mapping.Entry) c.getValue()).getKey().getValue() + '.'); + /** + * `except` is documented as a regex, and applied as such to properties files, but + * `org.openrewrite.yaml.ChangePropertyKey` matches it as a glob. Resolve the regex against the subkeys actually + * present under `oldPropertyKey` here, so both formats exclude the same subproperties. + */ + private @Nullable List excludedSubkeys(Yaml.Documents documents) { + if (except == null || except.isEmpty()) { + return except; + } + Pattern exceptedSubkey = Pattern.compile("(?:" + String.join("|", except) + ")\\b.*", Pattern.DOTALL); + Set subkeys = new YamlIsoVisitor>() { + @Override + public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, Set subkeys) { + if (entry.getValue() instanceof Yaml.Mapping && + NameCaseConvention.matchesGlobRelaxedBinding(propertyPath(entry, getCursor()), oldPropertyKey)) { + for (Yaml.Mapping.Entry child : ((Yaml.Mapping) entry.getValue()).getEntries()) { + String subkey = child.getKey().getValue(); + if (exceptedSubkey.matcher(subkey).matches()) { + subkeys.add(subkey); + } } } - return path.toString(); + return super.visitMappingEntry(entry, subkeys); } - }.reduce(documents, new AtomicBoolean()).get(); + }.reduce(documents, new LinkedHashSet<>()); + return new ArrayList<>(subkeys); + } + + /** + * Relocating a property into a section that already exists leaves two mapping entries with the same key; merge + * those back together, limited to the segments of `newPropertyKey` so unrelated duplicates are left alone. + */ + private class MergeRelocatedSectionsVisitor extends YamlIsoVisitor { + private final List newKeySegments = asList(newPropertyKey.split("\\.")); + + @Override + public Yaml.Mapping visitMapping(Yaml.Mapping mapping, ExecutionContext ctx) { + Yaml.Mapping m = super.visitMapping(mapping, ctx); + List entries = m.getEntries(); + return m.withEntries(ListUtils.map(entries, (i, entry) -> { + if (!isMergeable(entry)) { + return entry; + } + Yaml.Block value = entry.getValue(); + for (int j = 0; j < entries.size(); j++) { + Yaml.Mapping.Entry other = entries.get(j); + if (i == j || !isMergeable(other) || + !entry.getKey().getValue().equals(other.getKey().getValue())) { + continue; + } + if (j < i) { + return null; // already merged into the first entry with this key + } + value = (Yaml.Block) new MergeYamlVisitor<>(value, other.getValue(), false, null, null, null) + .visitNonNull(value, ctx, getCursor()); + } + return entry.withValue(value); + })); + } + + private boolean isMergeable(Yaml.Mapping.Entry entry) { + return newKeySegments.contains(entry.getKey().getValue()) && !entry.getPrefix().contains("#"); + } + } + + private static String propertyPath(Yaml.Mapping.Entry entry, Cursor cursor) { + StringBuilder path = new StringBuilder(entry.getKey().getValue()); + for (Cursor c = cursor.getParent(); c != null; c = c.getParent()) { + if (c.getValue() instanceof Yaml.Mapping.Entry) { + path.insert(0, ((Yaml.Mapping.Entry) c.getValue()).getKey().getValue() + '.'); + } + } + return path.toString(); } private String exceptRegex() { diff --git a/src/test/java/org/openrewrite/java/spring/ChangeSpringPropertyKeyTest.java b/src/test/java/org/openrewrite/java/spring/ChangeSpringPropertyKeyTest.java index 8ae2f576c..3aab9ae4b 100644 --- a/src/test/java/org/openrewrite/java/spring/ChangeSpringPropertyKeyTest.java +++ b/src/test/java/org/openrewrite/java/spring/ChangeSpringPropertyKeyTest.java @@ -15,7 +15,6 @@ */ package org.openrewrite.java.spring; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.InMemoryExecutionContext; @@ -331,6 +330,80 @@ void avoidRegenerativeChangesInYaml() { ); } + @Issue("https://github.com/openrewrite/rewrite-spring/issues/436") + @Test + void exceptAppliesAsRegexToYamlAsWellAsProperties() { + rewriteRun( + spec -> spec.recipe(new ChangeSpringPropertyKey("logging.file", "logging.file.name", List.of(".+"))), + mavenProject("project", + srcMainResources( + //language=properties + properties( + """ + logging.file.max-size = 10MB + """ + ), + //language=yaml + yaml( + """ + logging: + file: + max-size: 10MB + """ + ), + //language=properties + properties( + """ + logging.file = foo.txt + """, + """ + logging.file.name = foo.txt + """ + ), + //language=yaml + yaml( + """ + logging: + file: foo.txt + """, + """ + logging: + file: + name: foo.txt + """ + ) + ) + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-spring/issues/436") + @Test + void relocatedPropertyMergesIntoExistingSection() { + rewriteRun( + spec -> spec.recipe(new ChangeSpringPropertyKey("logging.path", "logging.file.path", null)), + mavenProject("project", + srcMainResources( + //language=yaml + yaml( + """ + logging: + file: + max-size: 10MB + path: ${user.home}/some-folder + """, + """ + logging: + file: + max-size: 10MB + path: ${user.home}/some-folder + """ + ) + ) + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite-spring/issues/432") @Test void loggingFileSubproperties() { @@ -429,7 +502,6 @@ void redisSslBooleanStillRenamed() { ); } - @Disabled @Issue("https://github.com/openrewrite/rewrite-spring/issues/436") @Test void loggingFileSubpropertiesYaml() {