[MNG-8005] Fix workspace reader drop bug (#1385)

Maven4 seems sets but then drops/overrides some workspace readers. Partially due hazy borders who or what manages them, as role is split between session factory and session factory caller.

Changes:
* session factory does NOTHING re workspace readers, it becomes fully the caller duty
* two spots calling session factory (default maven, extension bootstrap) are now fully in charge to properly setup workspace readers

Co-authored-by: Jonas Rutishauser <jonas.rutishauser@alumni.ethz.ch>

---

https://issues.apache.org/jira/browse/MNG-8005

maven-core/src/main/java/org/apache/maven/DefaultMaven.java

@@ -75,6 +75,7 @@
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.RepositorySystemSession.CloseableSession;
 import org.eclipse.aether.repository.WorkspaceReader;
+import org.eclipse.sisu.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.helpers.MessageFormatter;
@@ -110,6 +111,8 @@ public class DefaultMaven implements Maven {
 
     private final DefaultSessionFactory defaultSessionFactory;
 
+    private final WorkspaceReader ideWorkspaceReader;
+
     private final ProjectSelector projectSelector;
 
     @Inject
@@ -125,7 +128,8 @@ public DefaultMaven(
             BuildResumptionAnalyzer buildResumptionAnalyzer,
             BuildResumptionDataRepository buildResumptionDataRepository,
             SuperPomProvider superPomProvider,
-            DefaultSessionFactory defaultSessionFactory) {
+            DefaultSessionFactory defaultSessionFactory,
+            @Nullable @Named("ide") WorkspaceReader ideWorkspaceReader) {
         this.lookup = lookup;
         this.lifecycleStarter = lifecycleStarter;
         this.eventCatapult = eventCatapult;
@@ -136,6 +140,7 @@ public DefaultMaven(
         this.buildResumptionAnalyzer = buildResumptionAnalyzer;
         this.buildResumptionDataRepository = buildResumptionDataRepository;
         this.superPomProvider = superPomProvider;
+        this.ideWorkspaceReader = ideWorkspaceReader;
         this.defaultSessionFactory = defaultSessionFactory;
         this.projectSelector = new ProjectSelector(); // if necessary switch to DI
     }
@@ -209,7 +214,8 @@ private MavenExecutionResult doExecute(MavenExecutionRequest request) {
         // so that @SessionScoped components can be @Injected into AbstractLifecycleParticipants.
         //
         sessionScope.enter();
-        MavenChainedWorkspaceReader chainedWorkspaceReader = new MavenChainedWorkspaceReader();
+        MavenChainedWorkspaceReader chainedWorkspaceReader =
+                new MavenChainedWorkspaceReader(request.getWorkspaceReader(), ideWorkspaceReader);
         try (CloseableSession closeableSession = newCloseableSession(request, chainedWorkspaceReader)) {
             MavenSession session = new MavenSession(closeableSession, request, result);
             session.setSession(defaultSessionFactory.getSession(session));
@@ -238,8 +244,7 @@ private MavenExecutionResult doExecute(
         }
 
         try {
-            WorkspaceReader reactorReader = lookup.lookup(WorkspaceReader.class, ReactorReader.HINT);
-            chainedWorkspaceReader.setReaders(Collections.singletonList(reactorReader));
+            chainedWorkspaceReader.addReader(lookup.lookup(WorkspaceReader.class, ReactorReader.HINT));
         } catch (LookupException e) {
             return addExceptionToResult(result, e);
         }
@@ -336,7 +341,7 @@ private void setupWorkspaceReader(MavenSession session, MavenChainedWorkspaceRea
         // 1) Reactor workspace reader
         WorkspaceReader reactorReader = lookup.lookup(WorkspaceReader.class, ReactorReader.HINT);
         workspaceReaders.add(reactorReader);
-        // 2) Repository system session-scoped workspace reader
+        // 2) Repository system session-scoped workspace reader (contains ide and exec request reader)
         for (WorkspaceReader repoWorkspaceReader : chainedWorkspaceReader.getReaders()) {
             if (repoWorkspaceReader != null && repoWorkspaceReader != reactorReader) {
                 workspaceReaders.add(repoWorkspaceReader);

maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java

@@ -68,7 +68,6 @@
 import org.eclipse.aether.repository.ProxySelector;
 import org.eclipse.aether.repository.RemoteRepository;
 import org.eclipse.aether.repository.RepositoryPolicy;
-import org.eclipse.aether.repository.WorkspaceReader;
 import org.eclipse.aether.resolution.ResolutionErrorPolicy;
 import org.eclipse.aether.util.graph.manager.ClassicDependencyManager;
 import org.eclipse.aether.util.graph.version.*;
@@ -84,7 +83,6 @@
 import org.eclipse.aether.version.Version;
 import org.eclipse.aether.version.VersionRange;
 import org.eclipse.aether.version.VersionScheme;
-import org.eclipse.sisu.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -180,8 +178,6 @@ public class DefaultRepositorySystemSessionFactory {
 
     private final RepositorySystem repoSystem;
 
-    private final WorkspaceReader workspaceRepository;
-
     private final SettingsDecrypter settingsDecrypter;
 
     private final EventSpyDispatcher eventSpyDispatcher;
@@ -197,15 +193,13 @@ public class DefaultRepositorySystemSessionFactory {
     public DefaultRepositorySystemSessionFactory(
             ArtifactHandlerManager artifactHandlerManager,
             RepositorySystem repoSystem,
-            @Nullable @Named("ide") WorkspaceReader workspaceRepository,
             SettingsDecrypter settingsDecrypter,
             EventSpyDispatcher eventSpyDispatcher,
             RuntimeInformation runtimeInformation,
             TypeRegistry typeRegistry,
             VersionScheme versionScheme) {
         this.artifactHandlerManager = artifactHandlerManager;
         this.repoSystem = repoSystem;
-        this.workspaceRepository = workspaceRepository;
         this.settingsDecrypter = settingsDecrypter;
         this.eventSpyDispatcher = eventSpyDispatcher;
         this.runtimeInformation = runtimeInformation;
@@ -261,9 +255,6 @@ public SessionBuilder newRepositorySessionBuilder(MavenExecutionRequest request)
 
         session.setArtifactTypeRegistry(RepositoryUtils.newArtifactTypeRegistry(artifactHandlerManager));
 
-        session.setWorkspaceReader(
-                request.getWorkspaceReader() != null ? request.getWorkspaceReader() : workspaceRepository);
-
         DefaultSettingsDecryptionRequest decrypt = new DefaultSettingsDecryptionRequest();
         decrypt.setProxies(request.getProxies());
         decrypt.setServers(request.getServers());

maven-core/src/main/java/org/apache/maven/internal/aether/MavenChainedWorkspaceReader.java

@@ -19,12 +19,7 @@
 package org.apache.maven.internal.aether;
 
 import java.io.File;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.LinkedHashSet;
-import java.util.List;
+import java.util.*;
 import java.util.stream.Collectors;
 
 import org.apache.maven.model.Model;
@@ -37,6 +32,9 @@
 
 /**
  * A maven workspace reader that delegates to a chain of other readers, effectively aggregating their contents.
+ * <p>
+ * This class, while technically is not immutable, should be considered as such once set up. If not mutated, it is also
+ * thread-safe. <em>The mutation of this class instances should happen beforehand their use in session</em>.
  */
 public class MavenChainedWorkspaceReader implements MavenWorkspaceReader {
 
@@ -102,7 +100,10 @@ public List<String> findVersions(Artifact artifact) {
     }
 
     public void setReaders(Collection<WorkspaceReader> readers) {
-        this.readers = Collections.unmodifiableList(new ArrayList<>(readers));
+        requireNonNull(readers, "readers");
+        // skip possible null entries
+        this.readers = Collections.unmodifiableList(
+                new ArrayList<>(readers.stream().filter(Objects::nonNull).collect(Collectors.toList())));
         Key key = new Key(this.readers);
         this.repository = new WorkspaceRepository(key.getContentType(), key);
     }
@@ -111,6 +112,13 @@ public List<WorkspaceReader> getReaders() {
         return readers;
     }
 
+    public void addReader(WorkspaceReader workspaceReader) {
+        requireNonNull(workspaceReader, "workspaceReader");
+        ArrayList<WorkspaceReader> newReaders = new ArrayList<>(this.readers);
+        newReaders.add(workspaceReader);
+        setReaders(newReaders);
+    }
+
     private static class Key {
         private final List<Object> keys;
         private final String type;

maven-core/src/test/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactoryTest.java

@@ -84,7 +84,6 @@ void isNoSnapshotUpdatesTest() throws InvalidRepositoryException {
         DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory(
                 artifactHandlerManager,
                 aetherRepositorySystem,
-                null,
                 settingsDecrypter,
                 eventSpyDispatcher,
                 information,
@@ -108,7 +107,6 @@ void isSnapshotUpdatesTest() throws InvalidRepositoryException {
         DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory(
                 artifactHandlerManager,
                 aetherRepositorySystem,
-                null,
                 settingsDecrypter,
                 eventSpyDispatcher,
                 information,
@@ -144,7 +142,6 @@ void wagonProviderConfigurationTest() throws InvalidRepositoryException {
         DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory(
                 artifactHandlerManager,
                 aetherRepositorySystem,
-                null,
                 settingsDecrypter,
                 eventSpyDispatcher,
                 information,
@@ -188,7 +185,6 @@ void httpConfigurationWithHttpHeadersTest() throws InvalidRepositoryException {
         DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory(
                 artifactHandlerManager,
                 aetherRepositorySystem,
-                null,
                 settingsDecrypter,
                 eventSpyDispatcher,
                 information,
@@ -226,7 +222,6 @@ void connectTimeoutConfigurationTest() throws InvalidRepositoryException {
         DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory(
                 artifactHandlerManager,
                 aetherRepositorySystem,
-                null,
                 settingsDecrypter,
                 eventSpyDispatcher,
                 information,
@@ -268,7 +263,6 @@ void connectionTimeoutFromHttpConfigurationTest() throws InvalidRepositoryExcept
         DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory(
                 artifactHandlerManager,
                 aetherRepositorySystem,
-                null,
                 settingsDecrypter,
                 eventSpyDispatcher,
                 information,
@@ -304,7 +298,6 @@ void requestTimeoutConfigurationTest() throws InvalidRepositoryException {
         DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory(
                 artifactHandlerManager,
                 aetherRepositorySystem,
-                null,
                 settingsDecrypter,
                 eventSpyDispatcher,
                 information,
@@ -346,7 +339,6 @@ void readTimeoutFromHttpConfigurationTest() throws InvalidRepositoryException {
         DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory(
                 artifactHandlerManager,
                 aetherRepositorySystem,
-                null,
                 settingsDecrypter,
                 eventSpyDispatcher,
                 information,
@@ -365,7 +357,6 @@ void transportConfigurationTest() throws InvalidRepositoryException {
         DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory(
                 artifactHandlerManager,
                 aetherRepositorySystem,
-                null,
                 settingsDecrypter,
                 eventSpyDispatcher,
                 information,
@@ -412,7 +403,6 @@ void versionFilteringTest() throws InvalidRepositoryException {
         DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory(
                 artifactHandlerManager,
                 aetherRepositorySystem,
-                null,
                 settingsDecrypter,
                 eventSpyDispatcher,
                 information,

maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java

@@ -35,6 +35,7 @@
 import org.apache.maven.extension.internal.CoreExports;
 import org.apache.maven.extension.internal.CoreExtensionEntry;
 import org.apache.maven.internal.aether.DefaultRepositorySystemSessionFactory;
+import org.apache.maven.internal.aether.MavenChainedWorkspaceReader;
 import org.apache.maven.plugin.PluginResolutionException;
 import org.apache.maven.plugin.internal.DefaultPluginDependenciesResolver;
 import org.codehaus.plexus.DefaultPlexusContainer;
@@ -50,9 +51,11 @@
 import org.eclipse.aether.artifact.Artifact;
 import org.eclipse.aether.graph.DependencyFilter;
 import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.repository.WorkspaceReader;
 import org.eclipse.aether.resolution.ArtifactResult;
 import org.eclipse.aether.resolution.DependencyResult;
 import org.eclipse.aether.util.filter.ExclusionsDependencyFilter;
+import org.eclipse.sisu.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -77,24 +80,29 @@ public class BootstrapCoreExtensionManager {
 
     private final ClassRealm parentRealm;
 
+    private final WorkspaceReader ideWorkspaceReader;
+
     @Inject
     public BootstrapCoreExtensionManager(
             DefaultPluginDependenciesResolver pluginDependenciesResolver,
             DefaultRepositorySystemSessionFactory repositorySystemSessionFactory,
             CoreExports coreExports,
-            PlexusContainer container) {
+            PlexusContainer container,
+            @Nullable @Named("ide") WorkspaceReader ideWorkspaceReader) {
         this.pluginDependenciesResolver = pluginDependenciesResolver;
         this.repositorySystemSessionFactory = repositorySystemSessionFactory;
         this.coreExports = coreExports;
         this.classWorld = ((DefaultPlexusContainer) container).getClassWorld();
         this.parentRealm = container.getContainerRealm();
+        this.ideWorkspaceReader = ideWorkspaceReader;
     }
 
     public List<CoreExtensionEntry> loadCoreExtensions(
             MavenExecutionRequest request, Set<String> providedArtifacts, List<CoreExtension> extensions)
             throws Exception {
         try (CloseableSession repoSession = repositorySystemSessionFactory
                 .newRepositorySessionBuilder(request)
+                .setWorkspaceReader(new MavenChainedWorkspaceReader(request.getWorkspaceReader(), ideWorkspaceReader))
                 .build()) {
             List<RemoteRepository> repositories = RepositoryUtils.toRepos(request.getPluginArtifactRepositories());
             Interpolator interpolator = createInterpolator(request);