This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new 63e4736b3 Bug: GH-1711 make sure last wins (#1712)
63e4736b3 is described below
commit 63e4736b333c5d4ae5a8afb2a4728b2074a59cb5
Author: Tamas Cservenak <[email protected]>
AuthorDate: Fri Dec 12 13:50:35 2025 +0100
Bug: GH-1711 make sure last wins (#1712)
Nodes once created were immutable, now structure is made mutable for
parsing, with "last wins" strategy. Also, split and simplify things, as prefix
tree does not even need stop/allow, is groupTree only thing.
Fixes #1711
---
.../GroupIdRemoteRepositoryFilterSource.java | 9 +--
.../internal/impl/filter/ruletree/GroupTree.java | 72 ++++++++++++++++------
.../aether/internal/impl/filter/ruletree/Node.java | 40 +++---------
.../internal/impl/filter/ruletree/PrefixTree.java | 14 +++--
.../impl/filter/ruletree/GroupTreeTest.java | 63 ++++++++++++++++---
src/site/markdown/remote-repository-filtering.md | 2 +-
6 files changed, 128 insertions(+), 72 deletions(-)
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/GroupIdRemoteRepositoryFilterSource.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/GroupIdRemoteRepositoryFilterSource.java
index dcf25d268..c98953af0 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/GroupIdRemoteRepositoryFilterSource.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/GroupIdRemoteRepositoryFilterSource.java
@@ -254,7 +254,7 @@ public final class GroupIdRemoteRepositoryFilterSource
extends RemoteRepositoryF
rules(session)
.compute(normalized, (k, v) -> {
if (v == null || v == DISABLED || v ==
ENABLED_NO_INPUT) {
- v = new GroupTree("");
+ v = GroupTree.create("record");
}
return v;
})
@@ -277,15 +277,16 @@ public final class GroupIdRemoteRepositoryFilterSource
extends RemoteRepositoryF
normalizeRemoteRepository(session, remoteRepository),
r -> loadRepositoryRules(session, r));
}
- private static final GroupTree DISABLED = new GroupTree("disabled");
- private static final GroupTree ENABLED_NO_INPUT = new
GroupTree("enabled-no-input");
+ private static final GroupTree DISABLED = GroupTree.create("disabled");
+ private static final GroupTree ENABLED_NO_INPUT =
GroupTree.create("enabled-no-input");
private GroupTree loadRepositoryRules(RepositorySystemSession session,
RemoteRepository remoteRepository) {
if (isRepositoryFilteringEnabled(session, remoteRepository)) {
Path filePath = ruleFile(session, remoteRepository);
if (Files.isReadable(filePath)) {
try (Stream<String> lines = Files.lines(filePath,
StandardCharsets.UTF_8)) {
- GroupTree groupTree = new GroupTree("");
+ GroupTree groupTree =
+
GroupTree.create(filePath.getFileName().toString());
int rules = groupTree.loadNodes(lines);
logger.info("Loaded {} group rules for remote repository
{}", rules, remoteRepository.getId());
if (logger.isDebugEnabled()) {
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/GroupTree.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/GroupTree.java
index 7aaf74610..15d2e0510 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/GroupTree.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/GroupTree.java
@@ -41,6 +41,7 @@ import static java.util.stream.Collectors.toList;
* means "by default deny" (same effect as when this character is not present
in file). Use of limiter modifier
* on "root" like {@code "=*"} has no effect, is simply ignored.
*
+ * <p>
* Examples:
* <pre>
* {@code
@@ -58,10 +59,19 @@ import static java.util.stream.Collectors.toList;
* and "allow {@code org.apache.bar} groupID ONLY".
*
* <p>
- * In case of conflicting rules, parsing happens by "first wins", so line
closer to first line in file "wins", and conflicting
- * line is ignored.
+ * In case of conflicting rules, parsing happens by "last wins", so line
closer to last line in file "wins", and conflicting
+ * line value is lost.
*/
-public class GroupTree extends Node {
+public class GroupTree extends Node<GroupTree> {
+ /**
+ * Creates root, that is special: accept is never null.
+ */
+ public static GroupTree create(String name) {
+ GroupTree result = new GroupTree(name);
+ result.accept = false;
+ return result;
+ }
+
private static final String ROOT = "*";
private static final String MOD_EXCLUSION = "!";
private static final String MOD_STOP = "=";
@@ -70,8 +80,11 @@ public class GroupTree extends Node {
return Arrays.stream(groupId.split("\\.")).filter(e ->
!e.isEmpty()).collect(toList());
}
- public GroupTree(String name) {
- super(name, false, false);
+ private boolean stop;
+ private Boolean accept;
+
+ private GroupTree(String name) {
+ super(name);
}
public int loadNodes(Stream<String> linesStream) {
@@ -86,10 +99,10 @@ public class GroupTree extends Node {
public boolean loadNode(String line) {
if (!line.startsWith("#") && !line.trim().isEmpty()) {
- Node currentNode = this;
- boolean allow = true;
+ GroupTree currentNode = this;
+ boolean accept = true;
if (line.startsWith(MOD_EXCLUSION)) {
- allow = false;
+ accept = false;
line = line.substring(MOD_EXCLUSION.length());
}
boolean stop = false;
@@ -98,14 +111,17 @@ public class GroupTree extends Node {
line = line.substring(MOD_STOP.length());
}
if (ROOT.equals(line)) {
- this.setAllow(allow);
+ this.accept = accept;
return true;
}
List<String> groupElements = elementsOfGroup(line);
for (String groupElement : groupElements.subList(0,
groupElements.size() - 1)) {
- currentNode = currentNode.addSibling(groupElement, false,
null);
+ currentNode =
currentNode.siblings.computeIfAbsent(groupElement, GroupTree::new);
}
- currentNode.addSibling(groupElements.get(groupElements.size() -
1), stop, allow);
+ String lastElement = groupElements.get(groupElements.size() - 1);
+ currentNode = currentNode.siblings.computeIfAbsent(lastElement,
GroupTree::new);
+ currentNode.stop = stop;
+ currentNode.accept = accept;
return true;
}
return false;
@@ -115,20 +131,36 @@ public class GroupTree extends Node {
final List<String> current = new ArrayList<>();
final List<String> groupElements = elementsOfGroup(groupId);
Boolean accepted = null;
- Node currentNode = this;
+ GroupTree currentNode = this;
for (String groupElement : groupElements) {
current.add(groupElement);
- currentNode = currentNode.getSibling(groupElement);
+ currentNode = currentNode.siblings.get(groupElement);
if (currentNode == null) {
+ // we stepped off the tree; use value we got so far
break;
+ } else if (currentNode.stop && groupElements.equals(current)) {
+ // exact match
+ accepted = currentNode.accept;
+ break;
+ } else if (!currentNode.stop && currentNode.accept != null) {
+ // "inherit" if not STOP and allow is set; and most probably
we loop more
+ accepted = currentNode.accept;
}
- if (currentNode.isStop() && groupElements.equals(current)) {
- accepted = currentNode.isAllow();
- } else if (!currentNode.isStop() && currentNode.isAllow() != null)
{
- accepted = currentNode.isAllow();
- }
}
- // use `accepted`, if defined; otherwise fallback to root (it always
has `allow` set)
- return accepted != null ? accepted : this.isAllow();
+ // use 'accepted', if defined; otherwise fallback to root (it always
has 'allow' set)
+ return accepted != null ? accepted : this.accept;
+ }
+
+ @Override
+ public String toString() {
+ return (accept != null ? (accept ? "+" : "-") : "?") + (stop ? "=" :
"") + name;
+ }
+
+ @Override
+ public void dump(String prefix) {
+ System.out.println(prefix + this);
+ for (GroupTree node : siblings.values()) {
+ node.dump(prefix + " ");
+ }
}
}
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/Node.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/Node.java
index beccf29c2..1006666cf 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/Node.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/Node.java
@@ -18,22 +18,18 @@
*/
package org.eclipse.aether.internal.impl.filter.ruletree;
-import java.util.HashMap;
+import java.util.concurrent.ConcurrentHashMap;
/**
* A tree structure with rules.
*/
-class Node {
- private final String name;
- private final boolean stop;
- private Boolean allow;
- private final HashMap<String, Node> siblings;
+abstract class Node<N extends Node<?>> {
+ protected final String name;
+ protected final ConcurrentHashMap<String, N> siblings;
- protected Node(String name, boolean stop, Boolean allow) {
+ protected Node(String name) {
this.name = name;
- this.stop = stop;
- this.allow = allow;
- this.siblings = new HashMap<>();
+ this.siblings = new ConcurrentHashMap<>();
}
public String getName() {
@@ -44,34 +40,14 @@ class Node {
return siblings.isEmpty();
}
- public boolean isStop() {
- return stop;
- }
-
- public Boolean isAllow() {
- return allow;
- }
-
- public void setAllow(Boolean allow) {
- this.allow = allow;
- }
-
- protected Node addSibling(String name, boolean stop, Boolean allow) {
- return siblings.computeIfAbsent(name, n -> new Node(n, stop, allow));
- }
-
- protected Node getSibling(String name) {
- return siblings.get(name);
- }
-
@Override
public String toString() {
- return (allow != null ? (allow ? "+" : "-") : "?") + (stop ? "=" : "")
+ name;
+ return name;
}
public void dump(String prefix) {
System.out.println(prefix + this);
- for (Node node : siblings.values()) {
+ for (N node : siblings.values()) {
node.dump(prefix + " ");
}
}
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/PrefixTree.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/PrefixTree.java
index da11d65d4..f5f510ad2 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/PrefixTree.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/PrefixTree.java
@@ -34,6 +34,7 @@ import static java.util.stream.Collectors.toList;
* </ul>
* By default, artifact is allowed if layout converted path of it has a
matching prefix in this file.
*
+ * <p>
* Example prefix files:
* <ul>
* <li><a
href="https://repo.maven.apache.org/maven2/.meta/prefixes.txt";>Maven
Central</a></li>
@@ -41,13 +42,13 @@ import static java.util.stream.Collectors.toList;
* <li><a
href="https://repo.eclipse.org/content/repositories/tycho/.meta/prefixes.txt";>Eclipse
Tycho</a></li>
* </ul>
*/
-public class PrefixTree extends Node {
+public class PrefixTree extends Node<PrefixTree> {
private static List<String> elementsOfPath(final String path) {
return Arrays.stream(path.split("/")).filter(e ->
!e.isEmpty()).collect(toList());
}
public PrefixTree(String name) {
- super(name, false, null);
+ super(name);
}
public int loadNodes(Stream<String> linesStream) {
@@ -62,9 +63,9 @@ public class PrefixTree extends Node {
public boolean loadNode(String line) {
if (!line.startsWith("#") && !line.trim().isEmpty()) {
- Node currentNode = this;
+ PrefixTree currentNode = this;
for (String element : elementsOfPath(line)) {
- currentNode = currentNode.addSibling(element, false, null);
+ currentNode = currentNode.siblings.computeIfAbsent(element,
PrefixTree::new);
}
return true;
}
@@ -73,13 +74,14 @@ public class PrefixTree extends Node {
public boolean acceptedPath(String path) {
final List<String> pathElements = elementsOfPath(path);
- Node currentNode = this;
+ PrefixTree currentNode = this;
for (String pathElement : pathElements) {
- currentNode = currentNode.getSibling(pathElement);
+ currentNode = currentNode.siblings.get(pathElement);
if (currentNode == null || currentNode.isLeaf()) {
break;
}
}
+ // path is accepted if its elements lead to an existing node that is a
leaf node
return currentNode != null && currentNode.isLeaf();
}
}
diff --git
a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/filter/ruletree/GroupTreeTest.java
b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/filter/ruletree/GroupTreeTest.java
index b47d54d9a..d7ccc571a 100644
---
a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/filter/ruletree/GroupTreeTest.java
+++
b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/filter/ruletree/GroupTreeTest.java
@@ -34,7 +34,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
public class GroupTreeTest {
@Test
void smoke() {
- GroupTree groupTree = new GroupTree("root");
+ GroupTree groupTree = GroupTree.create("root");
groupTree.loadNodes(Stream.of(
"# comment",
"",
@@ -42,6 +42,8 @@ public class GroupTreeTest {
"!=org.apache.maven.foo",
"!org.apache.maven.bar",
"=org.apache.baz"));
+ groupTree.dump("");
+
assertTrue(groupTree.acceptedGroupId("org.apache.maven"));
assertTrue(groupTree.acceptedGroupId("org.apache.maven.aaa"));
@@ -59,7 +61,7 @@ public class GroupTreeTest {
@Test
void smokeWithPositiveDefault() {
- GroupTree groupTree = new GroupTree("root");
+ GroupTree groupTree = GroupTree.create("root");
groupTree.loadNodes(Stream.of(
"# comment",
"",
@@ -68,6 +70,8 @@ public class GroupTreeTest {
"!org.apache.maven.bar",
"=org.apache.baz",
"*"));
+ groupTree.dump("");
+
assertTrue(groupTree.acceptedGroupId("org.apache.maven"));
assertTrue(groupTree.acceptedGroupId("org.apache.maven.aaa"));
@@ -85,8 +89,10 @@ public class GroupTreeTest {
@Test
void smokeWithPositiveDefaultExclusionsOnly() {
- GroupTree groupTree = new GroupTree("root");
+ GroupTree groupTree = GroupTree.create("root");
groupTree.loadNodes(Stream.of("# comment", "*",
"!org.apache.maven.foo", "!=org.apache.maven.bar"));
+ groupTree.dump("");
+
// no applicable rule; root=ALLOW
assertTrue(groupTree.acceptedGroupId("org.apache.maven"));
assertTrue(groupTree.acceptedGroupId("org.apache.maven.aaa"));
@@ -105,7 +111,7 @@ public class GroupTreeTest {
@Test
void smokeWithNegativeDefault() {
- GroupTree groupTree = new GroupTree("root");
+ GroupTree groupTree = GroupTree.create("root");
groupTree.loadNodes(Stream.of(
"# comment",
"",
@@ -114,6 +120,8 @@ public class GroupTreeTest {
"!org.apache.maven.bar",
"=org.apache.baz",
"!*"));
+ groupTree.dump("");
+
assertTrue(groupTree.acceptedGroupId("org.apache.maven"));
assertTrue(groupTree.acceptedGroupId("org.apache.maven.aaa"));
@@ -132,7 +140,7 @@ public class GroupTreeTest {
@Test
void implicitAndExplicitDefaultIsSame() {
// w/o asterisk: uses coded defaults
- GroupTree implicitTree = new GroupTree("root");
+ GroupTree implicitTree = GroupTree.create("root");
implicitTree.loadNodes(Stream.of(
"# comment",
"",
@@ -140,9 +148,11 @@ public class GroupTreeTest {
"!=org.apache.maven.foo",
"!org.apache.maven.bar",
"=org.apache.baz"));
+ implicitTree.dump("");
+
HashMap<String, Boolean> implicitResults = new HashMap<>();
// w/ asterisk: set to same value as default (should cause no change)
- GroupTree explicitTree = new GroupTree("root");
+ GroupTree explicitTree = GroupTree.create("root");
explicitTree.loadNodes(Stream.of(
"# comment",
"",
@@ -175,9 +185,21 @@ public class GroupTreeTest {
GroupTree groupTree;
// REPRODUCER as given
- groupTree = new GroupTree("root");
+ groupTree = GroupTree.create("root");
// this is redundant, as 'org.apache' IMPLIES
'org.apache.maven.plugins'
groupTree.loadNodes(Stream.of("# comment", "", "org.apache",
"org.apache.maven.plugins"));
+ groupTree.dump("");
+
+ assertTrue(groupTree.acceptedGroupId("org.apache")); // this is given
+ assertTrue(groupTree.acceptedGroupId("org.apache.maven")); // implied
by first
+ assertTrue(groupTree.acceptedGroupId("org.apache.maven.plugins")); //
implied by first (line is redundant)
+ assertTrue(groupTree.acceptedGroupId("org.apache.maven.plugins.foo"));
// implied by first
+
+ // INVERTED REPRODUCER as given
+ groupTree = GroupTree.create("root");
+ // reproducer in opposite order
+ groupTree.loadNodes(Stream.of("# comment", "",
"org.apache.maven.plugins", "org.apache"));
+ groupTree.dump("");
assertTrue(groupTree.acceptedGroupId("org.apache")); // this is given
assertTrue(groupTree.acceptedGroupId("org.apache.maven")); // implied
by first
@@ -185,8 +207,9 @@ public class GroupTreeTest {
assertTrue(groupTree.acceptedGroupId("org.apache.maven.plugins.foo"));
// implied by first
// FIXED
- groupTree = new GroupTree("root");
+ groupTree = GroupTree.create("root");
groupTree.loadNodes(Stream.of("# comment", "", "=org.apache",
"org.apache.maven.plugins"));
+ groupTree.dump("");
assertTrue(groupTree.acceptedGroupId("org.apache")); // this is given
(=)
assertFalse(groupTree.acceptedGroupId("org.apache.maven")); // not
allowed
@@ -194,12 +217,34 @@ public class GroupTreeTest {
assertTrue(groupTree.acceptedGroupId("org.apache.maven.plugins.foo"));
// implied by above
// MIXED
- groupTree = new GroupTree("root");
+ groupTree = GroupTree.create("root");
groupTree.loadNodes(Stream.of("# comment", "", "org.apache",
"!=org.apache.maven.plugins"));
+ groupTree.dump("");
assertTrue(groupTree.acceptedGroupId("org.apache")); // this is given
assertTrue(groupTree.acceptedGroupId("org.apache.maven")); // implied
by first
assertFalse(groupTree.acceptedGroupId("org.apache.maven.plugins")); //
this is given (!=)
assertTrue(groupTree.acceptedGroupId("org.apache.maven.plugins.foo"));
// implied by first
}
+
+ @Test
+ void gh1711() {
+ GroupTree groupTree;
+
+ // "last wins" 1
+ groupTree = GroupTree.create("root");
+ groupTree.loadNodes(Stream.of("org.apache", "!org.apache"));
+ groupTree.dump("");
+
+ assertFalse(groupTree.acceptedGroupId("org.apache")); // last wins
+ assertFalse(groupTree.acceptedGroupId("org.apache.maven")); // last
wins
+
+ // "last wins" 2
+ groupTree = GroupTree.create("root");
+ groupTree.loadNodes(Stream.of("org.apache", "!org.apache",
"org.apache"));
+ groupTree.dump("");
+
+ assertTrue(groupTree.acceptedGroupId("org.apache")); // last wins
+ assertTrue(groupTree.acceptedGroupId("org.apache.maven")); // last wins
+ }
}
diff --git a/src/site/markdown/remote-repository-filtering.md
b/src/site/markdown/remote-repository-filtering.md
index 8012895fd..7e9dbd1c3 100644
--- a/src/site/markdown/remote-repository-filtering.md
+++ b/src/site/markdown/remote-repository-filtering.md
@@ -188,7 +188,7 @@ Be aware: In case a line with single asterisk `*` is
present, the whole logic of
hence there is no need to add "allowed entries" (they are allowed by default),
but one can add "disallowed entries" by
adding `!com.foo` and alike.
-Conflicting rules: rule parser is intentionally trivial, so in case of
conflicting rules the "first wins" strategy is
+Conflicting rules: rule parser is intentionally trivial, so in case of
conflicting rules the "last wins" strategy is
applied. Ideally, user should keep files sorted or handle them in a way one
can detect conflicts in it.
## Operation
