[MNG-6437] Better support for path and uri in property interpolation #812

gnodet · 2022-10-05T08:00:59Z

michael-o

Yet another problem with URIs is to get a decent string representation URI#toString() is not suited since it does not encode unsafe characters. Can this address this problem to invoke URI#toASCIIString() instead?

maven-core/src/main/java/org/apache/maven/plugin/PluginParameterExpressionEvaluatorV4.java

maven-core/src/main/java/org/apache/maven/plugin/ReflectionValueExtractor.java

maven-core/src/test/java/org/apache/maven/plugin/PluginParameterExpressionEvaluatorV4Test.java

gnodet · 2022-10-09T18:54:48Z

Yet another problem with URIs is to get a decent string representation URI#toString() is not suited since it does not encode unsafe characters. Can this address this problem to invoke URI#toASCIIString() instead?

${session.multiModuleProjectDirectory.uri} returns a URI object. If you interpolate it, the toString() method will be used by default. But one could also use ${session.multiModuleProjectDirectory.uri.ASCIIString} I think, as all toXxx methods are supported.
There may be a way to intercept URI -> String conversion, I'll look into it.

gnodet · 2022-10-09T20:26:07Z

Yet another problem with URIs is to get a decent string representation URI#toString() is not suited since it does not encode unsafe characters. Can this address this problem to invoke URI#toASCIIString() instead?

${session.multiModuleProjectDirectory.uri} returns a URI object. If you interpolate it, the toString() method will be used by default. But one could also use ${session.multiModuleProjectDirectory.uri.aSCIIString} I think, as all toXxx methods are supported. There may be a way to intercept URI -> String conversion, I'll look into it.

Fwiw, injecting a URI into a String property is forbidden because of the following code:
https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/plugin/PluginParameterExpressionEvaluatorV4.java#L376-L387
This means that plugins need to be injected with a URI field or use an explicit conversion.

Also, I just discovered that Path.toUri() is actually different than Path.toFile().toURI() as Path.toUri() actually encodes the path, so that the URI.toString() will actually return an encoded uri, i.e. the same as URI.toASCIIString():

Paths.get( "rép➜α" ).toUri().toString() -> "file:///Users/gnodet/work/git/maven4/maven-core/re%CC%81p%E2%9E%9C%CE%B1"
Paths.get( "rép➜α" ).toFile().toURI().toASCIIString() -> "file:///Users/gnodet/work/git/maven4/maven-core/re%CC%81p%E2%9E%9C%CE%B1"
Paths.get( "rép➜α" ).toFile().toURI().toString() -> "file:/Users/gnodet/work/git/maven4/maven-core/rép➜α"

In short, there's no need to go through the toASCIIString() method from a Path.

However, this PR only tackles plugin injection and not model interpolation. The toXxx and asXxx are currently not supported during interpolation. For this to work, the original plexus ReflectionValueExtractor would have to be modified instead of being copied/modified like in this PR.

gnodet · 2022-10-09T21:26:05Z

As shown by the tests failing on windows, my assumption that Path.toUri().toString() and Path.toUri().toASCIIString() are the same (i.e. the uri path is already encoded) seems only valid on unix platforms and not on windows...

michael-o · 2023-09-22T14:48:45Z

As shown by the tests failing on windows, my assumption that Path.toUri().toString() and Path.toUri().toASCIIString() are the same (i.e. the uri path is already encoded) seems only valid on unix platforms and not on windows...

Let me try to replicate, that is total BS.

michael-o · 2023-09-22T14:47:22Z

maven-core/src/main/java/org/apache/maven/plugin/ReflectionValueExtractor.java

+                Method method = classMap.findMethod(prefix + methodBase);
+                if (method != null) {
+                    return method.invoke(value, OBJECT_ARGS);
+                }


Shouldn't this be a separate, preceding PR?

No, that's one of the important change to support toXxx and asXxx.

I know, but can be logically done before this change, no? But anyway, let's leave this hunk as-is.

gnodet · 2023-09-22T14:52:34Z

@michael-o fancy another look ?

michael-o · 2023-09-22T15:04:04Z

Well, built your branch and tested on:

osipovmi@deblndw011x:/tmp/ма вен (MNG-6437 =)
$ ../apache-maven-4.0.0-alpha-8-SNAPSHOT/bin/mvn -v
Apache Maven 4.0.0-alpha-8-SNAPSHOT (521a57d768f1aab6dc792ee1a234c9b70267f3a2)
Maven home: /tmp/apache-maven-4.0.0-alpha-8-SNAPSHOT
Java version: 1.8.0_372, vendor: OpenJDK BSD Porting Team, runtime: /usr/local/openjdk8/jre
Default locale: de_DE, platform encoding: UTF-8
OS name: "freebsd", version: "12.4-stable", arch: "amd64", family: "unix"

It fails in mid-air:

[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.041 s <<< FAILURE! -- in org.apache.maven.model.building.DefaultModelBuilderTest
[ERROR] org.apache.maven.model.building.DefaultModelBuilderTest.testBuildRawModel -- Time elapsed: 0.005 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: not <null>
        at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
        at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at org.junit.jupiter.api.AssertNotNull.failNull(AssertNotNull.java:49)
        at org.junit.jupiter.api.AssertNotNull.assertNotNull(AssertNotNull.java:35)
        at org.junit.jupiter.api.AssertNotNull.assertNotNull(AssertNotNull.java:30)
        at org.junit.jupiter.api.Assertions.assertNotNull(Assertions.java:301)
        at org.apache.maven.model.building.DefaultModelBuilderTest.testBuildRawModel(DefaultModelBuilderTest.java:164)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727)
        at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
        at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
        at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
        at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
        at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
        at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)

I have tried help:evaluate, but all I get is null. I have found another issue: if an object contains #isFile() and #toFile(), the array will determine which to call.

See:

[INFO] Enter the Maven expression i.e. ${project.groupId} or 0 to exit?:
${session.multiModuleProjectDirectory.uri}
[INFO]
null object or invalid expression
[INFO] Enter the Maven expression i.e. ${project.groupId} or 0 to exit?:
${session.multiModuleProjectDirectory.uri.string}

What am I doing wrong?

gnodet · 2023-09-22T19:36:13Z

Well, built your branch and tested on:

osipovmi@deblndw011x:/tmp/ма вен (MNG-6437 =)
$ ../apache-maven-4.0.0-alpha-8-SNAPSHOT/bin/mvn -v
Apache Maven 4.0.0-alpha-8-SNAPSHOT (521a57d768f1aab6dc792ee1a234c9b70267f3a2)
Maven home: /tmp/apache-maven-4.0.0-alpha-8-SNAPSHOT
Java version: 1.8.0_372, vendor: OpenJDK BSD Porting Team, runtime: /usr/local/openjdk8/jre
Default locale: de_DE, platform encoding: UTF-8
OS name: "freebsd", version: "12.4-stable", arch: "amd64", family: "unix"

It fails in mid-air:

[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.041 s <<< FAILURE! -- in org.apache.maven.model.building.DefaultModelBuilderTest
[ERROR] org.apache.maven.model.building.DefaultModelBuilderTest.testBuildRawModel -- Time elapsed: 0.005 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: not <null>
        at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
        at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at org.junit.jupiter.api.AssertNotNull.failNull(AssertNotNull.java:49)
        at org.junit.jupiter.api.AssertNotNull.assertNotNull(AssertNotNull.java:35)
        at org.junit.jupiter.api.AssertNotNull.assertNotNull(AssertNotNull.java:30)
        at org.junit.jupiter.api.Assertions.assertNotNull(Assertions.java:301)
        at org.apache.maven.model.building.DefaultModelBuilderTest.testBuildRawModel(DefaultModelBuilderTest.java:164)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727)
        at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
        at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
        at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
        at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
        at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
        at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)

I have tried help:evaluate, but all I get is null. I have found another issue: if an object contains #isFile() and #toFile(), the array will determine which to call.

See:

[INFO] Enter the Maven expression i.e. ${project.groupId} or 0 to exit?:
${session.multiModuleProjectDirectory.uri}
[INFO]
null object or invalid expression
[INFO] Enter the Maven expression i.e. ${project.groupId} or 0 to exit?:
${session.multiModuleProjectDirectory.uri.string}

What am I doing wrong?

The PR only tackles plugin configuration injection, not model interpolation.
I'll see if/how that can be done...

michael-o · 2023-09-22T19:49:51Z

Well, built your branch and tested on:

osipovmi@deblndw011x:/tmp/ма вен (MNG-6437 =)
$ ../apache-maven-4.0.0-alpha-8-SNAPSHOT/bin/mvn -v
Apache Maven 4.0.0-alpha-8-SNAPSHOT (521a57d768f1aab6dc792ee1a234c9b70267f3a2)
Maven home: /tmp/apache-maven-4.0.0-alpha-8-SNAPSHOT
Java version: 1.8.0_372, vendor: OpenJDK BSD Porting Team, runtime: /usr/local/openjdk8/jre
Default locale: de_DE, platform encoding: UTF-8
OS name: "freebsd", version: "12.4-stable", arch: "amd64", family: "unix"

It fails in mid-air:

[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.041 s <<< FAILURE! -- in org.apache.maven.model.building.DefaultModelBuilderTest
[ERROR] org.apache.maven.model.building.DefaultModelBuilderTest.testBuildRawModel -- Time elapsed: 0.005 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: not <null>
        at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
        at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at org.junit.jupiter.api.AssertNotNull.failNull(AssertNotNull.java:49)
        at org.junit.jupiter.api.AssertNotNull.assertNotNull(AssertNotNull.java:35)
        at org.junit.jupiter.api.AssertNotNull.assertNotNull(AssertNotNull.java:30)
        at org.junit.jupiter.api.Assertions.assertNotNull(Assertions.java:301)
        at org.apache.maven.model.building.DefaultModelBuilderTest.testBuildRawModel(DefaultModelBuilderTest.java:164)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727)
        at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
        at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
        at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
        at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
        at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
        at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)

I have tried help:evaluate, but all I get is null. I have found another issue: if an object contains #isFile() and #toFile(), the array will determine which to call.
See:

[INFO] Enter the Maven expression i.e. ${project.groupId} or 0 to exit?:
${session.multiModuleProjectDirectory.uri}
[INFO]
null object or invalid expression
[INFO] Enter the Maven expression i.e. ${project.groupId} or 0 to exit?:
${session.multiModuleProjectDirectory.uri.string}

What am I doing wrong?

The PR only tackles plugin configuration injection, not model interpolation. I'll see if/how that can be done...

Darn, then I will retry next week.

gnodet · 2023-09-22T20:24:30Z

What am I doing wrong?

The PR only tackles plugin configuration injection, not model interpolation. I'll see if/how that can be done...

Actually, this is not the problem. The problem is that the maven-help-plugin is using the PluginParameterExpressionEvaluator and not the PluginParameterExpressionEvaluatorV4. However, after also enhancing the PluginParameterExpressionEvaluator class, the next problem is that ${session.multiModuleProjectDirectory} only works on the v4 api, as the MavenSession does not have the multiModuleProjectDirectory, which is deprecated.
After fixing everything that, I do have

[INFO] Enter the Maven expression i.e. ${project.groupId} or 0 to exit?:
${session.rootDirectory.uri}
[INFO] 
<uri>file:///Users/gnodet/work/git/maven/</uri>

…ri-in-properties

michael-o · 2023-09-29T13:41:01Z

I will take a good at it again next week, thanks for the hard work @gnodet.

michael-o self-requested a review October 5, 2022 11:56

gnodet force-pushed the MNG-6437 branch from fe4e6c0 to 66467dc Compare October 5, 2022 14:56

michael-o requested changes Oct 9, 2022

View reviewed changes

gnodet force-pushed the MNG-6437 branch 2 times, most recently from 4a1b6e7 to 569de56 Compare September 22, 2023 08:13

gnodet requested a review from michael-o September 22, 2023 11:17

gnodet mentioned this pull request Sep 22, 2023

Fix unneeded/wrong substring calls in expression evaluation and put back weak references in cache #1258

Merged

[MNG-6437] Better support for path and uri in property interpolation

521a57d

gnodet force-pushed the MNG-6437 branch from 569de56 to 521a57d Compare September 22, 2023 13:06

gnodet added this to the 4.0.0-alpha-8 milestone Sep 22, 2023

michael-o requested changes Sep 22, 2023

View reviewed changes

gnodet added 2 commits September 25, 2023 09:12

Support for maven 3 api

d84f465

Support for model interpolation

3aef3e1

gnodet force-pushed the MNG-6437 branch from 4614d7c to 3aef3e1 Compare September 25, 2023 07:12

gnodet requested a review from michael-o September 25, 2023 07:29

Merge remote-tracking branch 'origin/master' into MNG-6437-path-and-u…

8753fbd

…ri-in-properties

cstamas approved these changes Oct 17, 2023

View reviewed changes

gnodet merged commit 92a82dc into apache:master Oct 17, 2023
18 checks passed

gnodet deleted the MNG-6437 branch October 18, 2023 08:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MNG-6437] Better support for path and uri in property interpolation #812

[MNG-6437] Better support for path and uri in property interpolation #812

gnodet commented Oct 5, 2022

michael-o left a comment

gnodet commented Oct 9, 2022 •

edited

Loading

gnodet commented Oct 9, 2022

gnodet commented Oct 9, 2022 •

edited

Loading

michael-o commented Sep 22, 2023

michael-o Sep 22, 2023

gnodet Sep 22, 2023

michael-o Sep 22, 2023

gnodet commented Sep 22, 2023

michael-o commented Sep 22, 2023

gnodet commented Sep 22, 2023

michael-o commented Sep 22, 2023

gnodet commented Sep 22, 2023

michael-o commented Sep 29, 2023

[MNG-6437] Better support for path and uri in property interpolation #812

[MNG-6437] Better support for path and uri in property interpolation #812

Conversation

gnodet commented Oct 5, 2022

michael-o left a comment

Choose a reason for hiding this comment

gnodet commented Oct 9, 2022 • edited Loading

gnodet commented Oct 9, 2022

gnodet commented Oct 9, 2022 • edited Loading

michael-o commented Sep 22, 2023

michael-o Sep 22, 2023

Choose a reason for hiding this comment

gnodet Sep 22, 2023

Choose a reason for hiding this comment

michael-o Sep 22, 2023

Choose a reason for hiding this comment

gnodet commented Sep 22, 2023

michael-o commented Sep 22, 2023

gnodet commented Sep 22, 2023

michael-o commented Sep 22, 2023

gnodet commented Sep 22, 2023

michael-o commented Sep 29, 2023

gnodet commented Oct 9, 2022 •

edited

Loading

gnodet commented Oct 9, 2022 •

edited

Loading