Copilot commented on code in PR #306:
URL:
https://github.com/apache/maven-remote-resources-plugin/pull/306#discussion_r4229747348
##########
src/main/java/org/apache/maven/plugin/resources/remote/AbstractProcessRemoteResourcesMojo.java:
##########
@@ -980,8 +979,7 @@ protected void processResourceBundles(ClassLoader
classLoader, VelocityContext c
try (CachingOutputStream os = new
CachingOutputStream(outputFile);
Reader reader =
getReader(bundle.getSourceEncoding(), appendedVmResourceFile);
Writer writer =
getWriter(bundle.getSourceEncoding(), os)) {
- Velocity.init();
- Velocity.evaluate(context, writer, "remote-resources",
reader);
+ velocity.evaluate(context, writer, "remote-resources",
reader);
Review Comment:
`VelocityEngine#evaluate(...)` requires the engine to be
initialized/configured (typically via `velocity.init(...)`). Since this hunk
removes the local `Velocity.init()` call, this will break if the instance
engine hasn’t already been initialized earlier in the mojo execution path.
Ensure the instance `velocity` is initialized once during mojo setup (before
any rendering, including appended resources), rather than relying on per-render
initialization here.
##########
src/test/java/org/apache/maven/plugin/resources/remote/RemoteResourcesMojoTest.java:
##########
@@ -254,6 +254,33 @@ public void testFilteredBundles() throws Exception {
assertTrue(data.contains("projectsSortedByOrganization: {"));
}
+ public void testFilteredAppendedResource() throws Exception {
+ final MavenProjectResourcesStub project =
createTestProject("default-appended-filtered-resource");
+ final ProcessRemoteResourcesMojo mojo =
+ lookupProcessMojoWithSettings(project, new String[]
{"test:test:1.1"});
+
+ setupDefaultProject(project);
+
+ String path = pathOf(new DefaultArtifact(
+ "test", "test", VersionRange.createFromVersion("1.1"), null,
"jar", "", new DefaultArtifactHandler()));
+ File file = new File(path);
+ file.getParentFile().mkdirs();
+ buildResourceBundle("default-appended-filtered-resource-create", null,
new String[] {"FILTER.txt.vm"}, file);
+
+ File appendedResourcesDirectory = new File(project.getBasedir(),
"src/main/appended-resources");
+ appendedResourcesDirectory.mkdirs();
+ FileUtils.fileWrite(
+ new File(appendedResourcesDirectory,
"FILTER.txt.vm").getAbsolutePath(),
+ "appended: $project.name");
Review Comment:
This test uses the same path/name (`FILTER.txt.vm`) both inside the
generated bundle JAR and as an appended resource. That makes the test harder to
reason about and can mask failures depending on precedence/order (the output
`FILTER.txt` could come from the bundled resource rather than the appended one
in some implementations). To make the intent unambiguous, prefer using distinct
filenames (e.g., bundle contains `BUNDLE_FILTER.txt.vm` while appended contains
`APPENDED_FILTER.txt.vm`) or ensure the bundle does not include `FILTER.txt.vm`
at all.
##########
src/test/java/org/apache/maven/plugin/resources/remote/RemoteResourcesMojoTest.java:
##########
@@ -254,6 +254,33 @@ public void testFilteredBundles() throws Exception {
assertTrue(data.contains("projectsSortedByOrganization: {"));
}
+ public void testFilteredAppendedResource() throws Exception {
+ final MavenProjectResourcesStub project =
createTestProject("default-appended-filtered-resource");
+ final ProcessRemoteResourcesMojo mojo =
+ lookupProcessMojoWithSettings(project, new String[]
{"test:test:1.1"});
+
+ setupDefaultProject(project);
+
+ String path = pathOf(new DefaultArtifact(
+ "test", "test", VersionRange.createFromVersion("1.1"), null,
"jar", "", new DefaultArtifactHandler()));
+ File file = new File(path);
+ file.getParentFile().mkdirs();
+ buildResourceBundle("default-appended-filtered-resource-create", null,
new String[] {"FILTER.txt.vm"}, file);
+
+ File appendedResourcesDirectory = new File(project.getBasedir(),
"src/main/appended-resources");
+ appendedResourcesDirectory.mkdirs();
+ FileUtils.fileWrite(
+ new File(appendedResourcesDirectory,
"FILTER.txt.vm").getAbsolutePath(),
+ "appended: $project.name");
+ setVariableValueToObject(mojo, "appendedResourcesDirectory",
appendedResourcesDirectory);
+
+ mojo.execute();
+
+ file = (File) getVariableValueFromObject(mojo, "outputDirectory");
+ String data = FileUtils.fileRead(new File(file, "FILTER.txt"));
+ assertTrue(data.contains("appended: Test Project
default-appended-filtered-resource"));
Review Comment:
This test uses the same path/name (`FILTER.txt.vm`) both inside the
generated bundle JAR and as an appended resource. That makes the test harder to
reason about and can mask failures depending on precedence/order (the output
`FILTER.txt` could come from the bundled resource rather than the appended one
in some implementations). To make the intent unambiguous, prefer using distinct
filenames (e.g., bundle contains `BUNDLE_FILTER.txt.vm` while appended contains
`APPENDED_FILTER.txt.vm`) or ensure the bundle does not include `FILTER.txt.vm`
at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
