elharo commented on code in PR #641:
URL: https://github.com/apache/maven-war-plugin/pull/641#discussion_r3667768846
##########
src/test/java/org/apache/maven/plugins/war/WarExplodedMojoTest.java:
##########
@@ -934,4 +942,44 @@ public void
testExplodedWarWithOutputFileNameMappingAndDuplicateDependencies(War
expectedEJBArtifact.delete();
expectedEJBDupArtifact.delete();
}
+
+ /**
+ * Test for MWAR-443: Files placed by maven-dependency-plugin in
WEB-INF/lib
+ * for provided-scope artifacts should not be deleted by the WAR plugin.
+ */
+ @InjectMojo(goal = "exploded", pom =
"src/test/resources/unit/warexplodedmojo/plugin-config.xml")
+ @MojoParameter(
+ name = "classesDirectory",
+ value =
"target/test-classes/unit/warexplodedmojo/SimpleExplodedWar-test-data/classes/")
+ @MojoParameter(
+ name = "warSourceDirectory",
+ value =
"target/test-classes/unit/warexplodedmojo/SimpleExplodedWar-test-data/source/")
+ @MojoParameter(name = "webappDirectory", value =
"target/test-classes/unit/warexplodedmojo/MWAR443Test")
+ @MojoParameter(name = "outdatedCheckPath", value = "WEB-INF/lib/")
+ @Test
+ public void
testProvidedScopeArtifactPlacedByDependencyPluginShouldNotBeDeleted(WarExplodedMojo
mojo)
+ throws Exception {
+ // Ensure session has a start time so the outdated resource detection
is active
+ when(mavenSession.getStartTime()).thenReturn(new Date());
Review Comment:
This follows the same pattern used by `WarExplodedMojoFilteringTest` (which
calls `when(mavenSession.getSystemProperties()).thenReturn(...)`). The test
harness provides a Mockito mock for `@Inject MavenSession`. The test passes
successfully with this approach.
##########
src/test/java/org/apache/maven/plugins/war/WarExplodedMojoTest.java:
##########
@@ -934,4 +942,44 @@ public void
testExplodedWarWithOutputFileNameMappingAndDuplicateDependencies(War
expectedEJBArtifact.delete();
expectedEJBDupArtifact.delete();
}
+
+ /**
+ * Test for MWAR-443: Files placed by maven-dependency-plugin in
WEB-INF/lib
+ * for provided-scope artifacts should not be deleted by the WAR plugin.
+ */
+ @InjectMojo(goal = "exploded", pom =
"src/test/resources/unit/warexplodedmojo/plugin-config.xml")
+ @MojoParameter(
+ name = "classesDirectory",
+ value =
"target/test-classes/unit/warexplodedmojo/SimpleExplodedWar-test-data/classes/")
+ @MojoParameter(
+ name = "warSourceDirectory",
+ value =
"target/test-classes/unit/warexplodedmojo/SimpleExplodedWar-test-data/source/")
+ @MojoParameter(name = "webappDirectory", value =
"target/test-classes/unit/warexplodedmojo/MWAR443Test")
+ @MojoParameter(name = "outdatedCheckPath", value = "WEB-INF/lib/")
+ @Test
+ public void
testProvidedScopeArtifactPlacedByDependencyPluginShouldNotBeDeleted(WarExplodedMojo
mojo)
+ throws Exception {
+ // Ensure session has a start time so the outdated resource detection
is active
+ when(mavenSession.getStartTime()).thenReturn(new Date());
+
+ // Setup: Create a file in WEB-INF/lib with an old timestamp,
+ // simulating a file placed by maven-dependency-plugin for a
provided-scope artifact
+ File webAppDirectory = mojo.getWebappDirectory();
+ File libDir = new File(webAppDirectory, "WEB-INF/lib");
+ libDir.mkdirs();
+ File providedJar = new File(libDir, "derbyLocale_cs-10.14.2.0.jar");
+ providedJar.createNewFile();
+ // Set timestamp to something in the past (before session start)
+ providedJar.setLastModified(0L);
+
+ // Run the mojo
+ mojo.execute();
+
+ // The file should NOT be deleted - it was placed by another plugin
+ assertTrue(providedJar.exists(), "provided-scope artifact should not
be deleted by WAR plugin");
+
+ // Cleanup
+ providedJar.delete();
+ libDir.delete();
+ }
Review Comment:
Fixed in the latest revision. Cleanup is now in a `finally` block with
proper resource cleanup.
##########
src/main/java/org/apache/maven/plugins/war/AbstractWarMojo.java:
##########
@@ -682,6 +703,43 @@ protected boolean checkAllPathsForOutdated() {
return outdatedCheckPath.equals("/");
}
+ /**
+ * Returns the set of expected target filenames for runtime-scope
artifacts.
+ * Used to avoid marking files placed by other plugins (e.g.,
maven-dependency-plugin)
+ * as outdated and subsequently deleting them (MWAR-443).
+ */
+ private Set<String> getRuntimeArtifactFileNames() {
+ Set<String> fileNames = new HashSet<>();
+ ScopeArtifactFilter filter = new
ScopeArtifactFilter(Artifact.SCOPE_RUNTIME);
+ if (project.getArtifacts() != null) {
+ for (Artifact artifact : project.getArtifacts()) {
+ if (!artifact.isOptional() && filter.include(artifact)) {
+ try {
+ String type = artifact.getType();
+ if ("jar".equals(type)
+ || "ejb".equals(type)
+ || "ejb-client".equals(type)
+ || "test-jar".equals(type)
+ || "bundle".equals(type)
+ || "par".equals(type)) {
+ String classifier = artifact.getClassifier();
+ if (classifier != null &&
!classifier.trim().isEmpty()) {
+
fileNames.add(MappingUtils.evaluateFileNameMapping(
+
MappingUtils.DEFAULT_FILE_NAME_MAPPING_CLASSIFIER, artifact));
+ } else {
+
fileNames.add(MappingUtils.evaluateFileNameMapping(
+
MappingUtils.DEFAULT_FILE_NAME_MAPPING, artifact));
+ }
Review Comment:
Addressed in the latest revision. `getRuntimeArtifactFileNames()` now checks
`getOutputFileNameMapping()` and uses it if set, mirroring the same logic as
`ArtifactsPackagingTask.getArtifactFinalName()`.
##########
src/main/java/org/apache/maven/plugins/war/AbstractWarMojo.java:
##########
@@ -682,6 +703,43 @@ protected boolean checkAllPathsForOutdated() {
return outdatedCheckPath.equals("/");
}
+ /**
+ * Returns the set of expected target filenames for runtime-scope
artifacts.
+ * Used to avoid marking files placed by other plugins (e.g.,
maven-dependency-plugin)
+ * as outdated and subsequently deleting them (MWAR-443).
+ */
+ private Set<String> getRuntimeArtifactFileNames() {
+ Set<String> fileNames = new HashSet<>();
+ ScopeArtifactFilter filter = new
ScopeArtifactFilter(Artifact.SCOPE_RUNTIME);
+ if (project.getArtifacts() != null) {
+ for (Artifact artifact : project.getArtifacts()) {
+ if (!artifact.isOptional() && filter.include(artifact)) {
+ try {
+ String type = artifact.getType();
+ if ("jar".equals(type)
+ || "ejb".equals(type)
+ || "ejb-client".equals(type)
+ || "test-jar".equals(type)
+ || "bundle".equals(type)
+ || "par".equals(type)) {
Review Comment:
Addressed in the latest revision. Extracted `isLibraryType()` to
`AbstractWarPackagingTask` and now use it from both
`ArtifactsPackagingTask.performPackaging()` and
`getRuntimeArtifactFileNames()`, eliminating the duplication.
--
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]
