From 510dcd38cda55bf7f1091305703b77e10a5c5580 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 24 Jun 2022 12:00:45 +0200 Subject: [PATCH] [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. Important: This change also drops internal/impl things. Therefore, this is a breakage, IF someone uses internal/private/impl details from Resolver (mvnd does, but is fine, is our kiddo), and for the rest of the worlds: hands off, please. This closes #196 --- maven-resolver-impl/pom.xml | 6 +- .../aether/impl/DefaultServiceLocator.java | 3 - .../aether/impl/guice/AetherModule.java | 3 - .../DefaultSyncContextFactory.java | 114 +++++++++++++++--- .../named/NamedLockFactoryAdapter.java | 8 ++ .../named/NamedLockFactorySelector.java | 39 ------ .../NamedLockFactorySelectorSupport.java | 109 ----------------- .../named/SimpleNamedLockFactorySelector.java | 83 ------------- 8 files changed, 108 insertions(+), 257 deletions(-) delete mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelector.java delete mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelectorSupport.java delete mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SimpleNamedLockFactorySelector.java diff --git a/maven-resolver-impl/pom.xml b/maven-resolver-impl/pom.xml index 3e1ad9623..70d0bc668 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..96996698b 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_NAME = GAVNameMapper.NAME; + + private static final String FACTORY_KEY = "aether.syncContext.named.factory"; + + private static final String DEFAULT_FACTORY_NAME = LocalReadWriteLockNamedLockFactory.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, NAME_MAPPER_KEY ); + String namedLockFactoryName = ConfigUtils.getString( session, DEFAULT_FACTORY_NAME, FACTORY_KEY ); + NameMapper nameMapper = nameMappers.get( nameMapperName ); + if ( nameMapper == null ) + { + throw new IllegalArgumentException( "Unknown NameMapper name: " + nameMapperName + + ", 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/NamedLockFactoryAdapter.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java index b59b26c79..bc1a74205 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java @@ -78,6 +78,14 @@ public void shutdown() namedLockFactory.shutdown(); } + public String toString() + { + return getClass().getSimpleName() + + "(nameMapper=" + nameMapper + + ", namedLockFactory=" + namedLockFactory + + ")"; + } + private static class AdaptedLockSyncContext implements SyncContext { private static final Logger LOGGER = LoggerFactory.getLogger( AdaptedLockSyncContext.class ); 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 deleted file mode 100644 index 653d39b23..000000000 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelector.java +++ /dev/null @@ -1,39 +0,0 @@ -package org.eclipse.aether.internal.impl.synccontext.named; - -/* - * 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 org.eclipse.aether.named.NamedLockFactory; - -/** - * 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. - */ -public interface NamedLockFactorySelector -{ - /** - * Returns the selected {@link NamedLockFactory}, never null. - */ - NamedLockFactory getSelectedNamedLockFactory(); - - /** - * Returns the selected {@link NameMapper}, never null. - */ - NameMapper getSelectedNameMapper(); -} 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 deleted file mode 100644 index 9e525d8f8..000000000 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactorySelectorSupport.java +++ /dev/null @@ -1,109 +0,0 @@ -package org.eclipse.aether.internal.impl.synccontext.named; - -/* - * 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 org.eclipse.aether.named.NamedLockFactory; - -import java.util.Map; - -/** - * Selector implementation support class: by extending this class one may override defaults, or provide completely - * alternative way of configuration. This implementation uses Java System properties to select factory and name mapper. - * - * @since 1.7.3 - */ -public abstract class NamedLockFactorySelectorSupport - implements NamedLockFactorySelector -{ - public static final String FACTORY_KEY = "aether.syncContext.named.factory"; - - public static final String NAME_MAPPER_KEY = "aether.syncContext.named.nameMapper"; - - private final NamedLockFactory namedLockFactory; - - private final NameMapper nameMapper; - - public NamedLockFactorySelectorSupport( final Map factories, - final String defaultFactoryName, - final Map nameMappers, - final String defaultNameMapperName ) - { - this.namedLockFactory = selectNamedLockFactory( factories, getFactoryName( defaultFactoryName ) ); - this.nameMapper = selectNameMapper( nameMappers, getNameMapperName( defaultNameMapperName ) ); - } - - /** - * Returns the selected {@link NamedLockFactory}, never null. - */ - @Override - public NamedLockFactory getSelectedNamedLockFactory() - { - return namedLockFactory; - } - - /** - * Returns the selected {@link NameMapper}, never null. - */ - @Override - public NameMapper getSelectedNameMapper() - { - return nameMapper; - } - - /** - * Returns selected factory name (or passed in default) using System property value of {@link #FACTORY_KEY}. - */ - protected String getFactoryName( final String defaultFactoryName ) - { - return System.getProperty( FACTORY_KEY, defaultFactoryName ); - } - - /** - * Returns selected name mapper name (or passed in default) using System property value of {@link #NAME_MAPPER_KEY}. - */ - protected String getNameMapperName( final String defaultNameMapperName ) - { - return System.getProperty( NAME_MAPPER_KEY, defaultNameMapperName ); - } - - private NamedLockFactory selectNamedLockFactory( final Map factories, - final String factoryName ) - { - NamedLockFactory factory = factories.get( factoryName ); - if ( factory == null ) - { - throw new IllegalArgumentException( "Unknown NamedLockFactory name: " + factoryName - + ", known ones: " + factories.keySet() ); - } - return factory; - } - - private NameMapper selectNameMapper( final Map nameMappers, - final String mapperName ) - { - NameMapper nameMapper = nameMappers.get( mapperName ); - if ( nameMapper == null ) - { - throw new IllegalArgumentException( "Unknown NameMapper name: " + mapperName - + ", known ones: " + nameMappers.keySet() ); - } - return nameMapper; - } -} 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 deleted file mode 100644 index 964136227..000000000 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SimpleNamedLockFactorySelector.java +++ /dev/null @@ -1,83 +0,0 @@ -package org.eclipse.aether.internal.impl.synccontext.named; - -/* - * 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 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 javax.inject.Inject; -import javax.inject.Named; -import javax.inject.Singleton; -import java.util.HashMap; -import java.util.Map; - -/** - * Simple selector implementation that uses {@link LocalReadWriteLockNamedLockFactory} and {@link GAVNameMapper} as - * default name lock factory and name mapper. - * - * @since 1.7.3 - */ -@Singleton -@Named -public final class SimpleNamedLockFactorySelector - extends NamedLockFactorySelectorSupport -{ - private static final Map FACTORIES; - - private static final Map NAME_MAPPERS; - - static - { - 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() ); - - NAME_MAPPERS = new HashMap<>(); - NAME_MAPPERS.put( StaticNameMapper.NAME, new StaticNameMapper() ); - NAME_MAPPERS.put( GAVNameMapper.NAME, new GAVNameMapper() ); - NAME_MAPPERS.put( DiscriminatingNameMapper.NAME, new DiscriminatingNameMapper( new GAVNameMapper() ) ); - NAME_MAPPERS.put( FileGAVNameMapper.NAME, new FileGAVNameMapper() ); - } - - /** - * Default constructor for ServiceLocator. - */ - public SimpleNamedLockFactorySelector() - { - this( FACTORIES, NAME_MAPPERS ); - } - - @Inject - public SimpleNamedLockFactorySelector( final Map factories, - final Map nameMappers ) - { - super( - factories, - LocalReadWriteLockNamedLockFactory.NAME, - nameMappers, - GAVNameMapper.NAME - ); - } -}