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/01/10 07:19:59 UTC
[maven] branch maven-3.8.x updated: [MNG-7347] SessionScoped beans should be singletons for a given session (#653)
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch maven-3.8.x
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/maven-3.8.x by this push:
new b4518b5 [MNG-7347] SessionScoped beans should be singletons for a given session (#653)
b4518b5 is described below
commit b4518b5fe416a552a59e5201b4569a9bc0af3153
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Mon Jan 10 08:19:41 2022 +0100
[MNG-7347] SessionScoped beans should be singletons for a given session (#653)
---
.../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, 226 insertions(+), 95 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 548fe6c..1cbaf53 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,8 +90,12 @@ 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
- sessionScope.enter( reactorContext.getSessionScopeMemento() );
- sessionScope.seed( MavenSession.class, session );
+ final boolean scoped = session != rootSession;
+ if ( scoped )
+ {
+ sessionScope.enter();
+ sessionScope.seed( MavenSession.class, session );
+ }
try
{
@@ -145,7 +149,10 @@ public class LifecycleModuleBuilder
}
finally
{
- sessionScope.exit();
+ if ( scoped )
+ {
+ 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 cee8073..8344981 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,8 +107,7 @@ public class LifecycleStarter
ClassLoader oldContextClassLoader = Thread.currentThread().getContextClassLoader();
ReactorBuildStatus reactorBuildStatus = new ReactorBuildStatus( session.getProjectDependencyGraph() );
reactorContext =
- new ReactorContext( result, projectIndex, oldContextClassLoader, reactorBuildStatus,
- sessionScope.memento() );
+ new ReactorContext( result, projectIndex, oldContextClassLoader, reactorBuildStatus );
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 7df5314..076c622 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,7 +20,6 @@ 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.
@@ -40,17 +39,13 @@ public class ReactorContext
private final ReactorBuildStatus reactorBuildStatus;
- private final SessionScope.Memento sessionScope;
-
public ReactorContext( MavenExecutionResult result, ProjectIndex projectIndex,
- ClassLoader originalContextClassLoader, ReactorBuildStatus reactorBuildStatus,
- SessionScope.Memento sessionScope )
+ ClassLoader originalContextClassLoader, ReactorBuildStatus reactorBuildStatus )
{
this.result = result;
this.projectIndex = projectIndex;
this.originalContextClassLoader = originalContextClassLoader;
this.reactorBuildStatus = reactorBuildStatus;
- this.sessionScope = sessionScope;
}
public ReactorBuildStatus getReactorBuildStatus()
@@ -73,11 +68,4 @@ 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 ac423bc..41187fd 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.Collections;
-import java.util.HashMap;
-import java.util.LinkedList;
-import java.util.Map;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+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,18 +36,6 @@ import com.google.inject.util.Providers;
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>()
{
@@ -60,110 +48,127 @@ public class SessionScope
/**
* ScopeState
*/
- private static final class ScopeState
+ protected static final class ScopeState
{
- private final Map<Key<?>, Provider<?>> seeded = new HashMap<>();
+ private final ConcurrentMap<Key<?>, CachingProvider<?>> provided = new ConcurrentHashMap<>();
- private final Map<Key<?>, Object> provided = new HashMap<>();
- }
+ public <T> void seed( Class<T> clazz, Provider<T> value )
+ {
+ provided.put( Key.get( clazz ), new CachingProvider<>( value ) );
+ }
- private final ThreadLocal<LinkedList<ScopeState>> values = new ThreadLocal<>();
+ @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 void enter()
- {
- LinkedList<ScopeState> stack = values.get();
- if ( stack == null )
+ public Collection<CachingProvider<?>> providers()
{
- stack = new LinkedList<>();
- values.set( stack );
+ return provided.values();
}
- stack.addFirst( new ScopeState() );
+
}
- /**
- * @since 3.3.0
- */
- public void enter( Memento memento )
+ private final List<ScopeState> values = new CopyOnWriteArrayList<>();
+
+ public void enter()
{
- enter();
- getScopeState().seeded.putAll( memento.seeded );
+ values.add( 0, new ScopeState() );
}
- private ScopeState getScopeState()
+ protected ScopeState getScopeState()
{
- LinkedList<ScopeState> stack = values.get();
- if ( stack == null || stack.isEmpty() )
+ if ( values.isEmpty() )
{
- throw new IllegalStateException();
+ throw new OutOfScopeException( "Cannot access session scope outside of a scoping block" );
}
- return stack.getFirst();
+ return values.get( 0 );
}
public void exit()
{
- final LinkedList<ScopeState> stack = values.get();
- if ( stack == null || stack.isEmpty() )
+ if ( values.isEmpty() )
{
throw new IllegalStateException();
}
- 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() );
+ values.remove( 0 );
}
public <T> void seed( Class<T> clazz, Provider<T> value )
{
- getScopeState().seeded.put( Key.get( clazz ), value );
+ getScopeState().seed( clazz, value );
}
public <T> void seed( Class<T> clazz, final T value )
{
- getScopeState().seeded.put( Key.get( clazz ), Providers.of( value ) );
+ seed( clazz, new Provider<T>()
+ {
+ @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>()
{
- @SuppressWarnings( "unchecked" )
+ @Override
public T get()
{
- LinkedList<ScopeState> stack = values.get();
- if ( stack == null || stack.isEmpty() )
- {
- throw new OutOfScopeException( "Cannot access " + key + " outside of a scoping block" );
- }
+ return getScopeState().scope( key, unscoped ).get();
+ }
+ };
+ }
- ScopeState state = stack.getFirst();
+ /**
+ * CachingProvider
+ * @param <T>
+ */
+ protected static class CachingProvider<T> implements Provider<T>
+ {
+ private final Provider<T> provider;
+ private volatile T value;
- Provider<?> seeded = state.seeded.get( key );
+ CachingProvider( Provider<T> provider )
+ {
+ this.provider = provider;
+ }
- if ( seeded != null )
- {
- return (T) seeded.get();
- }
+ public T value()
+ {
+ return value;
+ }
- T provided = (T) state.provided.get( key );
- if ( provided == null && unscoped != null )
+ @Override
+ public T get()
+ {
+ if ( value == null )
+ {
+ synchronized ( this )
{
- provided = unscoped.get();
- state.provided.put( key, provided );
+ if ( value == null )
+ {
+ value = provider.get();
+ }
}
-
- 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
new file mode 100644
index 0000000..099e4dd
--- /dev/null
+++ b/maven-core/src/test/java/org/apache/maven/session/scope/internal/SessionScopeTest.java
@@ -0,0 +1,132 @@
+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();
+ }
+ }
+
+}