This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch maven-4.0.x
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/maven-4.0.x by this push:
new f4058491cb Fix #2486: Make Resource.addInclude() persist in project
model (#2534) (#2565)
f4058491cb is described below
commit f4058491cb1ae75d8e375d153cac56b9ef3836ee
Author: Guillaume Nodet <[email protected]>
AuthorDate: Fri Jul 4 21:45:12 2025 +0200
Fix #2486: Make Resource.addInclude() persist in project model (#2534)
(#2565)
When plugin developers tried to modify project resources using:
project.getResources().get(0).addInclude(\test\);
The addInclude() call had no effect because Resource objects were
disconnected from the underlying project model.
This fix implements a ConnectedResource pattern:
1. Created ConnectedResource class that extends Resource and maintains
references to the original SourceRoot, ProjectScope, and MavenProject
2. Override modification methods (addInclude/removeInclude/setIncludes/
setExcludes) to update both the Resource and underlying project model
3. Preserve SourceRoot ordering by replacing at the same index position
4. Modified getResources() to return ConnectedResource instances
Key benefits:
- Fixes the original issue: addInclude() now works correctly
- Preserves SourceRoot ordering during modifications
- Backward compatible: existing code continues to work
- Comprehensive: handles all modification methods
- Well-tested: includes tests for functionality and ordering
Files changed:
- MavenProject.java: Core fix implementation, made sources field
package-private
- ConnectedResource.java: New class extracted to meet file length limits
- ResourceIncludeTest.java: Comprehensive test suite
Closes #2486
(cherry picked from commit 0d7b61a4b4e0eaca83ef0517eac2e4fd9fef8fe0)
---
.../apache/maven/project/ConnectedResource.java | 130 ++++++++++++++
.../org/apache/maven/project/MavenProject.java | 11 +-
.../apache/maven/project/ResourceIncludeTest.java | 191 +++++++++++++++++++++
3 files changed, 330 insertions(+), 2 deletions(-)
diff --git
a/impl/maven-core/src/main/java/org/apache/maven/project/ConnectedResource.java
b/impl/maven-core/src/main/java/org/apache/maven/project/ConnectedResource.java
new file mode 100644
index 0000000000..ba1afa8472
--- /dev/null
+++
b/impl/maven-core/src/main/java/org/apache/maven/project/ConnectedResource.java
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.project;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.maven.api.ProjectScope;
+import org.apache.maven.api.SourceRoot;
+import org.apache.maven.impl.DefaultSourceRoot;
+import org.apache.maven.model.Resource;
+
+/**
+ * A Resource wrapper that maintains a connection to the underlying project
model.
+ * When includes/excludes are modified, the changes are propagated back to the
project's SourceRoots.
+ */
+class ConnectedResource extends Resource {
+ private final SourceRoot originalSourceRoot;
+ private final ProjectScope scope;
+ private final MavenProject project;
+
+ ConnectedResource(SourceRoot sourceRoot, ProjectScope scope, MavenProject
project) {
+ super(org.apache.maven.api.model.Resource.newBuilder()
+ .directory(sourceRoot.directory().toString())
+ .includes(sourceRoot.includes())
+ .excludes(sourceRoot.excludes())
+ .filtering(Boolean.toString(sourceRoot.stringFiltering()))
+ .build());
+ this.originalSourceRoot = sourceRoot;
+ this.scope = scope;
+ this.project = project;
+ }
+
+ @Override
+ public void addInclude(String include) {
+ // Update the underlying Resource model
+ super.addInclude(include);
+
+ // Update the project's SourceRoots
+ updateProjectSourceRoot();
+ }
+
+ @Override
+ public void removeInclude(String include) {
+ // Update the underlying Resource model
+ super.removeInclude(include);
+
+ // Update the project's SourceRoots
+ updateProjectSourceRoot();
+ }
+
+ @Override
+ public void addExclude(String exclude) {
+ // Update the underlying Resource model
+ super.addExclude(exclude);
+
+ // Update the project's SourceRoots
+ updateProjectSourceRoot();
+ }
+
+ @Override
+ public void removeExclude(String exclude) {
+ // Update the underlying Resource model
+ super.removeExclude(exclude);
+
+ // Update the project's SourceRoots
+ updateProjectSourceRoot();
+ }
+
+ @Override
+ public void setIncludes(List<String> includes) {
+ // Update the underlying Resource model
+ super.setIncludes(includes);
+
+ // Update the project's SourceRoots
+ updateProjectSourceRoot();
+ }
+
+ @Override
+ public void setExcludes(List<String> excludes) {
+ // Update the underlying Resource model
+ super.setExcludes(excludes);
+
+ // Update the project's SourceRoots
+ updateProjectSourceRoot();
+ }
+
+ private void updateProjectSourceRoot() {
+ // Convert the LinkedHashSet to a List to maintain order
+ List<SourceRoot> sourcesList = new ArrayList<>(project.sources);
+
+ // Find the index of the original SourceRoot
+ int index = -1;
+ for (int i = 0; i < sourcesList.size(); i++) {
+ SourceRoot source = sourcesList.get(i);
+ if (source.scope() == originalSourceRoot.scope()
+ && source.language() == originalSourceRoot.language()
+ &&
source.directory().equals(originalSourceRoot.directory())) {
+ index = i;
+ break;
+ }
+ }
+
+ if (index >= 0) {
+ // Replace the SourceRoot at the same position
+ SourceRoot newSourceRoot = new
DefaultSourceRoot(project.getBaseDirectory(), scope, this.getDelegate());
+ sourcesList.set(index, newSourceRoot);
+
+ // Update the project's sources, preserving order
+ project.sources.clear();
+ project.sources.addAll(sourcesList);
+ }
+ }
+}
diff --git
a/impl/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
b/impl/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
index 029a1680fe..3b934c922e 100644
--- a/impl/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
+++ b/impl/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
@@ -149,7 +149,7 @@ public class MavenProject implements Cloneable {
/**
* All sources of this project, in the order they were added.
*/
- private Set<SourceRoot> sources = new LinkedHashSet<>();
+ Set<SourceRoot> sources = new LinkedHashSet<>();
@Deprecated
private ArtifactRepository releaseArtifactRepository;
@@ -798,7 +798,10 @@ private Stream<SourceRoot> sources() {
@Override
public ListIterator<Resource> listIterator(int index) {
- return
sources().map(MavenProject::toResource).toList().listIterator(index);
+ return sources()
+ .map(sourceRoot -> toConnectedResource(sourceRoot,
scope))
+ .toList()
+ .listIterator(index);
}
@Override
@@ -828,6 +831,10 @@ private static Resource toResource(SourceRoot sourceRoot) {
.build());
}
+ private Resource toConnectedResource(SourceRoot sourceRoot, ProjectScope
scope) {
+ return new ConnectedResource(sourceRoot, scope, this);
+ }
+
private void addResource(ProjectScope scope, Resource resource) {
addSourceRoot(new DefaultSourceRoot(getBaseDirectory(), scope,
resource.getDelegate()));
}
diff --git
a/impl/maven-core/src/test/java/org/apache/maven/project/ResourceIncludeTest.java
b/impl/maven-core/src/test/java/org/apache/maven/project/ResourceIncludeTest.java
new file mode 100644
index 0000000000..cf3144a002
--- /dev/null
+++
b/impl/maven-core/src/test/java/org/apache/maven/project/ResourceIncludeTest.java
@@ -0,0 +1,191 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.project;
+
+import java.nio.file.Path;
+import java.util.List;
+
+import org.apache.maven.api.Language;
+import org.apache.maven.api.ProjectScope;
+import org.apache.maven.impl.DefaultSourceRoot;
+import org.apache.maven.model.Resource;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Test for the fix of issue #2486: Includes are not added to existing project
resource.
+ */
+class ResourceIncludeTest {
+
+ private MavenProject project;
+
+ @BeforeEach
+ void setUp() {
+ project = new MavenProject();
+ // Set a dummy pom file to establish the base directory
+ project.setFile(new java.io.File("./pom.xml"));
+
+ // Add a resource source root to the project
+ project.addSourceRoot(
+ new DefaultSourceRoot(ProjectScope.MAIN, Language.RESOURCES,
Path.of("src/main/resources")));
+ }
+
+ @Test
+ void testAddIncludeToExistingResource() {
+ // Get the first resource
+ List<Resource> resources = project.getResources();
+ assertEquals(1, resources.size(), "Should have one resource");
+
+ Resource resource = resources.get(0);
+ assertEquals(Path.of("src/main/resources").toString(),
resource.getDirectory());
+ assertTrue(resource.getIncludes().isEmpty(), "Initially should have no
includes");
+
+ // Add an include - this should work now
+ resource.addInclude("test");
+
+ // Verify the include was added
+ assertEquals(1, resource.getIncludes().size(), "Should have one
include");
+ assertEquals("test", resource.getIncludes().get(0), "Include should be
'test'");
+
+ // Verify that getting resources again still shows the include
+ List<Resource> resourcesAfter = project.getResources();
+ assertEquals(1, resourcesAfter.size(), "Should still have one
resource");
+ Resource resourceAfter = resourcesAfter.get(0);
+ assertEquals(1, resourceAfter.getIncludes().size(), "Should still have
one include");
+ assertEquals("test", resourceAfter.getIncludes().get(0), "Include
should still be 'test'");
+ }
+
+ @Test
+ void testAddMultipleIncludes() {
+ Resource resource = project.getResources().get(0);
+
+ // Add multiple includes
+ resource.addInclude("*.xml");
+ resource.addInclude("*.properties");
+
+ // Verify both includes are present
+ assertEquals(2, resource.getIncludes().size(), "Should have two
includes");
+ assertTrue(resource.getIncludes().contains("*.xml"), "Should contain
*.xml");
+ assertTrue(resource.getIncludes().contains("*.properties"), "Should
contain *.properties");
+
+ // Verify persistence
+ Resource resourceAfter = project.getResources().get(0);
+ assertEquals(2, resourceAfter.getIncludes().size(), "Should still have
two includes");
+ assertTrue(resourceAfter.getIncludes().contains("*.xml"), "Should
still contain *.xml");
+ assertTrue(resourceAfter.getIncludes().contains("*.properties"),
"Should still contain *.properties");
+ }
+
+ @Test
+ void testRemoveInclude() {
+ Resource resource = project.getResources().get(0);
+
+ // Add includes
+ resource.addInclude("*.xml");
+ resource.addInclude("*.properties");
+ assertEquals(2, resource.getIncludes().size());
+
+ // Remove one include
+ resource.removeInclude("*.xml");
+
+ // Verify only one include remains
+ assertEquals(1, resource.getIncludes().size(), "Should have one
include");
+ assertEquals("*.properties", resource.getIncludes().get(0), "Should
only have *.properties");
+
+ // Verify persistence
+ Resource resourceAfter = project.getResources().get(0);
+ assertEquals(1, resourceAfter.getIncludes().size(), "Should still have
one include");
+ assertEquals("*.properties", resourceAfter.getIncludes().get(0),
"Should still only have *.properties");
+ }
+
+ @Test
+ void testSetIncludes() {
+ Resource resource = project.getResources().get(0);
+
+ // Set includes directly
+ resource.setIncludes(List.of("*.txt", "*.md"));
+
+ // Verify includes were set
+ assertEquals(2, resource.getIncludes().size(), "Should have two
includes");
+ assertTrue(resource.getIncludes().contains("*.txt"), "Should contain
*.txt");
+ assertTrue(resource.getIncludes().contains("*.md"), "Should contain
*.md");
+
+ // Verify persistence
+ Resource resourceAfter = project.getResources().get(0);
+ assertEquals(2, resourceAfter.getIncludes().size(), "Should still have
two includes");
+ assertTrue(resourceAfter.getIncludes().contains("*.txt"), "Should
still contain *.txt");
+ assertTrue(resourceAfter.getIncludes().contains("*.md"), "Should still
contain *.md");
+ }
+
+ @Test
+ void testSourceRootOrderingPreserved() {
+ // Add multiple resource source roots
+ project.addSourceRoot(
+ new DefaultSourceRoot(ProjectScope.MAIN, Language.RESOURCES,
Path.of("src/main/resources2")));
+ project.addSourceRoot(
+ new DefaultSourceRoot(ProjectScope.MAIN, Language.RESOURCES,
Path.of("src/main/resources3")));
+
+ // Verify initial order
+ List<Resource> resources = project.getResources();
+ assertEquals(3, resources.size(), "Should have three resources");
+ assertEquals(Path.of("src/main/resources").toString(),
resources.get(0).getDirectory());
+ assertEquals(Path.of("src/main/resources2").toString(),
resources.get(1).getDirectory());
+ assertEquals(Path.of("src/main/resources3").toString(),
resources.get(2).getDirectory());
+
+ // Modify the middle resource
+ resources.get(1).addInclude("*.properties");
+
+ // Verify order is preserved after modification
+ List<Resource> resourcesAfter = project.getResources();
+ assertEquals(3, resourcesAfter.size(), "Should still have three
resources");
+ assertEquals(
+ Path.of("src/main/resources").toString(),
resourcesAfter.get(0).getDirectory());
+ assertEquals(
+ Path.of("src/main/resources2").toString(),
resourcesAfter.get(1).getDirectory());
+ assertEquals(
+ Path.of("src/main/resources3").toString(),
resourcesAfter.get(2).getDirectory());
+
+ // Verify the modification was applied to the correct resource
+ assertTrue(
+ resourcesAfter.get(1).getIncludes().contains("*.properties"),
+ "Middle resource should have the include");
+ assertTrue(resourcesAfter.get(0).getIncludes().isEmpty(), "First
resource should not have includes");
+ assertTrue(resourcesAfter.get(2).getIncludes().isEmpty(), "Third
resource should not have includes");
+ }
+
+ @Test
+ void testUnderlyingSourceRootsUpdated() {
+ Resource resource = project.getResources().get(0);
+
+ // Add an include
+ resource.addInclude("*.xml");
+
+ // Verify that the underlying SourceRoot collection was updated
+ java.util.stream.Stream<org.apache.maven.api.SourceRoot>
resourceSourceRoots =
+ project.getEnabledSourceRoots(ProjectScope.MAIN,
Language.RESOURCES);
+
+ java.util.List<org.apache.maven.api.SourceRoot> sourceRootsList =
resourceSourceRoots.toList();
+ assertEquals(1, sourceRootsList.size(), "Should have one resource
source root");
+
+ org.apache.maven.api.SourceRoot sourceRoot = sourceRootsList.get(0);
+ assertTrue(sourceRoot.includes().contains("*.xml"), "Underlying
SourceRoot should contain the include");
+ }
+}
