You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2022/04/12 15:03:27 UTC

[maven] branch revert-653-MNG-7347-3.8.x created (now b791bad43)

This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a change to branch revert-653-MNG-7347-3.8.x
in repository https://gitbox.apache.org/repos/asf/maven.git


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

This branch includes the following new commits:

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

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



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

Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a commit to branch revert-653-MNG-7347-3.8.x
in repository https://gitbox.apache.org/repos/asf/maven.git

commit b791bad435b471c3ac759440fa0442388396b5d1
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Tue Apr 12 17:03:23 2022 +0200

    Revert "[MNG-7347] SessionScoped beans should be singletons for a given session (#653)"
    
    This reverts commit b4518b5fe416a552a59e5201b4569a9bc0af3153.
---
 .../lifecycle/internal/LifecycleModuleBuilder.java |  13 +-
 .../maven/lifecycle/internal/LifecycleStarter.java |   3 +-
 .../maven/lifecycle/internal/ReactorContext.java   |  14 +-
 .../maven/session/scope/internal/SessionScope.java | 159 ++++++++++-----------
 .../session/scope/internal/SessionScopeTest.java   | 132 -----------------
 5 files changed, 95 insertions(+), 226 deletions(-)

diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java
index 1cbaf5334..548fe6c8f 100644
--- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleModuleBuilder.java
+++ b/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;
-        if ( scoped )
-        {
-            sessionScope.enter();
-            sessionScope.seed( MavenSession.class, session );
-        }
+        sessionScope.enter( reactorContext.getSessionScopeMemento() );
+        sessionScope.seed( MavenSession.class, session );
         try
         {
 
@@ -149,10 +145,7 @@ public class LifecycleModuleBuilder
         }
         finally
         {
-            if ( scoped )
-            {
-                sessionScope.exit();
-            }
+            sessionScope.exit();
 
             session.setCurrentProject( null );
 
diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java
index 834498126..cee807392 100644
--- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java
+++ b/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 );
diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ReactorContext.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ReactorContext.java
index 076c6229f..7df531404 100644
--- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/ReactorContext.java
+++ b/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;
+    }
 }
diff --git a/maven-core/src/main/java/org/apache/maven/session/scope/internal/SessionScope.java b/maven-core/src/main/java/org/apache/maven/session/scope/internal/SessionScope.java
index 41187fd80..ac423bc6c 100644
--- a/maven-core/src/main/java/org/apache/maven/session/scope/internal/SessionScope.java
+++ b/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.List;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.Map;
 
 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,127 +60,110 @@ 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 ) );
-        }
+        private final Map<Key<?>, Object> provided = new HashMap<>();
+    }
 
-        @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;
-        }
+    private final ThreadLocal<LinkedList<ScopeState>> values = new ThreadLocal<>();
 
-        public Collection<CachingProvider<?>> providers()
+    public void enter()
+    {
+        LinkedList<ScopeState> stack = values.get();
+        if ( stack == null )
         {
-            return provided.values();
+            stack = new LinkedList<>();
+            values.set( stack );
         }
-
+        stack.addFirst( new ScopeState() );
     }
 
-    private final List<ScopeState> values = new CopyOnWriteArrayList<>();
-
-    public void enter()
+    /**
+     * @since 3.3.0
+     */
+    public void enter( Memento memento )
     {
-        values.add( 0, new ScopeState() );
+        enter();
+        getScopeState().seeded.putAll( memento.seeded );
     }
 
-    protected ScopeState getScopeState()
+    private ScopeState getScopeState()
     {
-        if ( values.isEmpty() )
+        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>()
-        {
-            @Override
-            public T get()
-            {
-                return value;
-            }
-        } );
+        getScopeState().seeded.put( Key.get( clazz ), Providers.of( 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" );
+                }
 
-    /**
-     * CachingProvider
-     * @param <T>
-     */
-    protected static class CachingProvider<T> implements Provider<T>
-    {
-        private final Provider<T> provider;
-        private volatile T value;
+                ScopeState state = stack.getFirst();
 
-        CachingProvider( Provider<T> provider )
-        {
-            this.provider = provider;
-        }
+                Provider<?> seeded = state.seeded.get( key );
 
-        public T value()
-        {
-            return value;
-        }
+                if ( seeded != null )
+                {
+                    return (T) seeded.get();
+                }
 
-        @Override
-        public T get()
-        {
-            if ( value == null )
-            {
-                synchronized ( this )
+                T provided = (T) state.provided.get( key );
+                if ( provided == null && unscoped != null )
                 {
-                    if ( value == null )
-                    {
-                        value = provider.get();
-                    }
+                    provided = unscoped.get();
+                    state.provided.put( key, provided );
                 }
+
+                return provided;
             }
-            return value;
-        }
+        };
     }
 
     @SuppressWarnings( { "unchecked" } )
diff --git a/maven-core/src/test/java/org/apache/maven/session/scope/internal/SessionScopeTest.java b/maven-core/src/test/java/org/apache/maven/session/scope/internal/SessionScopeTest.java
deleted file mode 100644
index 099e4dddb..000000000
--- a/maven-core/src/test/java/org/apache/maven/session/scope/internal/SessionScopeTest.java
+++ /dev/null
@@ -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();
-        }
-    }
-
-}