Disable value property resolution in org.openrewrite.maven.AddProperty

[jpraet]

What problem are you trying to solve?

With this recipe

  - org.openrewrite.maven.AddProperty:
      key: java_home
      value: ${java_home.jdk21}
      trustParent: true

I'm trying to add <java_home>${java_home.jdk21}</java_home> to the properties in the pom.xml.
Where the <java_home.jdk21> property is defined in the ~/.m2/settings.xml on the developer workstations.
(in my case C:\tools\redhat-jdk21, but this can be different for other devs, and on our build server).

Unfortunately the property is set with the resolved value to <java_home>C:\tools\redhat-jdk21</java_home>.

Describe the solution you'd like

An option to escape / disable the maven property resolution.

[timtebeek]

Yes seems reasonable; would you be willing to help look into this? Even if it's just adding a test at first, that's already helpful. From there we can then look through what's needed to make it pass.

For now I'm doubting between if we should resolve variables at all, or if we need to offer a means to escape property resolve.

[jpraet]

On second thought, I don't think it's the org.openrewrite.maven.AddProperty recipe that does this.

Looks like the rewrite-maven-plugin resolves the property when it loads the recipe yaml file:

rewrite-maven-plugin/src/main/java/org/openrewrite/maven/AbstractRewriteMojo.java

Lines 121 to 142 in cbba2d4

	protected Environment environment(@Nullable ClassLoader recipeClassLoader) throws MojoExecutionException {
	Environment.Builder env = Environment.builder(project.getProperties());
	if (recipeClassLoader == null) {
	env.scanRuntimeClasspath()
	.scanUserHome();
	} else {
	env.load(new ClasspathScanningLoader(project.getProperties(), recipeClassLoader));
	}
	try {
	Config rewriteConfig = getConfig();
	if (rewriteConfig != null) {
	try (InputStream is = rewriteConfig.inputStream) {
	env.load(new YamlResourceLoader(is, rewriteConfig.uri, project.getProperties()));
	}
	}
	} catch (IOException e) {
	throw new MojoExecutionException("Unable to load rewrite configuration", e);
	}
	return env.build();
	}

[der-eismann]

We have issues with this using AddDependency (and probably other recipes) as well. Applying this

  - org.openrewrite.maven.AddDependency:
      groupId: com.examplelibrary.messaging
      artifactId: library
      version: ${example.messaging.version}

Will explicitly set the version to the resolved value (if already existing) instead of using ${example.messaging.version}. Would be great if this could be disabled.

[timtebeek]

hi! Yes understand the use case; it's just that up to now that had been added quite deliberately. We could maybe disable it with a toggle, as I'm not seeing any option to escape replacements in the PropertyPlaceholderHelper as used from YamlResourceLoader.

[timtebeek]

Figured push up a quick PR; if either of you could try that out and let me know your thoughts that'd be appreciated

• Optionally resolve properties in yaml #741

[timtebeek]

@jpraet / @der-eismann did either of you try out the latest snapshot already? Just to be sure this matches what you'd hoped for.

[der-eismann]

Hey @timtebeek, I can confirm it's working fine for me. Thanks for the quick action!
Before:

  - org.openrewrite.maven.AddDependency:
      groupId: com.example.library
      artifactId: spring-boot-starter-openapi
      version: "${openapi.version}"
=>
        <dependency>
            <groupId>com.example.library</groupId>
            <artifactId>spring-boot-starter-openapi</artifactId>
            <version>2.2.1</version>
        </dependency>

After calling it with -Drewrite.resolvePropertiesInYaml=false

  - org.openrewrite.maven.AddDependency:
      groupId: com.example.library
      artifactId: spring-boot-starter-openapi
      version: "${openapi.version}"
=>
        <dependency>
            <groupId>com.example.library</groupId>
            <artifactId>spring-boot-starter-openapi</artifactId>
            <version>${openapi.version}</version>
        </dependency>

---

Side note: The docs for the snapshots are slightly wrong:

Snapshot versions are 1 minor version ahead of the latest release with -SNAPSHOT added to the end. For instance, if the latest release is 2.1.2, the snapshot version would be 2.2.1-SNAPSHOT:

The snapshot version would be 2.2.0-SNAPSHOT. Took me a while to figure out it's 5.24.0-SNAPSHOT I have to use. At the same time I don't know how there can be a x.x.1-SNAPSHOT version then. Or is it 1 minor ahead and 1 patch behind?

[timtebeek]

The snapshot version would be 2.2.0-SNAPSHOT. Took me a while to figure out it's 5.24.0-SNAPSHOT I have to use.

Thanks! Fixed just now openrewrite/rewrite-docs@3b85d0c ; we have some imperfect automations to change version numbers across the docs; this was inadvertently changed in a recent update of the docs:
openrewrite/rewrite-docs@021c479#diff-73f5abbf41c5f56f6052fd3a83df0aaccb82021ad8c198d22ee77241cde9cc6c

/cc @mike-solomon

At the same time I don't know how there can be a x.x.1-SNAPSHOT version then. Or is it 1 minor ahead and 1 patch behind?

There will indeed never be a x.x.1-SNAPSHOT version, as part of our use of nebula version strategy. The next version is always x.y.0-SNAPSHOT, with y = current minor version + 1

[der-eismann]

There will indeed never be a x.x.1-SNAPSHOT version, as part of our use of nebula version strategy. The next version is always x.y.0-SNAPSHOT, with y = current minor version + 1

There are some though, but that's too offtopic here. Thanks for fixing the docs!

[timtebeek]

Ah yes the Maven plugin is a slightly special case, as that doesn't use our typical Gradle plugins. For this project it can indeed happen that we do a .patch release when the releaser feels like it (usually me 👋🏻 ). In general though, we still use the Maven release plugin, which in our case also bumps to the next minor release version, as for instance seen in this automated commit:
1af95aa

[jpraet]

Unfortunately, -Drewrite.resolvePropertiesInYaml=false does not solve the problem for me, because I need some properties in my recipe to be resolved, but some others not. So this all or nothing configuration flag doesn't address that.

I would need some more targeted mechanism (like an escape character) to exclude only certain properties from being resolved.

[timtebeek]

Unfortunately, -Drewrite.resolvePropertiesInYaml=false does not solve the problem for me, because I need some properties in my recipe to be resolved, but some others not. So this all or nothing configuration flag doesn't address that.

I would need some more targeted mechanism (like an escape character) to exclude only certain properties from being resolved.

Open to any proposals you might have; it's easier to review a concrete PR if you feel this would be helpful.