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/05/09 08:16:56 UTC
[maven] branch master updated: [MNG-7459] Revert "[MNG-7347] SessionScoped beans should be singletons for a given session (#621)"
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new 1c22f9452 [MNG-7459] Revert "[MNG-7347] SessionScoped beans should be singletons for a given session (#621)"
1c22f9452 is described below
commit 1c22f94522ec6686ded044a4e0ca0b3cb9d76e62
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Mon May 9 10:11:54 2022 +0200
[MNG-7459] Revert "[MNG-7347] SessionScoped beans should be singletons for a given session (#621)"
This reverts commit faf5d5d274fb4a3348cc330b604d530d0ebb60ce.
---
.../lifecycle/internal/LifecycleModuleBuilder.java | 13 +-
.../maven/lifecycle/internal/LifecycleStarter.java | 3 +-
.../maven/lifecycle/internal/ReactorContext.java | 14 ++-
.../maven/session/scope/internal/SessionScope.java | 137 ++++++++++++---------
.../session/scope/internal/SessionScopeTest.java | 81 ------------
5 files changed, 96 insertions(+), 152 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 405ab5fc5..a2f90bee1 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
@@ -89,12 +89,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
{
@@ -148,10 +144,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 cf023c055..260dd2554 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
@@ -122,7 +122,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 8d30c10ef..de97d4479 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,23 +19,35 @@ package org.apache.maven.session.scope.internal;
* under the License.
*/
-import java.util.Collection;
-import java.util.List;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-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
*/
public class SessionScope
- implements Scope
+ 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 = () ->
{
@@ -45,99 +57,106 @@ public class SessionScope
/**
* ScopeState
*/
- protected static final class ScopeState
+ private static final class ScopeState
{
- private final Map<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, Provider<T> unscoped )
- {
- Provider<?> provider = provided.computeIfAbsent( key, k -> new CachingProvider<>( unscoped ) );
- 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.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, ( Provider<T> ) () -> 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 () -> getScopeState().scope( key, unscoped ).get();
- }
+ return () ->
+ {
+ LinkedList<ScopeState> stack = values.get();
+ if ( stack == null || stack.isEmpty() )
+ {
+ throw new OutOfScopeException( "Cannot access " + key + " outside of a scoping block" );
+ }
- 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 )
+ T provided = (T) state.provided.get( key );
+ if ( provided == null && unscoped != null )
{
- synchronized ( this )
- {
- if ( value == null )
- {
- value = provider.get();
- }
- }
+ provided = unscoped.get();
+ state.provided.put( key, provided );
}
- return value;
- }
+
+ return provided;
+ };
}
@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 30874f437..000000000
--- a/maven-core/src/test/java/org/apache/maven/session/scope/internal/SessionScopeTest.java
+++ /dev/null
@@ -1,81 +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.building.DefaultModelSourceTransformer;
-import org.apache.maven.model.building.ModelSourceTransformer;
-import org.apache.maven.model.locator.DefaultModelLocator;
-import org.apache.maven.model.locator.ModelLocator;
-import org.junit.jupiter.api.Test;
-
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertNotSame;
-import static org.junit.jupiter.api.Assertions.assertSame;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-
-public class SessionScopeTest {
-
- @Test
- public void testScope() throws Exception
- {
- SessionScope scope = new SessionScope();
-
- assertThrows( OutOfScopeException.class, () -> scope.seed( ModelLocator.class, new DefaultModelLocator() ) );
-
- Provider<ModelLocator> pml = scope.scope( Key.get( ModelLocator.class), DefaultModelLocator::new );
- assertNotNull( pml );
- assertThrows( OutOfScopeException.class, pml::get );
-
- Provider<ModelSourceTransformer> pmst = scope.scope( Key.get( ModelSourceTransformer.class ), DefaultModelSourceTransformer::new );
- assertNotNull( pmst );
-
- scope.enter();
-
- final DefaultModelLocator dml1 = new DefaultModelLocator();
- scope.seed( ModelLocator.class, dml1 );
-
- assertSame( dml1, pml.get() );
-
- ModelSourceTransformer mst1 = pmst.get();
- assertSame( mst1, pmst.get() );
- Provider<ModelSourceTransformer> pmst1 = scope.scope( Key.get( ModelSourceTransformer.class ), DefaultModelSourceTransformer::new );
- assertNotNull( pmst1 );
- assertSame( mst1, pmst1.get() );
-
- scope.enter();
-
- pmst1 = scope.scope( Key.get( ModelSourceTransformer.class ), DefaultModelSourceTransformer::new );
- assertNotNull( pmst1 );
- assertNotSame( mst1, pmst1.get() );
-
- scope.exit();
-
- assertSame( mst1, pmst.get() );
-
- scope.exit();
-
- assertThrows( OutOfScopeException.class, pmst::get );
- assertThrows( OutOfScopeException.class, () -> scope.seed( ModelLocator.class, new DefaultModelLocator() ) );
- }
-}