diff --git a/maven-resolver-api/src/main/java/org/eclipse/aether/AbstractForwardingRepositorySystemSession.java b/maven-resolver-api/src/main/java/org/eclipse/aether/AbstractForwardingRepositorySystemSession.java index 008a2df55..8fe24c1ab 100644 --- a/maven-resolver-api/src/main/java/org/eclipse/aether/AbstractForwardingRepositorySystemSession.java +++ b/maven-resolver-api/src/main/java/org/eclipse/aether/AbstractForwardingRepositorySystemSession.java @@ -20,6 +20,7 @@ */ import java.util.Map; +import java.util.function.Consumer; import org.eclipse.aether.artifact.ArtifactTypeRegistry; import org.eclipse.aether.collection.DependencyGraphTransformer; @@ -218,4 +219,27 @@ public FileTransformerManager getFileTransformerManager() return getSession().getFileTransformerManager(); } + @Override + public final void addOnCloseHandler( Consumer handler ) + { + getSession().addOnCloseHandler( handler ); + } + + @Override + public final boolean isClosed() + { + return getSession().isClosed(); + } + + /** + * This method is special: by default it throws (nested session should never be closed), the "top level" session + * should be closed instead. Still, this method is NOT {@code final}, to allow implementations overriding it, + * and in case when needed, handle forwarded session as "top level" session. + */ + @Override + public void close() + { + throw new IllegalStateException( "Forwarding session should not be closed, " + + "close the top-level (forwarded) session instead." ); + } } diff --git a/maven-resolver-api/src/main/java/org/eclipse/aether/DefaultRepositorySystemSession.java b/maven-resolver-api/src/main/java/org/eclipse/aether/DefaultRepositorySystemSession.java index 90b32f48f..4450487f6 100644 --- a/maven-resolver-api/src/main/java/org/eclipse/aether/DefaultRepositorySystemSession.java +++ b/maven-resolver-api/src/main/java/org/eclipse/aether/DefaultRepositorySystemSession.java @@ -19,12 +19,16 @@ * under the License. */ +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.Map; import static java.util.Objects.requireNonNull; import java.util.Collection; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; import org.eclipse.aether.artifact.Artifact; import org.eclipse.aether.artifact.ArtifactType; @@ -62,6 +66,8 @@ public final class DefaultRepositorySystemSession implements RepositorySystemSession { + private final AtomicBoolean closed; + private boolean readOnly; private boolean offline; @@ -127,6 +133,7 @@ public final class DefaultRepositorySystemSession */ public DefaultRepositorySystemSession() { + closed = new AtomicBoolean( false ); systemProperties = new HashMap<>(); systemPropertiesView = Collections.unmodifiableMap( systemProperties ); userProperties = new HashMap<>(); @@ -153,6 +160,7 @@ public DefaultRepositorySystemSession( RepositorySystemSession session ) { requireNonNull( session, "repository system session cannot be null" ); + closed = new AtomicBoolean( false ); setOffline( session.isOffline() ); setIgnoreArtifactDescriptorRepositories( session.isIgnoreArtifactDescriptorRepositories() ); setResolutionErrorPolicy( session.getResolutionErrorPolicy() ); @@ -194,7 +202,7 @@ public boolean isOffline() */ public DefaultRepositorySystemSession setOffline( boolean offline ) { - failIfReadOnly(); + verifyStateForMutation(); this.offline = offline; return this; } @@ -215,7 +223,7 @@ public boolean isIgnoreArtifactDescriptorRepositories() public DefaultRepositorySystemSession setIgnoreArtifactDescriptorRepositories( boolean ignoreArtifactDescriptorRepositories ) { - failIfReadOnly(); + verifyStateForMutation(); this.ignoreArtifactDescriptorRepositories = ignoreArtifactDescriptorRepositories; return this; } @@ -234,7 +242,7 @@ public ResolutionErrorPolicy getResolutionErrorPolicy() */ public DefaultRepositorySystemSession setResolutionErrorPolicy( ResolutionErrorPolicy resolutionErrorPolicy ) { - failIfReadOnly(); + verifyStateForMutation(); this.resolutionErrorPolicy = resolutionErrorPolicy; return this; } @@ -254,7 +262,7 @@ public ArtifactDescriptorPolicy getArtifactDescriptorPolicy() public DefaultRepositorySystemSession setArtifactDescriptorPolicy( ArtifactDescriptorPolicy artifactDescriptorPolicy ) { - failIfReadOnly(); + verifyStateForMutation(); this.artifactDescriptorPolicy = artifactDescriptorPolicy; return this; } @@ -276,7 +284,7 @@ public String getChecksumPolicy() */ public DefaultRepositorySystemSession setChecksumPolicy( String checksumPolicy ) { - failIfReadOnly(); + verifyStateForMutation(); this.checksumPolicy = checksumPolicy; return this; } @@ -298,7 +306,7 @@ public String getUpdatePolicy() */ public DefaultRepositorySystemSession setUpdatePolicy( String updatePolicy ) { - failIfReadOnly(); + verifyStateForMutation(); this.updatePolicy = updatePolicy; return this; } @@ -323,7 +331,7 @@ public LocalRepositoryManager getLocalRepositoryManager() */ public DefaultRepositorySystemSession setLocalRepositoryManager( LocalRepositoryManager localRepositoryManager ) { - failIfReadOnly(); + verifyStateForMutation(); this.localRepositoryManager = localRepositoryManager; return this; } @@ -336,7 +344,7 @@ public FileTransformerManager getFileTransformerManager() public DefaultRepositorySystemSession setFileTransformerManager( FileTransformerManager fileTransformerManager ) { - failIfReadOnly(); + verifyStateForMutation(); this.fileTransformerManager = fileTransformerManager; if ( this.fileTransformerManager == null ) { @@ -359,7 +367,7 @@ public WorkspaceReader getWorkspaceReader() */ public DefaultRepositorySystemSession setWorkspaceReader( WorkspaceReader workspaceReader ) { - failIfReadOnly(); + verifyStateForMutation(); this.workspaceReader = workspaceReader; return this; } @@ -377,7 +385,7 @@ public RepositoryListener getRepositoryListener() */ public DefaultRepositorySystemSession setRepositoryListener( RepositoryListener repositoryListener ) { - failIfReadOnly(); + verifyStateForMutation(); this.repositoryListener = repositoryListener; return this; } @@ -395,7 +403,7 @@ public TransferListener getTransferListener() */ public DefaultRepositorySystemSession setTransferListener( TransferListener transferListener ) { - failIfReadOnly(); + verifyStateForMutation(); this.transferListener = transferListener; return this; } @@ -444,7 +452,7 @@ public Map getSystemProperties() */ public DefaultRepositorySystemSession setSystemProperties( Map systemProperties ) { - failIfReadOnly(); + verifyStateForMutation(); this.systemProperties = copySafe( systemProperties, String.class ); systemPropertiesView = Collections.unmodifiableMap( this.systemProperties ); return this; @@ -459,7 +467,7 @@ public DefaultRepositorySystemSession setSystemProperties( Map systemPrope */ public DefaultRepositorySystemSession setSystemProperty( String key, String value ) { - failIfReadOnly(); + verifyStateForMutation(); if ( value != null ) { systemProperties.put( key, value ); @@ -489,7 +497,7 @@ public Map getUserProperties() */ public DefaultRepositorySystemSession setUserProperties( Map userProperties ) { - failIfReadOnly(); + verifyStateForMutation(); this.userProperties = copySafe( userProperties, String.class ); userPropertiesView = Collections.unmodifiableMap( this.userProperties ); return this; @@ -504,7 +512,7 @@ public DefaultRepositorySystemSession setUserProperties( Map userPropertie */ public DefaultRepositorySystemSession setUserProperty( String key, String value ) { - failIfReadOnly(); + verifyStateForMutation(); if ( value != null ) { userProperties.put( key, value ); @@ -533,7 +541,7 @@ public Map getConfigProperties() */ public DefaultRepositorySystemSession setConfigProperties( Map configProperties ) { - failIfReadOnly(); + verifyStateForMutation(); this.configProperties = copySafe( configProperties, Object.class ); configPropertiesView = Collections.unmodifiableMap( this.configProperties ); return this; @@ -548,7 +556,7 @@ public DefaultRepositorySystemSession setConfigProperties( Map configPrope */ public DefaultRepositorySystemSession setConfigProperty( String key, Object value ) { - failIfReadOnly(); + verifyStateForMutation(); if ( value != null ) { configProperties.put( key, value ); @@ -575,7 +583,7 @@ public MirrorSelector getMirrorSelector() */ public DefaultRepositorySystemSession setMirrorSelector( MirrorSelector mirrorSelector ) { - failIfReadOnly(); + verifyStateForMutation(); this.mirrorSelector = mirrorSelector; if ( this.mirrorSelector == null ) { @@ -600,7 +608,7 @@ public ProxySelector getProxySelector() */ public DefaultRepositorySystemSession setProxySelector( ProxySelector proxySelector ) { - failIfReadOnly(); + verifyStateForMutation(); this.proxySelector = proxySelector; if ( this.proxySelector == null ) { @@ -625,7 +633,7 @@ public AuthenticationSelector getAuthenticationSelector() */ public DefaultRepositorySystemSession setAuthenticationSelector( AuthenticationSelector authenticationSelector ) { - failIfReadOnly(); + verifyStateForMutation(); this.authenticationSelector = authenticationSelector; if ( this.authenticationSelector == null ) { @@ -647,7 +655,7 @@ public ArtifactTypeRegistry getArtifactTypeRegistry() */ public DefaultRepositorySystemSession setArtifactTypeRegistry( ArtifactTypeRegistry artifactTypeRegistry ) { - failIfReadOnly(); + verifyStateForMutation(); this.artifactTypeRegistry = artifactTypeRegistry; if ( this.artifactTypeRegistry == null ) { @@ -669,7 +677,7 @@ public DependencyTraverser getDependencyTraverser() */ public DefaultRepositorySystemSession setDependencyTraverser( DependencyTraverser dependencyTraverser ) { - failIfReadOnly(); + verifyStateForMutation(); this.dependencyTraverser = dependencyTraverser; return this; } @@ -687,7 +695,7 @@ public DependencyManager getDependencyManager() */ public DefaultRepositorySystemSession setDependencyManager( DependencyManager dependencyManager ) { - failIfReadOnly(); + verifyStateForMutation(); this.dependencyManager = dependencyManager; return this; } @@ -705,7 +713,7 @@ public DependencySelector getDependencySelector() */ public DefaultRepositorySystemSession setDependencySelector( DependencySelector dependencySelector ) { - failIfReadOnly(); + verifyStateForMutation(); this.dependencySelector = dependencySelector; return this; } @@ -724,7 +732,7 @@ public VersionFilter getVersionFilter() */ public DefaultRepositorySystemSession setVersionFilter( VersionFilter versionFilter ) { - failIfReadOnly(); + verifyStateForMutation(); this.versionFilter = versionFilter; return this; } @@ -744,7 +752,7 @@ public DependencyGraphTransformer getDependencyGraphTransformer() public DefaultRepositorySystemSession setDependencyGraphTransformer( DependencyGraphTransformer dependencyGraphTransformer ) { - failIfReadOnly(); + verifyStateForMutation(); this.dependencyGraphTransformer = dependencyGraphTransformer; return this; } @@ -762,7 +770,7 @@ public SessionData getData() */ public DefaultRepositorySystemSession setData( SessionData data ) { - failIfReadOnly(); + verifyStateForMutation(); this.data = data; if ( this.data == null ) { @@ -784,7 +792,7 @@ public RepositoryCache getCache() */ public DefaultRepositorySystemSession setCache( RepositoryCache cache ) { - failIfReadOnly(); + verifyStateForMutation(); this.cache = cache; return this; } @@ -799,12 +807,19 @@ public void setReadOnly() readOnly = true; } - private void failIfReadOnly() + /** + * Verifies this instance state for mutation operations: mutated instance must not be read-only or closed. + */ + private void verifyStateForMutation() { if ( readOnly ) { throw new IllegalStateException( "repository system session is read-only" ); } + if ( isClosed() ) + { + throw new IllegalStateException( "repository system session is closed" ); + } } static class NullProxySelector @@ -873,4 +888,41 @@ public Collection getTransformersForArtifact( Artifact artifact } } + private final CopyOnWriteArrayList> onCloseHandlers + = new CopyOnWriteArrayList<>(); + + @Override + public void addOnCloseHandler( Consumer handler ) + { + verifyStateForMutation(); + requireNonNull( handler, "handler cannot be null" ); + onCloseHandlers.add( 0, handler ); + } + + @Override + public boolean isClosed() + { + return closed.get(); + } + + @Override + public void close() + { + if ( closed.compareAndSet( false, true ) ) + { + ArrayList exceptions = new ArrayList<>(); + for ( Consumer onCloseHandler : onCloseHandlers ) + { + try + { + onCloseHandler.accept( this ); + } + catch ( Exception e ) + { + exceptions.add( e ); + } + } + MultiRuntimeException.mayThrow( "session on-close handler failures", exceptions ); + } + } } diff --git a/maven-resolver-api/src/main/java/org/eclipse/aether/MultiRuntimeException.java b/maven-resolver-api/src/main/java/org/eclipse/aether/MultiRuntimeException.java new file mode 100644 index 000000000..24d48f935 --- /dev/null +++ b/maven-resolver-api/src/main/java/org/eclipse/aether/MultiRuntimeException.java @@ -0,0 +1,74 @@ +package org.eclipse.aether; + +/* + * 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 java.util.List; + +import static java.util.Objects.requireNonNull; + +/** + * Runtime exception to be thrown when multiple actions were executed and one or more failed. To be used when no + * fallback on resolver side is needed or is possible. + * + * @since TBD + */ +public final class MultiRuntimeException + extends RuntimeException +{ + private final List throwables; + + private MultiRuntimeException( String message, List throwables ) + { + super( message ); + this.throwables = throwables; + for ( Throwable throwable : throwables ) + { + addSuppressed( throwable ); + } + } + + /** + * Returns the list of throwables that are wrapped in this exception. + * + * @return The list of throwables, never {@code null}. + */ + public List getThrowables() + { + return throwables; + } + + /** + * Helper method that receives a (non-null) message and (non-null) list of throwable, and following happens: + *
    + *
  • if list is empty - nothing
    • + *
  • if list not empty - {@link MultiRuntimeException} is thrown wrapping all elements
    • + *
+ */ + public static void mayThrow( String message, List throwables ) + { + requireNonNull( message ); + requireNonNull( throwables ); + + if ( !throwables.isEmpty() ) + { + throw new MultiRuntimeException( message, throwables ); + } + } +} diff --git a/maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java b/maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java index 7aef49af2..0c2bbac90 100644 --- a/maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java +++ b/maven-resolver-api/src/main/java/org/eclipse/aether/RepositorySystemSession.java @@ -20,6 +20,7 @@ */ import java.util.Map; +import java.util.function.Consumer; import org.eclipse.aether.artifact.ArtifactTypeRegistry; import org.eclipse.aether.collection.DependencyGraphTransformer; @@ -48,7 +49,7 @@ * @noimplement This interface is not intended to be implemented by clients. * @noextend This interface is not intended to be extended by clients. */ -public interface RepositorySystemSession +public interface RepositorySystemSession extends AutoCloseable { /** @@ -271,4 +272,57 @@ public interface RepositorySystemSession @Deprecated FileTransformerManager getFileTransformerManager(); + /** + * Adds "on close" handler to this session, it must not be {@code null}. Note: when handlers are invoked, the + * passed in (this) session is ALREADY CLOSED (the {@link #isClosed()} method returns {@code true}). This implies, + * that handlers cannot use {@link RepositorySystem} to resolve/collect/and so on, handlers are meant to perform + * some internal cleanup on session close. Attempt to add handler to closed session will throw. + * + * @since TBD + */ + void addOnCloseHandler( Consumer handler ); + + /** + * Returns {@code true} if this instance was already closed. Closed sessions should NOT be used anymore. + * + * @return {@code true} if session was closed. + * @since TBD + */ + boolean isClosed(); + + /** + * Closes this session and possibly releases related resources by invoking "on close" handlers added by + * {@link #addOnCloseHandler(Consumer)} method. This method may be invoked multiple times, + * but only first invocation will actually invoke handlers, subsequent invocations will be no-op. + *

+ * When close action happens, all the registered handlers will be invoked (even if some throws), and at end + * of operation a {@link MultiRuntimeException} may be thrown signaling that some handler(s) failed. This exception + * may be ignored, is at the discretion of caller. + *

+ * Important: ideally it is the session creator who should be responsible to close the session. The "nested" + * (delegating, wrapped) sessions {@link AbstractForwardingRepositorySystemSession} and alike) by default + * (without overriding the {@link AbstractForwardingRepositorySystemSession#close()} method) are prevented to close + * session, and it is the "recommended" behaviour as well. On the other hand, "nested" session may receive new + * "on close" handler registrations, but those registrations are passed to delegated session, and will be invoked + * once the "top level" (delegated) session is closed. + *

+ * New session "copy" instances created using copy-constructor + * {@link DefaultRepositorySystemSession#DefaultRepositorySystemSession(RepositorySystemSession)} result in new, + * independent session instances, and they do NOT share states like "read-only", "closed" and "on close handlers" + * with the copied session. Hence, they should be treated as "top level" sessions as well. + *

+ * The recommended pattern for "top level" sessions is the usual try-with-resource: + * + * 

 {@code
+     * try ( RepositorySystemSession session = create session... )
+     * {
+     *   ... use/nest session
+     * }
+     * }
+ * + * @since TBD + */ + @Override + void close(); + } diff --git a/maven-resolver-api/src/test/java/org/eclipse/aether/DefaultRepositorySystemSessionTest.java b/maven-resolver-api/src/test/java/org/eclipse/aether/DefaultRepositorySystemSessionTest.java index ccca26ad6..1fe084640 100644 --- a/maven-resolver-api/src/test/java/org/eclipse/aether/DefaultRepositorySystemSessionTest.java +++ b/maven-resolver-api/src/test/java/org/eclipse/aether/DefaultRepositorySystemSessionTest.java @@ -140,7 +140,10 @@ public void testCopyRepositorySystemSession() throws Exception for ( Method method : methods ) { - assertEquals( method.getName(), method.invoke( session ) == null, method.invoke( newSession ) == null ); + if ( method.getParameterCount() == 0 ) + { + assertEquals( method.getName(), method.invoke( session ) == null, method.invoke( newSession ) == null ); + } } } diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/sisu/SisuRepositorySystemDemoModule.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/sisu/SisuRepositorySystemDemoModule.java index 71adb55b7..d4a43c8fd 100644 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/sisu/SisuRepositorySystemDemoModule.java +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/sisu/SisuRepositorySystemDemoModule.java @@ -23,7 +23,6 @@ import com.google.inject.Binder; import com.google.inject.Module; -import org.eclipse.sisu.bean.LifecycleModule; import org.eclipse.sisu.inject.MutableBeanLocator; import org.eclipse.sisu.wire.ParameterKeys; @@ -35,7 +34,6 @@ public class SisuRepositorySystemDemoModule implements Module @Override public void configure( final Binder binder ) { - binder.install( new LifecycleModule() ); // NOTE: Maven 3.8.1 used in demo has Sisu Index for ALL components (older Maven does NOT have!) binder.bind( ParameterKeys.PROPERTIES ).toInstance( System.getProperties() ); binder.bind( ShutdownThread.class ).asEagerSingleton(); diff --git a/maven-resolver-impl/pom.xml b/maven-resolver-impl/pom.xml index 70d0bc668..0f9fe22f8 100644 --- a/maven-resolver-impl/pom.xml +++ b/maven-resolver-impl/pom.xml @@ -68,11 +68,6 @@ provided true - - javax.annotation - javax.annotation-api - 1.3.2 - org.eclipse.sisu org.eclipse.sisu.inject diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositorySystem.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositorySystem.java index f079cff6a..1f7a1c2e6 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositorySystem.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositorySystem.java @@ -466,6 +466,10 @@ private void validateSession( RepositorySystemSession session ) invalidSession( session.getAuthenticationSelector(), "authentication selector" ); invalidSession( session.getArtifactTypeRegistry(), "artifact type registry" ); invalidSession( session.getData(), "data" ); + if ( session.isClosed() ) + { + throw new IllegalStateException( "session is already closed" ); + } } private void validateRepositories( List repositories ) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SummaryFileTrustedChecksumsSource.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SummaryFileTrustedChecksumsSource.java index d6f7e21cc..4568723b1 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SummaryFileTrustedChecksumsSource.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SummaryFileTrustedChecksumsSource.java @@ -29,13 +29,17 @@ import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; -import java.nio.file.StandardOpenOption; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; +import org.eclipse.aether.MultiRuntimeException; import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.artifact.Artifact; import org.eclipse.aether.repository.ArtifactRepository; @@ -74,6 +78,8 @@ public final class SummaryFileTrustedChecksumsSource private static final String CHECKSUMS_CACHE_KEY = SummaryFileTrustedChecksumsSource.class.getName() + ".checksums"; + private static final String RECORDING_KEY = SummaryFileTrustedChecksumsSource.class.getName() + ".recording"; + private static final Logger LOGGER = LoggerFactory.getLogger( SummaryFileTrustedChecksumsSource.class ); public SummaryFileTrustedChecksumsSource() @@ -111,10 +117,17 @@ protected Map performLookup( RepositorySystemSession session, return checksums; } + @SuppressWarnings( "unchecked" ) @Override protected SummaryFileWriter getWriter( RepositorySystemSession session, Path basedir ) { - return new SummaryFileWriter( basedir, isOriginAware( session ) ); + ConcurrentHashMap> recordedLines = (ConcurrentHashMap>) session.getData() + .computeIfAbsent( RECORDING_KEY, () -> + { + session.addOnCloseHandler( this::saveSessionRecordedLines ); + return new ConcurrentHashMap<>(); + } ); + return new SummaryFileWriter( basedir, isOriginAware( session ), recordedLines ); } private ConcurrentHashMap loadProvidedChecksums( Path basedir, @@ -201,41 +214,38 @@ private String calculateSummaryPath( boolean originAware, return fileName + "." + checksumAlgorithmFactory.getFileExtension(); } - /** - * Note: this implementation will work only in single-thread (T1) model. While not ideal, the "workaround" is - * possible in both, Maven and Maven Daemon: force single threaded execution model while "recording" (in mvn: - * do not pass any {@code -T} CLI parameter, while for mvnd use {@code -1} CLI parameter. - * - * TODO: this will need to be reworked for at least two reasons: a) avoid duplicates in summary file and b) - * support multi threaded builds (probably will need "on session close" hook). - */ private class SummaryFileWriter implements Writer { private final Path basedir; private final boolean originAware; - private SummaryFileWriter( Path basedir, boolean originAware ) + private final ConcurrentHashMap> recordedLines; + + private SummaryFileWriter( Path basedir, + boolean originAware, + ConcurrentHashMap> recordedLines ) { this.basedir = basedir; this.originAware = originAware; + this.recordedLines = recordedLines; } @Override public void addTrustedArtifactChecksums( Artifact artifact, ArtifactRepository artifactRepository, List checksumAlgorithmFactories, - Map trustedArtifactChecksums ) throws IOException + Map trustedArtifactChecksums ) { for ( ChecksumAlgorithmFactory checksumAlgorithmFactory : checksumAlgorithmFactories ) { String checksum = requireNonNull( trustedArtifactChecksums.get( checksumAlgorithmFactory.getName() ) ); - String summaryLine = ArtifactIdUtils.toId( artifact ) + " " + checksum + "\n"; + String summaryLine = ArtifactIdUtils.toId( artifact ) + " " + checksum; Path summaryPath = basedir.resolve( calculateSummaryPath( originAware, artifactRepository, checksumAlgorithmFactory ) ); - Files.createDirectories( summaryPath.getParent() ); - Files.write( summaryPath, summaryLine.getBytes( StandardCharsets.UTF_8 ), - StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.APPEND ); + + recordedLines.computeIfAbsent( summaryPath, p -> Collections.synchronizedSet( new TreeSet<>() ) ) + .add( summaryLine ); } } @@ -245,4 +255,35 @@ public void close() // nop } } + + @SuppressWarnings( "unchecked" ) + private void saveSessionRecordedLines( RepositorySystemSession session ) + { + Map> recordedLines = (Map>) session.getData().get( RECORDING_KEY ); + if ( recordedLines == null || recordedLines.isEmpty() ) + { + return; + } + + ArrayList exceptions = new ArrayList<>(); + for ( Map.Entry> entry : recordedLines.entrySet() ) + { + Path file = entry.getKey(); + Set lines = entry.getValue(); + if ( !lines.isEmpty() ) + { + LOGGER.info( "Saving {} checksums to '{}'", lines.size(), file ); + try + { + Files.createDirectories( file.getParent() ); + Files.write( file, lines ); + } + catch ( IOException e ) + { + exceptions.add( e ); + } + } + } + MultiRuntimeException.mayThrow( "session save checksums failure", exceptions ); + } } 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 65ba3d736..d7f6bd6c7 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,14 +19,12 @@ * under the License. */ -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; @@ -46,8 +44,6 @@ 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; @@ -59,8 +55,6 @@ public final class DefaultSyncContextFactory implements SyncContextFactory, Service { - 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"; @@ -75,8 +69,6 @@ public final class DefaultSyncContextFactory private Map namedLockFactories; - private final CopyOnWriteArrayList createdAdapters = new CopyOnWriteArrayList<>(); - /** * Constructor used with DI, where factories are injected and selected based on key. */ @@ -122,50 +114,39 @@ public void initService( final ServiceLocator locator ) public SyncContext newInstance( final RepositorySystemSession session, final boolean shared ) { requireNonNull( session, "session cannot be null" ); - NamedLockFactoryAdapter adapter = - (NamedLockFactoryAdapter) session.getData().computeIfAbsent( - ADAPTER_KEY, - () -> createAdapter( session ) - ); + NamedLockFactoryAdapter adapter = getOrCreateSessionAdapter( session ); return adapter.newInstance( session, shared ); } - private NamedLockFactoryAdapter createAdapter( final RepositorySystemSession session ) + private NamedLockFactoryAdapter getOrCreateSessionAdapter( 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 ) + return (NamedLockFactoryAdapter) session.getData().computeIfAbsent( ADAPTER_KEY, () -> { - throw new IllegalArgumentException( "Unknown NamedLockFactory name: " + namedLockFactoryName - + ", known ones: " + namedLockFactories.keySet() ); - } - NamedLockFactoryAdapter adapter = new NamedLockFactoryAdapter( nameMapper, namedLockFactory ); - createdAdapters.add( adapter ); - return adapter; + 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() ); + } + session.addOnCloseHandler( this::shutDownSessionAdapter ); + return new NamedLockFactoryAdapter( nameMapper, namedLockFactory ); + } ); } - @PreDestroy - public void shutdown() + private void shutDownSessionAdapter( RepositorySystemSession session ) { - LOGGER.debug( "Shutting down created adapters..." ); - createdAdapters.forEach( adapter -> - { - try - { - adapter.shutdown(); - } - catch ( Exception e ) - { - LOGGER.warn( "Could not shutdown: {}", adapter, e ); - } - } - ); + NamedLockFactoryAdapter adapter = (NamedLockFactoryAdapter) session.getData().get( ADAPTER_KEY ); + if ( adapter != null ) + { + adapter.shutdown(); + } } }