From b9145e425e44df9be0622e4737e3642a02991fc0 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 24 Jun 2022 12:00:45 +0200 Subject: [PATCH 1/2] [MRESOLVER-266] Simplify named lock adapter creation and align configuration Simplified adapter creation and now as everything else in resolver, session config properties are used as configuration source, not Java system properties. --- maven-resolver-impl/pom.xml | 6 +- .../aether/impl/DefaultServiceLocator.java | 3 - .../aether/impl/guice/AetherModule.java | 3 - .../DefaultSyncContextFactory.java | 114 +++++++++++++++--- .../named/NamedLockFactorySelector.java | 4 + .../NamedLockFactorySelectorSupport.java | 3 + .../named/SimpleNamedLockFactorySelector.java | 3 + 7 files changed, 110 insertions(+), 26 deletions(-) diff --git a/maven-resolver-impl/pom.xml b/maven-resolver-impl/pom.xml index ad6ecdea7..4aae04a31 100644 --- a/maven-resolver-impl/pom.xml +++ b/maven-resolver-impl/pom.xml @@ -69,9 +69,9 @@ true - javax.annotation - javax.annotation-api - 1.3.2 + javax.annotation + javax.annotation-api + 1.3.2 org.eclipse.sisu diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/DefaultServiceLocator.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/DefaultServiceLocator.java index 815c07cd7..75841aec4 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/DefaultServiceLocator.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/DefaultServiceLocator.java @@ -58,8 +58,6 @@ import org.eclipse.aether.internal.impl.SimpleLocalRepositoryManagerFactory; import org.eclipse.aether.internal.impl.slf4j.Slf4jLoggerFactory; import org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory; -import org.eclipse.aether.internal.impl.synccontext.named.NamedLockFactorySelector; -import org.eclipse.aether.internal.impl.synccontext.named.SimpleNamedLockFactorySelector; import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactorySelector; import org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider; import org.eclipse.aether.spi.connector.layout.RepositoryLayoutFactory; @@ -227,7 +225,6 @@ public DefaultServiceLocator() addService( LocalRepositoryManagerFactory.class, EnhancedLocalRepositoryManagerFactory.class ); addService( LoggerFactory.class, Slf4jLoggerFactory.class ); addService( TrackingFileManager.class, DefaultTrackingFileManager.class ); - addService( NamedLockFactorySelector.class, SimpleNamedLockFactorySelector.class ); addService( ChecksumAlgorithmFactorySelector.class, DefaultChecksumAlgorithmFactorySelector.class ); addService( LocalPathComposer.class, DefaultLocalPathComposer.class ); } diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java index 02bd7b08b..ce7ccc28d 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java @@ -56,8 +56,6 @@ import org.eclipse.aether.internal.impl.collect.bf.BfDependencyCollector; import org.eclipse.aether.internal.impl.collect.df.DfDependencyCollector; import org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory; -import org.eclipse.aether.internal.impl.synccontext.named.NamedLockFactorySelector; -import org.eclipse.aether.internal.impl.synccontext.named.SimpleNamedLockFactorySelector; import org.eclipse.aether.internal.impl.synccontext.named.GAVNameMapper; import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper; import org.eclipse.aether.internal.impl.synccontext.named.NameMapper; @@ -203,7 +201,6 @@ protected void configure() bind( ChecksumAlgorithmFactorySelector.class ) .to( DefaultChecksumAlgorithmFactorySelector.class ).in ( Singleton.class ); - bind( NamedLockFactorySelector.class ).to( SimpleNamedLockFactorySelector.class ).in( Singleton.class ); bind( SyncContextFactory.class ).to( DefaultSyncContextFactory.class ).in( Singleton.class ); bind( org.eclipse.aether.impl.SyncContextFactory.class ) .to( org.eclipse.aether.internal.impl.synccontext.legacy.DefaultSyncContextFactory.class ) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java index b488cb4b9..03b9d073c 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java @@ -19,20 +19,34 @@ * under the License. */ -import java.util.Objects; - import javax.annotation.PreDestroy; import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; + import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.SyncContext; +import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper; +import org.eclipse.aether.internal.impl.synccontext.named.FileGAVNameMapper; +import org.eclipse.aether.internal.impl.synccontext.named.GAVNameMapper; +import org.eclipse.aether.internal.impl.synccontext.named.NameMapper; import org.eclipse.aether.internal.impl.synccontext.named.NamedLockFactoryAdapter; -import org.eclipse.aether.internal.impl.synccontext.named.NamedLockFactorySelector; +import org.eclipse.aether.internal.impl.synccontext.named.StaticNameMapper; +import org.eclipse.aether.named.NamedLockFactory; +import org.eclipse.aether.named.providers.FileLockNamedLockFactory; +import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory; +import org.eclipse.aether.named.providers.LocalSemaphoreNamedLockFactory; +import org.eclipse.aether.named.providers.NoopNamedLockFactory; import org.eclipse.aether.spi.locator.Service; import org.eclipse.aether.spi.locator.ServiceLocator; import org.eclipse.aether.spi.synccontext.SyncContextFactory; +import org.eclipse.aether.util.ConfigUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static java.util.Objects.requireNonNull; @@ -44,20 +58,41 @@ public final class DefaultSyncContextFactory implements SyncContextFactory, Service { - private NamedLockFactoryAdapter namedLockFactoryAdapter; + private static final Logger LOGGER = LoggerFactory.getLogger( DefaultSyncContextFactory.class ); + + private static final String ADAPTER_KEY = DefaultSyncContextFactory.class.getName() + ".adapter"; + + private static final String NAME_MAPPER_KEY = "aether.syncContext.named.nameMapper"; + + private static final String DEFAULT_NAME_MAPPER = FileGAVNameMapper.NAME; + + private static final String FACTORY_KEY = "aether.syncContext.named.factory"; + + private static final String DEFAULT_FACTORY = FileLockNamedLockFactory.NAME; + + private Map nameMappers; + + private Map namedLockFactories; + + private final CopyOnWriteArrayList createdAdapters = new CopyOnWriteArrayList<>(); /** * Constructor used with DI, where factories are injected and selected based on key. */ @Inject - public DefaultSyncContextFactory( final NamedLockFactorySelector selector ) + public DefaultSyncContextFactory( final Map nameMappers, + final Map namedLockFactories ) { - this.namedLockFactoryAdapter = new NamedLockFactoryAdapter( - selector.getSelectedNameMapper(), - selector.getSelectedNamedLockFactory() - ); + this.nameMappers = requireNonNull( nameMappers ); + this.namedLockFactories = requireNonNull( namedLockFactories ); } + /** + * ServiceLocator default ctor. + * + * @deprecated Will be removed once ServiceLocator removed. + */ + @Deprecated public DefaultSyncContextFactory() { // ctor for ServiceLoader @@ -66,24 +101,69 @@ public DefaultSyncContextFactory() @Override public void initService( final ServiceLocator locator ) { - NamedLockFactorySelector selector = Objects.requireNonNull( - locator.getService( NamedLockFactorySelector.class ) ); - this.namedLockFactoryAdapter = new NamedLockFactoryAdapter( - selector.getSelectedNameMapper(), - selector.getSelectedNamedLockFactory() - ); + HashMap mappers = new HashMap<>(); + mappers.put( StaticNameMapper.NAME, new StaticNameMapper() ); + mappers.put( GAVNameMapper.NAME, new GAVNameMapper() ); + mappers.put( DiscriminatingNameMapper.NAME, new DiscriminatingNameMapper( new GAVNameMapper() ) ); + mappers.put( FileGAVNameMapper.NAME, new FileGAVNameMapper() ); + this.nameMappers = mappers; + + HashMap factories = new HashMap<>(); + factories.put( NoopNamedLockFactory.NAME, new NoopNamedLockFactory() ); + factories.put( LocalReadWriteLockNamedLockFactory.NAME, new LocalReadWriteLockNamedLockFactory() ); + factories.put( LocalSemaphoreNamedLockFactory.NAME, new LocalSemaphoreNamedLockFactory() ); + factories.put( FileLockNamedLockFactory.NAME, new FileLockNamedLockFactory() ); + this.namedLockFactories = factories; } @Override public SyncContext newInstance( final RepositorySystemSession session, final boolean shared ) { requireNonNull( session, "session cannot be null" ); - return namedLockFactoryAdapter.newInstance( session, shared ); + NamedLockFactoryAdapter adapter = + (NamedLockFactoryAdapter) session.getData().computeIfAbsent( + ADAPTER_KEY, + () -> createAdapter( session ) + ); + return adapter.newInstance( session, shared ); + } + + private NamedLockFactoryAdapter createAdapter( final RepositorySystemSession session ) + { + String nameMapperName = ConfigUtils.getString( session, DEFAULT_NAME_MAPPER, NAME_MAPPER_KEY ); + String namedLockFactoryName = ConfigUtils.getString( session, DEFAULT_FACTORY, FACTORY_KEY ); + NameMapper nameMapper = nameMappers.get( nameMapperName ); + if ( nameMapper == null ) + { + throw new IllegalArgumentException( "Unknown NameMapper name: " + namedLockFactoryName + + ", known ones: " + nameMappers.keySet() ); + } + NamedLockFactory namedLockFactory = namedLockFactories.get( namedLockFactoryName ); + if ( namedLockFactory == null ) + { + throw new IllegalArgumentException( "Unknown NamedLockFactory name: " + namedLockFactoryName + + ", known ones: " + namedLockFactories.keySet() ); + } + NamedLockFactoryAdapter adapter = new NamedLockFactoryAdapter( nameMapper, namedLockFactory ); + createdAdapters.add( adapter ); + return adapter; } @PreDestroy public void shutdown() { - namedLockFactoryAdapter.shutdown(); + LOGGER.debug( "Shutting down created adapters." ); + createdAdapters.forEach( adapter -> + { + try + { + adapter.shutdown(); + } + catch ( Exception e ) + { + LOGGER.warn( "Could not shutdown adapter", e ); + } + } + ); } } diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelector.java index 653d39b23..36b92e60f 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelector.java @@ -24,7 +24,11 @@ /** * Selector for {@link NamedLockFactory} and {@link NameMapper} that selects and exposes selected ones. Essentially * all the named locks configuration is here. Implementations may use different strategies to perform selection. + * + * @deprecated left in place but is completely unused. + * @see org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory */ +@Deprecated public interface NamedLockFactorySelector { /** diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelectorSupport.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelectorSupport.java index 9e525d8f8..e906805db 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelectorSupport.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelectorSupport.java @@ -28,7 +28,10 @@ * alternative way of configuration. This implementation uses Java System properties to select factory and name mapper. * * @since 1.7.3 + * @deprecated left in place but is completely unused. + * @see org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory */ +@Deprecated public abstract class NamedLockFactorySelectorSupport implements NamedLockFactorySelector { diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SimpleNamedLockFactorySelector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SimpleNamedLockFactorySelector.java index 964136227..938b2ed01 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SimpleNamedLockFactorySelector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SimpleNamedLockFactorySelector.java @@ -36,7 +36,10 @@ * default name lock factory and name mapper. * * @since 1.7.3 + * @deprecated left in place but is completely unused. + * @see org.eclipse.aether.internal.impl.synccontext.DefaultSyncContextFactory */ +@Deprecated @Singleton @Named public final class SimpleNamedLockFactorySelector From 1d85cec35abbbb441f02848388d6d715800390af Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 24 Jun 2022 12:18:49 +0200 Subject: [PATCH 2/2] Keep original defaults --- .../internal/impl/synccontext/DefaultSyncContextFactory.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java index 03b9d073c..27c9f108e 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/DefaultSyncContextFactory.java @@ -64,11 +64,11 @@ public final class DefaultSyncContextFactory private static final String NAME_MAPPER_KEY = "aether.syncContext.named.nameMapper"; - private static final String DEFAULT_NAME_MAPPER = FileGAVNameMapper.NAME; + private static final String DEFAULT_NAME_MAPPER = GAVNameMapper.NAME; private static final String FACTORY_KEY = "aether.syncContext.named.factory"; - private static final String DEFAULT_FACTORY = FileLockNamedLockFactory.NAME; + private static final String DEFAULT_FACTORY = LocalReadWriteLockNamedLockFactory.NAME; private Map nameMappers;