Revert "[MNG-7347] SessionScoped beans should be singletons for a given session (#653)" (#715)

This reverts commit b4518b5fe4.

maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java

@@ -90,12 +90,8 @@ public class LifecycleModuleBuilder
 
         // session may be different from rootSession seeded in DefaultMaven
         // explicitly seed the right session here to make sure it is used by Guice
-        final boolean scoped = session != rootSession;
+        sessionScope.enter( reactorContext.getSessionScopeMemento() );
-        if ( scoped )
+        sessionScope.seed( MavenSession.class, session );
-        {
-            sessionScope.enter();
-            sessionScope.seed( MavenSession.class, session );
-        }
         try
         {
 
@@ -149,10 +145,7 @@ public class LifecycleModuleBuilder
         }
         finally
         {
-            if ( scoped )
+            sessionScope.exit();
-            {
-                sessionScope.exit();
-            }
 
             session.setCurrentProject( null );
 

maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java

@@ -107,7 +107,8 @@ public class LifecycleStarter
             ClassLoader oldContextClassLoader = Thread.currentThread().getContextClassLoader();
             ReactorBuildStatus reactorBuildStatus = new ReactorBuildStatus( session.getProjectDependencyGraph() );
             reactorContext =
-                new ReactorContext( result, projectIndex, oldContextClassLoader, reactorBuildStatus );
+                new ReactorContext( result, projectIndex, oldContextClassLoader, reactorBuildStatus,
+                                    sessionScope.memento() );
 
             String builderId = session.getRequest().getBuilderId();
             Builder builder = builders.get( builderId );

maven-core/src/main/java/org/apache/maven/lifecycle/internal/ReactorContext.java

@@ -20,6 +20,7 @@ package org.apache.maven.lifecycle.internal;
  */
 
 import org.apache.maven.execution.MavenExecutionResult;
+import org.apache.maven.session.scope.internal.SessionScope;
 
 /**
  * Context that is fixed for the entire reactor build.
@@ -39,13 +40,17 @@ public class ReactorContext
 
     private final ReactorBuildStatus reactorBuildStatus;
 
+    private final SessionScope.Memento sessionScope;
+
     public ReactorContext( MavenExecutionResult result, ProjectIndex projectIndex,
-                           ClassLoader originalContextClassLoader, ReactorBuildStatus reactorBuildStatus )
+                           ClassLoader originalContextClassLoader, ReactorBuildStatus reactorBuildStatus,
+                           SessionScope.Memento sessionScope )
     {
         this.result = result;
         this.projectIndex = projectIndex;
         this.originalContextClassLoader = originalContextClassLoader;
         this.reactorBuildStatus = reactorBuildStatus;
+        this.sessionScope = sessionScope;
     }
 
     public ReactorBuildStatus getReactorBuildStatus()
@@ -68,4 +73,11 @@ public class ReactorContext
         return originalContextClassLoader;
     }
 
+    /**
+     * @since 3.3.0
+     */
+    public SessionScope.Memento getSessionScopeMemento()
+    {
+        return sessionScope;
+    }
 }

maven-core/src/main/java/org/apache/maven/session/scope/internal/SessionScope.java

@@ -19,16 +19,16 @@ package org.apache.maven.session.scope.internal;
  * under the License.
  */
 
-import java.util.Collection;
+import java.util.Collections;
-import java.util.List;
+import java.util.HashMap;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.LinkedList;
-import java.util.concurrent.ConcurrentMap;
+import java.util.Map;
-import java.util.concurrent.CopyOnWriteArrayList;
 
 import com.google.inject.Key;
 import com.google.inject.OutOfScopeException;
 import com.google.inject.Provider;
 import com.google.inject.Scope;
+import com.google.inject.util.Providers;
 
 /**
  * SessionScope
@@ -36,6 +36,18 @@ import com.google.inject.Scope;
 public class SessionScope
     implements Scope
 {
+    /**
+     * @since 3.3.0
+     */
+    public static class Memento
+    {
+        final Map<Key<?>, Provider<?>> seeded;
+
+        Memento( final Map<Key<?>, Provider<?>> seeded )
+        {
+            this.seeded = Collections.unmodifiableMap( new HashMap<>( seeded ) );
+        }
+    }
 
     private static final Provider<Object> SEEDED_KEY_PROVIDER = new Provider<Object>()
     {
@@ -48,129 +60,112 @@ public class SessionScope
     /**
      * ScopeState
      */
-    protected static final class ScopeState
+    private static final class ScopeState
     {
-        private final ConcurrentMap<Key<?>, CachingProvider<?>> provided = new ConcurrentHashMap<>();
+        private final Map<Key<?>, Provider<?>> seeded = new HashMap<>();
-
-        public <T> void seed( Class<T> clazz, Provider<T> value )
-        {
-            provided.put( Key.get( clazz ), new CachingProvider<>( value ) );
-        }
-
-        @SuppressWarnings( "unchecked" )
-        public <T> Provider<T> scope( Key<T> key, final Provider<T> unscoped )
-        {
-            Provider<?> provider = provided.get( key );
-            if ( provider == null )
-            {
-                CachingProvider<?> newValue = new CachingProvider<>( unscoped );
-                provider = provided.putIfAbsent( key, newValue );
-                if ( provider == null )
-                {
-                    provider = newValue;
-                }
-            }
-            return ( Provider<T> ) provider;
-        }
-
-        public Collection<CachingProvider<?>> providers()
-        {
-            return provided.values();
-        }
 
+        private final Map<Key<?>, Object> provided = new HashMap<>();
     }
 
-    private final List<ScopeState> values = new CopyOnWriteArrayList<>();
+    private final ThreadLocal<LinkedList<ScopeState>> values = new ThreadLocal<>();
 
     public void enter()
     {
-        values.add( 0, new ScopeState() );
+        LinkedList<ScopeState> stack = values.get();
+        if ( stack == null )
+        {
+            stack = new LinkedList<>();
+            values.set( stack );
+        }
+        stack.addFirst( new ScopeState() );
     }
 
-    protected ScopeState getScopeState()
+    /**
+     * @since 3.3.0
+     */
+    public void enter( Memento memento )
     {
-        if ( values.isEmpty() )
+        enter();
+        getScopeState().seeded.putAll( memento.seeded );
+    }
+
+    private ScopeState getScopeState()
+    {
+        LinkedList<ScopeState> stack = values.get();
+        if ( stack == null || stack.isEmpty() )
         {
-            throw new OutOfScopeException( "Cannot access session scope outside of a scoping block" );
+            throw new IllegalStateException();
         }
-        return values.get( 0 );
+        return stack.getFirst();
     }
 
     public void exit()
     {
-        if ( values.isEmpty() )
+        final LinkedList<ScopeState> stack = values.get();
+        if ( stack == null || stack.isEmpty() )
         {
             throw new IllegalStateException();
         }
-        values.remove( 0 );
+        stack.removeFirst();
+        if ( stack.isEmpty() )
+        {
+            values.remove();
+        }
+    }
+
+    /**
+     * @since 3.3.0
+     */
+    public Memento memento()
+    {
+        LinkedList<ScopeState> stack = values.get();
+        return new Memento( stack != null ? stack.getFirst().seeded : Collections.<Key<?>, Provider<?>>emptyMap() );
     }
 
     public <T> void seed( Class<T> clazz, Provider<T> value )
     {
-        getScopeState().seed( clazz, value );
+        getScopeState().seeded.put( Key.get( clazz ), value );
     }
 
     public <T> void seed( Class<T> clazz, final T value )
     {
-        seed( clazz, new Provider<T>()
+        getScopeState().seeded.put( Key.get( clazz ), Providers.of( value ) );
-        {
-            @Override
-            public T get()
-            {
-                return value;
-            }
-        } );
     }
 
     public <T> Provider<T> scope( final Key<T> key, final Provider<T> unscoped )
     {
-        // Lazy evaluating provider
         return new Provider<T>()
         {
-            @Override
+            @SuppressWarnings( "unchecked" )
             public T get()
             {
-                return getScopeState().scope( key, unscoped ).get();
+                LinkedList<ScopeState> stack = values.get();
+                if ( stack == null || stack.isEmpty() )
+                {
+                    throw new OutOfScopeException( "Cannot access " + key + " outside of a scoping block" );
+                }
+
+                ScopeState state = stack.getFirst();
+
+                Provider<?> seeded = state.seeded.get( key );
+
+                if ( seeded != null )
+                {
+                    return (T) seeded.get();
+                }
+
+                T provided = (T) state.provided.get( key );
+                if ( provided == null && unscoped != null )
+                {
+                    provided = unscoped.get();
+                    state.provided.put( key, provided );
+                }
+
+                return provided;
             }
         };
     }
 
-    /**
-     * CachingProvider
-     * @param <T>
-     */
-    protected static class CachingProvider<T> implements Provider<T>
-    {
-        private final Provider<T> provider;
-        private volatile T value;
-
-        CachingProvider( Provider<T> provider )
-        {
-            this.provider = provider;
-        }
-
-        public T value()
-        {
-            return value;
-        }
-
-        @Override
-        public T get()
-        {
-            if ( value == null )
-            {
-                synchronized ( this )
-                {
-                    if ( value == null )
-                    {
-                        value = provider.get();
-                    }
-                }
-            }
-            return value;
-        }
-    }
-
     @SuppressWarnings( { "unchecked" } )
     public static <T> Provider<T> seededKeyProvider()
     {

maven-core/src/test/java/org/apache/maven/session/scope/internal/SessionScopeTest.java

@@ -1,132 +0,0 @@
-package org.apache.maven.session.scope.internal;
-
-/*
- * 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.
- */
-
-import javax.inject.Provider;
-
-import com.google.inject.Key;
-import com.google.inject.OutOfScopeException;
-import org.apache.maven.model.locator.DefaultModelLocator;
-import org.apache.maven.model.locator.ModelLocator;
-import org.apache.maven.plugin.DefaultPluginRealmCache;
-import org.apache.maven.plugin.PluginRealmCache;
-import org.junit.Test;
-
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.fail;
-
-public class SessionScopeTest {
-
-    @Test
-    public void testScope() throws Exception
-    {
-        SessionScope scope = new SessionScope();
-
-        try
-        {
-            scope.seed( ModelLocator.class, new DefaultModelLocator() );
-            fail( "Expected a " + OutOfScopeException.class.getName() + " exception to be thrown" );
-        }
-        catch ( OutOfScopeException e )
-        {
-            // expected
-        }
-
-        Provider<ModelLocator> pml = scope.scope( Key.get( ModelLocator.class), new DefaultModelLocatorProvider() );
-        assertNotNull( pml );
-        try
-        {
-            pml.get();
-            fail( "Expected a " + OutOfScopeException.class.getName() + " exception to be thrown" );
-        }
-        catch ( OutOfScopeException e )
-        {
-            // expected
-        }
-
-        Provider<PluginRealmCache> pmst = scope.scope( Key.get( PluginRealmCache.class ), new DefaultPluginRealmCacheProvider() );
-        assertNotNull( pmst );
-
-        scope.enter();
-
-        final DefaultModelLocator dml1 = new DefaultModelLocator();
-        scope.seed( ModelLocator.class, dml1 );
-
-        assertSame( dml1, pml.get() );
-
-        PluginRealmCache mst1 = pmst.get();
-        assertSame( mst1, pmst.get() );
-        Provider<PluginRealmCache> pmst1 = scope.scope( Key.get( PluginRealmCache.class ), new DefaultPluginRealmCacheProvider() );
-        assertNotNull( pmst1 );
-        assertSame( mst1, pmst1.get() );
-
-        scope.enter();
-
-        pmst1 = scope.scope( Key.get( PluginRealmCache.class ), new DefaultPluginRealmCacheProvider() );
-        assertNotNull( pmst1 );
-        assertNotSame( mst1, pmst1.get() );
-
-        scope.exit();
-
-        assertSame( mst1, pmst.get() );
-
-        scope.exit();
-
-        try
-        {
-            pmst.get();
-            fail( "Expected a " + OutOfScopeException.class.getName() + " exception to be thrown" );
-        }
-        catch ( OutOfScopeException e )
-        {
-            // expected
-        }
-        try
-        {
-            scope.seed( ModelLocator.class, new DefaultModelLocator() );
-            fail( "Expected a " + OutOfScopeException.class.getName() + " exception to be thrown" );
-        }
-        catch ( OutOfScopeException e )
-        {
-            // expected
-        }
-    }
-
-    private static class DefaultPluginRealmCacheProvider implements com.google.inject.Provider<PluginRealmCache>
-    {
-        @Override
-        public PluginRealmCache get()
-        {
-            return new DefaultPluginRealmCache();
-        }
-    }
-
-    private static class DefaultModelLocatorProvider implements com.google.inject.Provider<ModelLocator>
-    {
-        @Override
-        public ModelLocator get()
-        {
-            return new DefaultModelLocator();
-        }
-    }
-
-}