Bug: Filter fixes (#1655)

Three major issues: 
* (prefix) one was that it tried to fix the Maven issue of remote repository uniqueness (globally), and that attempt was in fact wrong, fix is elsewhere. The reason it tried to do this is in fact to circumvent locking issues #1644.
* (manager) other issue was filter lifecycle, manager did it wrongly (data is _inherited on customized sessions_): the bug caused that one session was used to acquire filters and same filter got used for potentially other (maybe even reconfigured) session. This overlook forced filters to implement hoops and loops (to prevent recursion), but also prevented any third party code to have saying in filter operation.
* (both filters) make sure RemoteRepositories used as keys for prefixes are _same_ (normalized as bare)

Changes:
* fix manager to make sure filters and session are aligned
* fix/simplify prefix filter recursion prevention
* introduce public and documented way to inhibit prefix discovery

Note: as this PR now removes  the "hack" (that in fact tried to circumvent #1644) we are now back at state we were before #1575 (the "by make repositories unique" bit). As we see, this change (without locking fix) will cause Maven IT failures, as ITs are executed in parallel, and "prefix discovery" initiated over same local repository will/may cause locking conflicts and hence timeouts (result is sporadically failing ITs due locking timeouts). Hence, this fix is to be followed by fix for #1644 

Fixes #1654 
Fixes #1667

Author: cstamas

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/DefaultRemoteRepositoryFilterManager.java

@@ -59,7 +59,9 @@ public DefaultRemoteRepositoryFilterManager(Map<String, RemoteRepositoryFilterSo
 
     @Override
     public RemoteRepositoryFilter getRemoteRepositoryFilter(RepositorySystemSession session) {
-        return (RemoteRepositoryFilter) session.getData().computeIfAbsent(INSTANCE_KEY, () -> {
+        // use session specific key to distinguish between "derived" sessions
+        String instanceSpecificKey = INSTANCE_KEY + "." + session.hashCode();
+        return (RemoteRepositoryFilter) session.getData().computeIfAbsent(instanceSpecificKey, () -> {
             HashMap<String, RemoteRepositoryFilter> filters = new HashMap<>();
             for (Map.Entry<String, RemoteRepositoryFilterSource> entry : sources.entrySet()) {
                 RemoteRepositoryFilter filter = entry.getValue().getRemoteRepositoryFilter(session);

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/GroupIdRemoteRepositoryFilterSource.java

@@ -109,6 +109,21 @@ public final class GroupIdRemoteRepositoryFilterSource extends RemoteRepositoryF
 
     public static final boolean DEFAULT_ENABLED = true;
 
+    /**
+     * Configuration to skip the GroupId filter for given request. This configuration is evaluated and if {@code true}
+     * the GroupId remote filter will not kick in.
+     *
+     * @since 2.0.14
+     * @configurationSource {@link RepositorySystemSession#getConfigProperties()}
+     * @configurationType {@link java.lang.Boolean}
+     * @configurationRepoIdSuffix Yes
+     * @configurationDefaultValue {@link #DEFAULT_SKIPPED}
+     */
+    public static final String CONFIG_PROP_SKIPPED =
+            RemoteRepositoryFilterSourceSupport.CONFIG_PROPS_PREFIX + NAME + ".skipped";
+
+    public static final boolean DEFAULT_SKIPPED = false;
+
     /**
      * The basedir where to store filter files. If path is relative, it is resolved from local repository root.
      *
@@ -160,15 +175,22 @@ public GroupIdRemoteRepositoryFilterSource(
 
     @Override
     protected boolean isEnabled(RepositorySystemSession session) {
-        return ConfigUtils.getBoolean(session, DEFAULT_ENABLED, CONFIG_PROP_ENABLED);
+        return ConfigUtils.getBoolean(session, DEFAULT_ENABLED, CONFIG_PROP_ENABLED)
+                && !ConfigUtils.getBoolean(session, DEFAULT_SKIPPED, CONFIG_PROP_SKIPPED);
     }
 
     private boolean isRepositoryFilteringEnabled(RepositorySystemSession session, RemoteRepository remoteRepository) {
         if (isEnabled(session)) {
             return ConfigUtils.getBoolean(
-                    session,
-                    ConfigUtils.getBoolean(session, true, CONFIG_PROP_ENABLED + ".*"),
-                    CONFIG_PROP_ENABLED + "." + remoteRepository.getId());
+                            session,
+                            DEFAULT_ENABLED,
+                            CONFIG_PROP_ENABLED + "." + remoteRepository.getId(),
+                            CONFIG_PROP_ENABLED + ".*")
+                    && !ConfigUtils.getBoolean(
+                            session,
+                            DEFAULT_SKIPPED,
+                            CONFIG_PROP_SKIPPED + "." + remoteRepository.getId(),
+                            CONFIG_PROP_SKIPPED + ".*");
         }
         return false;
     }
@@ -193,10 +215,11 @@ public void postProcess(RepositorySystemSession session, List<ArtifactResult> ar
                     if (isRepositoryFilteringEnabled(session, remoteRepository)) {
                         ruleFile(session, remoteRepository); // populate it; needed for save
                         String line = "=" + artifactResult.getArtifact().getGroupId();
+                        RemoteRepository normalized = normalizeRemoteRepository(session, remoteRepository);
                         recordedRules
-                                .computeIfAbsent(remoteRepository, k -> new TreeSet<>())
+                                .computeIfAbsent(normalized, k -> new TreeSet<>())
                                 .add(line);
-                        rules.compute(remoteRepository, (k, v) -> {
+                        rules.compute(normalized, (k, v) -> {
                                     if (v == null || v == GroupTree.SENTINEL) {
                                         v = new GroupTree("");
                                     }
@@ -210,15 +233,16 @@ public void postProcess(RepositorySystemSession session, List<ArtifactResult> ar
     }
 
     private Path ruleFile(RepositorySystemSession session, RemoteRepository remoteRepository) {
-        return ruleFiles.computeIfAbsent(remoteRepository, r -> getBasedir(
+        return ruleFiles.computeIfAbsent(normalizeRemoteRepository(session, remoteRepository), r -> getBasedir(
                         session, LOCAL_REPO_PREFIX_DIR, CONFIG_PROP_BASEDIR, false)
                 .resolve(GROUP_ID_FILE_PREFIX
                         + RepositoryIdHelper.cachedIdToPathSegment(session).apply(remoteRepository)
                         + GROUP_ID_FILE_SUFFIX));
     }
 
     private GroupTree cacheRules(RepositorySystemSession session, RemoteRepository remoteRepository) {
-        return rules.computeIfAbsent(remoteRepository, r -> loadRepositoryRules(session, r));
+        return rules.computeIfAbsent(
+                normalizeRemoteRepository(session, remoteRepository), r -> loadRepositoryRules(session, r));
     }
 
     private GroupTree loadRepositoryRules(RepositorySystemSession session, RemoteRepository remoteRepository) {

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/PrefixesRemoteRepositoryFilterSource.java

@@ -25,7 +25,6 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collections;
-import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Supplier;
 
@@ -106,9 +105,7 @@ public final class PrefixesRemoteRepositoryFilterSource extends RemoteRepository
      * <strong>Initial setup:</strong> Don't provide any files - rely on auto-discovery as repositories are accessed.
      * <strong>Override when needed:</strong> Create {@code prefixes-myrepoId.txt} files in {@code .mvn/rrf/} and
      * commit to version control.
-     * <strong>Caching:</strong> Auto-discovered prefix files are cached in the local repository with unique IDs
-     * (using {@link RepositoryIdHelper#remoteRepositoryUniqueId(RemoteRepository)}) to prevent conflicts that
-     * could cause build failures.
+     * <strong>Caching:</strong> Auto-discovered prefix files are cached in the local repository.
      *
      * @configurationSource {@link RepositorySystemSession#getConfigProperties()}
      * @configurationType {@link java.lang.Boolean}
@@ -119,6 +116,58 @@ public final class PrefixesRemoteRepositoryFilterSource extends RemoteRepository
 
     public static final boolean DEFAULT_ENABLED = true;
 
+    /**
+     * Configuration to skip the Prefixes filter for given request. This configuration is evaluated and if {@code true}
+     * the prefixes remote filter will not kick in. Main use case is by filter itself, to prevent recursion during
+     * discovery of remote prefixes file, but this also allows other components to control prefix filter discovery, while
+     * leaving configuration like {@link #CONFIG_PROP_ENABLED} still show the "real state".
+     *
+     * @since 2.0.14
+     * @configurationSource {@link RepositorySystemSession#getConfigProperties()}
+     * @configurationType {@link java.lang.Boolean}
+     * @configurationRepoIdSuffix Yes
+     * @configurationDefaultValue {@link #DEFAULT_SKIPPED}
+     */
+    public static final String CONFIG_PROP_SKIPPED =
+            RemoteRepositoryFilterSourceSupport.CONFIG_PROPS_PREFIX + NAME + ".skipped";
+
+    public static final boolean DEFAULT_SKIPPED = false;
+
+    /**
+     * Configuration to allow Prefixes filter to auto-discover prefixes from mirrored repositories as well. For this to
+     * work <em>Maven should be aware</em> that given remote repository is mirror and is usually backed by MRM. Given
+     * multiple MRM implementations messes up prefixes file, is better to just skip these. In other case, one may use
+     * {@link #CONFIG_PROP_ENABLED} with repository ID suffix.
+     *
+     * @since 2.0.14
+     * @configurationSource {@link RepositorySystemSession#getConfigProperties()}
+     * @configurationType {@link java.lang.Boolean}
+     * @configurationRepoIdSuffix Yes
+     * @configurationDefaultValue {@link #DEFAULT_USE_MIRRORED_REPOSITORIES}
+     */
+    public static final String CONFIG_PROP_USE_MIRRORED_REPOSITORIES =
+            RemoteRepositoryFilterSourceSupport.CONFIG_PROPS_PREFIX + NAME + ".useMirroredRepositories";
+
+    public static final boolean DEFAULT_USE_MIRRORED_REPOSITORIES = false;
+
+    /**
+     * Configuration to allow Prefixes filter to auto-discover prefixes from repository managers as well. For this to
+     * work <em>Maven should be aware</em> that given remote repository is backed by repository manager.
+     * Given multiple MRM implementations messes up prefixes file, is better to just skip these. In other case, one may use
+     * {@link #CONFIG_PROP_ENABLED} with repository ID suffix.
+     * <em>Note: as of today, nothing sets this on remote repositories, but is added for future.</em>
+     *
+     * @since 2.0.14
+     * @configurationSource {@link RepositorySystemSession#getConfigProperties()}
+     * @configurationType {@link java.lang.Boolean}
+     * @configurationRepoIdSuffix Yes
+     * @configurationDefaultValue {@link #DEFAULT_USE_REPOSITORY_MANAGERS}
+     */
+    public static final String CONFIG_PROP_USE_REPOSITORY_MANAGERS =
+            RemoteRepositoryFilterSourceSupport.CONFIG_PROPS_PREFIX + NAME + ".useRepositoryManagers";
+
+    public static final boolean DEFAULT_USE_REPOSITORY_MANAGERS = false;
+
     /**
      * The basedir where to store filter files. If path is relative, it is resolved from local repository root.
      *
@@ -146,8 +195,6 @@ public final class PrefixesRemoteRepositoryFilterSource extends RemoteRepository
 
     private final ConcurrentHashMap<RemoteRepository, RepositoryLayout> layouts;
 
-    private final ConcurrentHashMap<RemoteRepository, Boolean> ongoingUpdates;
-
     @Inject
     public PrefixesRemoteRepositoryFilterSource(
             Supplier<MetadataResolver> metadataResolver,
@@ -158,20 +205,26 @@ public PrefixesRemoteRepositoryFilterSource(
         this.repositoryLayoutProvider = requireNonNull(repositoryLayoutProvider);
         this.prefixes = new ConcurrentHashMap<>();
         this.layouts = new ConcurrentHashMap<>();
-        this.ongoingUpdates = new ConcurrentHashMap<>();
     }
 
     @Override
     protected boolean isEnabled(RepositorySystemSession session) {
-        return ConfigUtils.getBoolean(session, DEFAULT_ENABLED, CONFIG_PROP_ENABLED);
+        return ConfigUtils.getBoolean(session, DEFAULT_ENABLED, CONFIG_PROP_ENABLED)
+                && !ConfigUtils.getBoolean(session, DEFAULT_SKIPPED, CONFIG_PROP_SKIPPED);
     }
 
     private boolean isRepositoryFilteringEnabled(RepositorySystemSession session, RemoteRepository remoteRepository) {
         if (isEnabled(session)) {
             return ConfigUtils.getBoolean(
-                    session,
-                    ConfigUtils.getBoolean(session, true, CONFIG_PROP_ENABLED + ".*"),
-                    CONFIG_PROP_ENABLED + "." + remoteRepository.getId());
+                            session,
+                            DEFAULT_ENABLED,
+                            CONFIG_PROP_ENABLED + "." + remoteRepository.getId(),
+                            CONFIG_PROP_ENABLED + ".*")
+                    && !ConfigUtils.getBoolean(
+                            session,
+                            DEFAULT_SKIPPED,
+                            CONFIG_PROP_SKIPPED + "." + remoteRepository.getId(),
+                            CONFIG_PROP_SKIPPED + ".*");
         }
         return false;
     }
@@ -190,7 +243,7 @@ public RemoteRepositoryFilter getRemoteRepositoryFilter(RepositorySystemSession
      * @return the layout instance of {@code null} if layout not supported.
      */
     private RepositoryLayout cacheLayout(RepositorySystemSession session, RemoteRepository remoteRepository) {
-        return layouts.computeIfAbsent(remoteRepository, r -> {
+        return layouts.computeIfAbsent(normalizeRemoteRepository(session, remoteRepository), r -> {
             try {
                 return repositoryLayoutProvider.newRepositoryLayout(session, remoteRepository);
             } catch (NoRepositoryLayoutException e) {
@@ -201,22 +254,9 @@ private RepositoryLayout cacheLayout(RepositorySystemSession session, RemoteRepo
 
     private PrefixTree cachePrefixTree(
             RepositorySystemSession session, Path basedir, RemoteRepository remoteRepository) {
-        return ongoingUpdatesGuard(
-                remoteRepository,
-                () -> prefixes.computeIfAbsent(
-                        remoteRepository, r -> loadPrefixTree(session, basedir, remoteRepository)),
-                () -> PrefixTree.SENTINEL);
-    }
-
-    private <T> T ongoingUpdatesGuard(RemoteRepository remoteRepository, Supplier<T> unblocked, Supplier<T> blocked) {
-        if (!remoteRepository.isBlocked() && null == ongoingUpdates.putIfAbsent(remoteRepository, Boolean.TRUE)) {
-            try {
-                return unblocked.get();
-            } finally {
-                ongoingUpdates.remove(remoteRepository);
-            }
-        }
-        return blocked.get();
+        return prefixes.computeIfAbsent(
+                normalizeRemoteRepository(session, remoteRepository),
+                r -> loadPrefixTree(session, basedir, remoteRepository));
     }
 
     private PrefixTree loadPrefixTree(
@@ -225,8 +265,12 @@ private PrefixTree loadPrefixTree(
             String origin = "user-provided";
             Path filePath = resolvePrefixesFromLocalConfiguration(session, baseDir, remoteRepository);
             if (filePath == null) {
-                origin = "auto-discovered";
-                filePath = resolvePrefixesFromRemoteRepository(session, remoteRepository);
+                if (!supportedResolvePrefixesForRemoteRepository(session, remoteRepository)) {
+                    origin = "unsupported";
+                } else {
+                    origin = "auto-discovered";
+                    filePath = resolvePrefixesFromRemoteRepository(session, remoteRepository);
+                }
             }
             if (filePath != null) {
                 PrefixesSource prefixesSource = PrefixesSource.of(remoteRepository, filePath);
@@ -273,6 +317,18 @@ private Path resolvePrefixesFromLocalConfiguration(
         }
     }
 
+    private boolean supportedResolvePrefixesForRemoteRepository(
+            RepositorySystemSession session, RemoteRepository remoteRepository) {
+        if (remoteRepository.isRepositoryManager()) {
+            return ConfigUtils.getBoolean(
+                    session, DEFAULT_USE_REPOSITORY_MANAGERS, CONFIG_PROP_USE_REPOSITORY_MANAGERS);
+        } else {
+            return remoteRepository.getMirroredRepositories().isEmpty()
+                    || ConfigUtils.getBoolean(
+                            session, DEFAULT_USE_MIRRORED_REPOSITORIES, CONFIG_PROP_USE_MIRRORED_REPOSITORIES);
+        }
+    }
+
     private Path resolvePrefixesFromRemoteRepository(
             RepositorySystemSession session, RemoteRepository remoteRepository) {
         MetadataResolver mr = metadataResolver.get();
@@ -282,35 +338,22 @@ private Path resolvePrefixesFromRemoteRepository(
             RemoteRepository prepared = rm.aggregateRepositories(
                             session, Collections.emptyList(), Collections.singletonList(remoteRepository), true)
                     .get(0);
-            // make it unique
-            RemoteRepository unique = new RemoteRepository.Builder(prepared)
-                    .setId(RepositoryIdHelper.remoteRepositoryUniqueId(remoteRepository))
-                    .build();
-            // supplier for path
-            Supplier<Path> supplier = () -> {
-                MetadataRequest request =
-                        new MetadataRequest(new DefaultMetadata(PREFIX_FILE_PATH, Metadata.Nature.RELEASE_OR_SNAPSHOT));
-                // use unique repository; this will result in prefix (repository metadata) cached under unique id
-                request.setRepository(unique);
-                request.setDeleteLocalCopyIfMissing(true);
-                request.setFavorLocalRepository(true);
-                MetadataResult result = mr.resolveMetadata(
-                                new DefaultRepositorySystemSession(session).setTransferListener(null),
-                                Collections.singleton(request))
-                        .get(0);
-                if (result.isResolved()) {
-                    return result.getMetadata().getPath();
-                } else {
-                    return null;
-                }
-            };
-
-            // prevent recursive calls; but we need extra work if not dealing with Central (as in that case outer call
-            // shields us)
-            if (Objects.equals(prepared.getId(), unique.getId())) {
-                return supplier.get();
+            // retrieve prefix as metadata from repository
+            MetadataRequest request =
+                    new MetadataRequest(new DefaultMetadata(PREFIX_FILE_PATH, Metadata.Nature.RELEASE_OR_SNAPSHOT));
+            request.setRepository(prepared);
+            request.setDeleteLocalCopyIfMissing(true);
+            request.setFavorLocalRepository(true);
+            MetadataResult result = mr.resolveMetadata(
+                            new DefaultRepositorySystemSession(session)
+                                    .setTransferListener(null)
+                                    .setConfigProperty(CONFIG_PROP_SKIPPED, Boolean.TRUE.toString()),
+                            Collections.singleton(request))
+                    .get(0);
+            if (result.isResolved()) {
+                return result.getMetadata().getPath();
             } else {
-                return ongoingUpdatesGuard(unique, supplier, () -> null);
+                return null;
             }
         }
         return null;
@@ -347,15 +390,15 @@ public Result acceptMetadata(RemoteRepository remoteRepository, Metadata metadat
                     repositoryLayout.getLocation(metadata, false).getPath());
         }
 
-        private Result acceptPrefix(RemoteRepository remoteRepository, String path) {
-            PrefixTree prefixTree = cachePrefixTree(session, basedir, remoteRepository);
+        private Result acceptPrefix(RemoteRepository repository, String path) {
+            PrefixTree prefixTree = cachePrefixTree(session, basedir, repository);
             if (PrefixTree.SENTINEL == prefixTree) {
                 return NOT_PRESENT_RESULT;
             }
             if (prefixTree.acceptedPath(path)) {
-                return new SimpleResult(true, "Path " + path + " allowed from " + remoteRepository);
+                return new SimpleResult(true, "Path " + path + " allowed from " + repository);
             } else {
-                return new SimpleResult(false, "Prefix " + path + " NOT allowed from " + remoteRepository);
+                return new SimpleResult(false, "Path " + path + " NOT allowed from " + repository);
             }
         }
     }