Skip to content

[MWAR-443] Fix deletion of files placed by maven-dependency-plugin #641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
elharo wants to merge 1 commit into from
Closed

[MWAR-443] Fix deletion of files placed by maven-dependency-plugin #641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 64 additions & 1 deletion src/main/java/org/apache/maven/plugins/war/AbstractWarMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,15 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;

import org.apache.maven.archiver.MavenArchiveConfiguration;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
import org.apache.maven.artifact.resolver.filter.ScopeArtifactFilter;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.model.Resource;
import org.apache.maven.plugin.AbstractMojo;
Expand All @@ -42,6 +46,7 @@
import org.apache.maven.plugin.logging.Log;
import org.apache.maven.plugins.annotations.Parameter;
import org.apache.maven.plugins.war.overlay.OverlayManager;
import org.apache.maven.plugins.war.packaging.AbstractWarPackagingTask;
import org.apache.maven.plugins.war.packaging.CopyUserManifestTask;
import org.apache.maven.plugins.war.packaging.OverlayPackagingTask;
import org.apache.maven.plugins.war.packaging.WarPackagingContext;
Expand All @@ -54,6 +59,7 @@
import org.apache.maven.shared.filtering.MavenFilteringException;
import org.apache.maven.shared.filtering.MavenResourcesExecution;
import org.apache.maven.shared.filtering.MavenResourcesFiltering;
import org.apache.maven.shared.mapping.MappingUtils;
import org.apache.maven.shared.utils.StringUtils;
import org.codehaus.plexus.archiver.jar.JarArchiver;
import org.codehaus.plexus.archiver.manager.ArchiverManager;
Expand Down Expand Up @@ -655,6 +661,10 @@ private class DefaultWarPackagingContext implements WarPackagingContext {
outdatedCheckPath = outdatedCheckPath.replace('/', '\\');
}
}
// MWAR-443: Compute expected filenames for runtime-scope artifacts
// so we don't mark files placed by other plugins (e.g., maven-dependency-plugin)
// as outdated and subsequently delete them.
final Set<String> runtimeArtifactFileNames = getRuntimeArtifactFileNames();
Files.walkFileTree(webappDirectory.toPath(), new SimpleFileVisitor<Path>() {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
Expand All @@ -665,7 +675,19 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
.relativize(file)
.toString();
if (checkAllPathsForOutdated() || path.startsWith(outdatedCheckPath)) {
outdatedResources.add(path);
// MWAR-443: For files under the artifact lib directory,
// only mark as outdated if they match a runtime-scope artifact
// that the WAR plugin would copy. This prevents deleting files
// placed by other plugins (e.g., maven-dependency-plugin) for
// non-runtime-scope dependencies.
String normalizedPath = path.replace('\\', '/');
if (normalizedPath.startsWith("WEB-INF/lib/")
&& !runtimeArtifactFileNames.contains(
file.toFile().getName())) {
// Skip: file was placed by another plugin, not managed by WAR plugin
} else {
outdatedResources.add(path);
}
}
}
return super.visitFile(file, attrs);
Expand All @@ -682,6 +704,47 @@ 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)
&& AbstractWarPackagingTask.isLibraryType(artifact.getType())) {
try {
String targetFileName;
if (getOutputFileNameMapping() != null) {
targetFileName =
MappingUtils.evaluateFileNameMapping(getOutputFileNameMapping(), artifact);
} else {
String classifier = artifact.getClassifier();
if (classifier != null && !classifier.trim().isEmpty()) {
targetFileName = MappingUtils.evaluateFileNameMapping(
MappingUtils.DEFAULT_FILE_NAME_MAPPING_CLASSIFIER, artifact);
} else {
targetFileName = MappingUtils.evaluateFileNameMapping(
MappingUtils.DEFAULT_FILE_NAME_MAPPING, artifact);
}
Comment on lines +726 to +733

@elharo elharo Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in the latest revision. getRuntimeArtifactFileNames() now checks getOutputFileNameMapping() and uses it if set, mirroring the same logic as ArtifactsPackagingTask.getArtifactFinalName().

}
if ("par".equals(artifact.getType())) {
targetFileName = targetFileName.substring(0, targetFileName.lastIndexOf('.')) + ".jar";
}
fileNames.add(targetFileName);
} catch (Exception e) {
getLog().debug("Could not compute expected filename for artifact: " + artifact, e);
}
}
}
}
return fileNames;
}

@Override
public MavenProject getProject() {
return project;
Expand Down
26 changes: 25 additions & 1 deletion src/main/java/org/apache/maven/plugins/war/packaging/AbstractWarPackagingTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@
// fix for MWAR-36, ensures that the parent dir are created first
targetFile.getParentFile().mkdirs();

context.getMavenFileFilter().copyFile(file, targetFile, true, context.getFilterWrappers(), encoding);

Check warning on line 259 in src/main/java/org/apache/maven/plugins/war/packaging/AbstractWarPackagingTask.java

View workflow job for this annotation

GitHub Actions / Verify / ubuntu-latest jdk-17-zulu 3.10.0-rc-1 

copyFile(java.io.File,java.io.File,boolean,java.util.List<org.apache.maven.shared.filtering.FilterWrapper>,java.lang.String) in org.apache.maven.shared.filtering.MavenFileFilter has been deprecated
} catch (MavenFilteringException e) {
throw new MojoExecutionException(e.getMessage(), e);
}
Expand Down Expand Up @@ -334,7 +334,7 @@

try {
JarArchiver archiver = context.getJarArchiver();
archiver.addDirectory(source);

Check warning on line 337 in src/main/java/org/apache/maven/plugins/war/packaging/AbstractWarPackagingTask.java

View workflow job for this annotation

GitHub Actions / Verify / ubuntu-latest jdk-17-zulu 3.10.0-rc-1 

addDirectory(java.io.File) in org.codehaus.plexus.archiver.AbstractArchiver has been deprecated
archiver.setDestFile(destination);
archiver.createArchive();
} catch (ArchiverException e) {
Expand All @@ -346,6 +346,14 @@
FileUtils.copyFile(source.getCanonicalFile(), destination);
// preserve timestamp
destination.setLastModified(readAttributes.lastModifiedTime().toMillis());
// normalize permissions: clear executable, set read-for-all, write-for-owner
// on all copied files (not just WEB-INF/lib jars)
boolean ok = destination.setExecutable(false, false);
ok &= destination.setReadable(true, false);
ok &= destination.setWritable(true, true);
if (!ok) {
context.getLog().debug("Could not normalize permissions for " + targetFilename);
}
context.getLog().debug(" + " + targetFilename + " has been copied.");
}
return true;
Expand All @@ -360,7 +368,8 @@
* @throws java.io.IOException if an error occurred while reading the file
*/
protected String getEncoding(File webXml) throws IOException {
try (XmlStreamReader xmlReader = new XmlStreamReader(webXml)) {
try (XmlStreamReader xmlReader =
XmlStreamReader.builder().setFile(webXml).get()) {
return xmlReader.getEncoding();
}
}
Expand Down Expand Up @@ -496,4 +505,19 @@
}
return true;
}

/**
* Returns {@code true} if the artifact with the given type would be placed in {@code WEB-INF/lib/}.
*
* @param type the artifact type
* @return {@code true} if the type is a library type that goes to WEB-INF/lib
*/
public static boolean isLibraryType(String type) {
return "jar".equals(type)
|| "ejb".equals(type)
|| "ejb-client".equals(type)
|| "test-jar".equals(type)
|| "bundle".equals(type)
|| "par".equals(type);
}
}
12 changes: 4 additions & 8 deletions src/main/java/org/apache/maven/plugins/war/packaging/ArtifactsPackagingTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,10 @@ public void performPackaging(WarPackagingContext context) throws MojoExecutionEx
copyFile(id, context, artifact.getFile(), MODULES_PATH + targetFileName);
} else if ("xar".equals(type)) {
copyFile(id, context, artifact.getFile(), EXTENSIONS_PATH + targetFileName);
} else if ("jar".equals(type)
|| "ejb".equals(type)
|| "ejb-client".equals(type)
|| "test-jar".equals(type)
|| "bundle".equals(type)) {
copyFile(id, context, artifact.getFile(), LIB_PATH + targetFileName);
} else if ("par".equals(type)) {
targetFileName = targetFileName.substring(0, targetFileName.lastIndexOf('.')) + ".jar";
} else if (isLibraryType(type)) {
if ("par".equals(type)) {
targetFileName = targetFileName.substring(0, targetFileName.lastIndexOf('.')) + ".jar";
}
copyFile(id, context, artifact.getFile(), LIB_PATH + targetFileName);
} else if ("war".equals(type)) {
// Nothing to do here, it is an overlay and it's already handled
Expand Down
51 changes: 51 additions & 0 deletions src/test/java/org/apache/maven/plugins/war/WarExplodedMojoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,19 @@
*/
package org.apache.maven.plugins.war;

import javax.inject.Inject;

import java.io.File;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Locale;

import org.apache.maven.api.plugin.testing.InjectMojo;
import org.apache.maven.api.plugin.testing.MojoExtension;
import org.apache.maven.api.plugin.testing.MojoParameter;
import org.apache.maven.api.plugin.testing.MojoTest;
import org.apache.maven.artifact.handler.DefaultArtifactHandler;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.plugin.testing.stubs.ArtifactStub;
import org.apache.maven.plugins.war.stub.AarArtifactStub;
import org.apache.maven.plugins.war.stub.EJBArtifactStub;
Expand All @@ -50,10 +54,14 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.when;

@MojoTest
public class WarExplodedMojoTest {

@Inject
private MavenSession mavenSession;

@InjectMojo(goal = "exploded", pom = "src/test/resources/unit/warexplodedmojo/plugin-config.xml")
@MojoParameter(
name = "classesDirectory",
Expand Down Expand Up @@ -934,4 +942,47 @@ 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.
// Uses same pattern as WarExplodedMojoFilteringTest which also calls
// when(mavenSession.get...()).thenReturn(...)
when(mavenSession.getStartTime()).thenReturn(new Date());
Comment on lines +960 to +965

@elharo elharo Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


File webAppDirectory = mojo.getWebappDirectory();
File libDir = new File(webAppDirectory, "WEB-INF/lib");
File providedJar = new File(libDir, "derbyLocale_cs-10.14.2.0.jar");
try {
// 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
libDir.mkdirs();
providedJar.createNewFile();
// Set timestamp to something in the past (before session start)
providedJar.setLastModified(0L);

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");
} finally {
// Cleanup
providedJar.delete();
libDir.delete();
}
}
Comment on lines +967 to +987

@elharo elharo Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the latest revision. Cleanup is now in a finally block with proper resource cleanup.

}
Loading