You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by cs...@apache.org on 2021/10/21 11:53:17 UTC
[maven-resolver] branch file-lock-improvement created (now 7030b51)
This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a change to branch file-lock-improvement
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git.
at 7030b51 File Locking change proposal
This branch includes the following new commits:
new 7030b51 File Locking change proposal
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[maven-resolver] 01/01: File Locking change proposal
Posted by cs...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch file-lock-improvement
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
commit 7030b516e86f050c1a38b14b868e70cb5790e9d9
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Thu Oct 21 13:52:13 2021 +0200
File Locking change proposal
Drop the hoops and loops, sort out everything in
name mapper (resolution against session baseDir)
instead doing path->string->path translations
in mapper then in FS lock factory.
---
.../impl/synccontext/NamedLockFactorySelector.java | 13 +++-
.../named/FileLockNamedLockFactory.java | 91 ----------------------
.../synccontext/named/NamedLockFactoryAdapter.java | 7 +-
.../impl/synccontext/named/TakariNameMapper.java | 49 ++++++++++--
.../impl/synccontext/FileLockAdapterTest.java | 3 +-
.../FileLockNamedLockFactory.java} | 32 +++++---
.../aether/named/support/FileSystemFriendly.java | 22 ++----
.../named/FileLockNamedLockFactorySupportTest.java | 26 ++++---
8 files changed, 97 insertions(+), 146 deletions(-)
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java
index 0d7a878..d947a1d 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactorySelector.java
@@ -28,15 +28,16 @@ import javax.inject.Named;
import javax.inject.Singleton;
import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper;
-import org.eclipse.aether.internal.impl.synccontext.named.FileLockNamedLockFactory;
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.StaticNameMapper;
import org.eclipse.aether.internal.impl.synccontext.named.TakariNameMapper;
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.named.support.FileSystemFriendly;
/**
* Selector for {@link NamedLockFactory} and {@link NameMapper} that selects and exposes selected ones. Essentially
@@ -75,6 +76,16 @@ public final class NamedLockFactorySelector
{
this.namedLockFactory = selectNamedLockFactory( factories );
this.nameMapper = selectNameMapper( nameMappers );
+
+ if ( namedLockFactory instanceof FileSystemFriendly )
+ {
+ if ( !( nameMapper instanceof FileSystemFriendly ) )
+ {
+ throw new IllegalArgumentException(
+ "Misconfiguration: FS friendly lock factory requires FS friendly name mapper"
+ );
+ }
+ }
}
/**
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/FileLockNamedLockFactory.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/FileLockNamedLockFactory.java
deleted file mode 100644
index a1dbbc8..0000000
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/FileLockNamedLockFactory.java
+++ /dev/null
@@ -1,91 +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.RepositorySystemSession;
-import org.eclipse.aether.named.support.FileLockNamedLockFactorySupport;
-import org.eclipse.aether.named.support.NamedLockSupport;
-
-import javax.inject.Named;
-import javax.inject.Singleton;
-import java.io.File;
-import java.io.IOException;
-import java.io.UncheckedIOException;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.concurrent.ConcurrentHashMap;
-
-/**
- * A {@link SessionAwareNamedLockFactory} that uses advisory file locking.
- *
- * @since TBD
- */
-@Singleton
-@Named( FileLockNamedLockFactory.NAME )
-public class FileLockNamedLockFactory
- extends FileLockNamedLockFactorySupport
- implements SessionAwareNamedLockFactory
-{
- public static final String NAME = "file-lock";
-
- private final ConcurrentHashMap<String, Path> baseDirs;
-
- public FileLockNamedLockFactory()
- {
- this.baseDirs = new ConcurrentHashMap<>();
- }
-
- @Override
- public NamedLockSupport getLock( final RepositorySystemSession session, final String name )
- {
- File localRepositoryBasedir = session.getLocalRepository().getBasedir();
- Path baseDir = baseDirs.computeIfAbsent(
- localRepositoryBasedir.getPath(), k ->
- {
- try
- {
- return new File( localRepositoryBasedir, ".locks" ).getCanonicalFile().toPath();
- }
- catch ( IOException e )
- {
- throw new UncheckedIOException( e );
- }
- }
- );
-
- String fileName = baseDir.resolve( name ).toAbsolutePath().toString();
- return super.getLock( fileName );
- }
-
- @Override
- public NamedLockSupport getLock( final String filename )
- {
- throw new UnsupportedOperationException( "This factory is session aware" );
- }
-
- /**
- * Use name as is, it is already resolved in {@link #getLock(RepositorySystemSession, String)} method.
- */
- @Override
- protected Path resolveName( final String name )
- {
- return Paths.get( name );
- }
-}
\ No newline at end of file
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 6fb68d3..1468278 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
@@ -80,8 +80,6 @@ public final class NamedLockFactoryAdapter
private final NameMapper lockNaming;
- private final SessionAwareNamedLockFactory sessionAwareNamedLockFactory;
-
private final NamedLockFactory namedLockFactory;
private final long time;
@@ -97,8 +95,6 @@ public final class NamedLockFactoryAdapter
this.session = session;
this.shared = shared;
this.lockNaming = lockNaming;
- this.sessionAwareNamedLockFactory = namedLockFactory instanceof SessionAwareNamedLockFactory
- ? (SessionAwareNamedLockFactory) namedLockFactory : null;
this.namedLockFactory = namedLockFactory;
this.time = time;
this.timeUnit = timeUnit;
@@ -118,8 +114,7 @@ public final class NamedLockFactoryAdapter
int acquiredLockCount = 0;
for ( String key : keys )
{
- NamedLock namedLock = sessionAwareNamedLockFactory != null ? sessionAwareNamedLockFactory
- .getLock( session, key ) : namedLockFactory.getLock( key );
+ NamedLock namedLock = namedLockFactory.getLock( key );
try
{
LOGGER.trace( "Acquiring {} lock for '{}'",
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/TakariNameMapper.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/TakariNameMapper.java
index 179ec69..f4e5481 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/TakariNameMapper.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/TakariNameMapper.java
@@ -22,14 +22,21 @@ package org.eclipse.aether.internal.impl.synccontext.named;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.support.FileSystemFriendly;
import javax.inject.Named;
import javax.inject.Singleton;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Path;
import java.util.Collection;
import java.util.TreeSet;
+import java.util.concurrent.ConcurrentHashMap;
/**
- * A {@link NameMapper} that creates same name mapping as Takari Local Repository does, without baseDir (local repo).
+ * A {@link NameMapper} that creates same name mapping as Takari Local Repository does, with baseDir (local repo).
* Part of code blatantly copies parts of the Takari {@code LockingSyncContext}.
*
* @see <a href="https://github.com/takari/takari-local-repository/blob/master/src/main/java/io/takari/aether/concurrency/LockingSyncContext.java">Takari
@@ -37,42 +44,68 @@ import java.util.TreeSet;
*/
@Singleton
@Named( TakariNameMapper.NAME )
-public class TakariNameMapper implements NameMapper
+public class TakariNameMapper
+ implements NameMapper, FileSystemFriendly
{
public static final String NAME = "takari";
private static final char SEPARATOR = '~';
+ private final ConcurrentHashMap<String, Path> baseDirs;
+
+ public TakariNameMapper()
+ {
+ this.baseDirs = new ConcurrentHashMap<>();
+ }
+
@Override
public TreeSet<String> nameLocks( final RepositorySystemSession session,
final Collection<? extends Artifact> artifacts,
final Collection<? extends Metadata> metadatas )
{
+ File localRepositoryBasedir = session.getLocalRepository().getBasedir();
+ Path baseDir = baseDirs.computeIfAbsent(
+ localRepositoryBasedir.getPath(), k ->
+ {
+ try
+ {
+ return new File( localRepositoryBasedir, ".locks" ).getCanonicalFile().toPath();
+ }
+ catch ( IOException e )
+ {
+ throw new UncheckedIOException( e );
+ }
+ }
+ );
+
TreeSet<String> paths = new TreeSet<>();
if ( artifacts != null )
{
for ( Artifact artifact : artifacts )
{
- paths.add( getPath( artifact ) + ".aetherlock" );
+ paths.add( getPath( baseDir, artifact ) + ".aetherlock" );
}
}
if ( metadatas != null )
{
for ( Metadata metadata : metadatas )
{
- paths.add( getPath( metadata ) + ".aetherlock" );
+ paths.add( getPath( baseDir, metadata ) + ".aetherlock" );
}
}
return paths;
}
- private String getPath( final Artifact artifact )
+ private String getPath( final Path baseDir, final Artifact artifact )
{
// NOTE: Don't use LRM.getPath*() as those paths could be different across processes, e.g. due to staging LRMs.
- return artifact.getGroupId() + SEPARATOR + artifact.getArtifactId() + SEPARATOR + artifact.getBaseVersion();
+ String path = artifact.getGroupId()
+ + SEPARATOR + artifact.getArtifactId()
+ + SEPARATOR + artifact.getBaseVersion();
+ return baseDir.resolve( path ).toAbsolutePath().toString();
}
- private String getPath( final Metadata metadata )
+ private String getPath( final Path baseDir, final Metadata metadata )
{
// NOTE: Don't use LRM.getPath*() as those paths could be different across processes, e.g. due to staging.
String path = "";
@@ -88,6 +121,6 @@ public class TakariNameMapper implements NameMapper
}
}
}
- return path;
+ return baseDir.resolve( path ).toAbsolutePath().toString();
}
}
\ No newline at end of file
diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/FileLockAdapterTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/FileLockAdapterTest.java
index ec35390..e0d157b 100644
--- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/FileLockAdapterTest.java
+++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/FileLockAdapterTest.java
@@ -19,9 +19,8 @@ package org.eclipse.aether.internal.impl.synccontext;
* under the License.
*/
-import org.eclipse.aether.internal.impl.synccontext.named.FileLockNamedLockFactory;
import org.eclipse.aether.internal.impl.synccontext.named.TakariNameMapper;
-import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory;
+import org.eclipse.aether.named.providers.FileLockNamedLockFactory;
import org.junit.BeforeClass;
public class FileLockAdapterTest
diff --git a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileLockNamedLockFactorySupport.java b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java
similarity index 78%
rename from maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileLockNamedLockFactorySupport.java
rename to maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java
index 2149e8a..2ba5a1a 100644
--- a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileLockNamedLockFactorySupport.java
+++ b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java
@@ -1,4 +1,4 @@
-package org.eclipse.aether.named.support;
+package org.eclipse.aether.named.providers;
/*
* Licensed to the Apache Software Foundation (ASF) under one
@@ -24,35 +24,43 @@ import java.io.UncheckedIOException;
import java.nio.channels.FileChannel;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.util.concurrent.ConcurrentHashMap;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.eclipse.aether.named.support.FileLockNamedLock;
+import org.eclipse.aether.named.support.FileSystemFriendly;
+import org.eclipse.aether.named.support.NamedLockFactorySupport;
+import org.eclipse.aether.named.support.NamedLockSupport;
+
/**
- * Named locks factory of {@link FileLockNamedLock}s. This is a bit special implementation, as it is abstract. This
- * class does not "know" how to resolve lock names to FS paths.
+ * Named locks factory of {@link FileLockNamedLock}s. This is a bit special implementation, as it expects locks names
+ * to be fully qualified absolute file system paths.
*
* @since TBD
*/
-public abstract class FileLockNamedLockFactorySupport
+@Singleton
+@Named( FileLockNamedLockFactory.NAME )
+public class FileLockNamedLockFactory
extends NamedLockFactorySupport
+ implements FileSystemFriendly
{
+ public static final String NAME = "file-lock";
+
private final ConcurrentHashMap<String, FileChannel> channels;
- public FileLockNamedLockFactorySupport()
+ public FileLockNamedLockFactory()
{
this.channels = new ConcurrentHashMap<>();
}
- /**
- * Implementations should be able to "resolve" lock name to {@link Path}, this method must <strong>never return
- * {@code null}!</strong>
- */
- protected abstract Path resolveName( final String name );
-
@Override
protected NamedLockSupport createLock( final String name )
{
- Path path = resolveName( name );
+ Path path = Paths.get( name );
FileChannel fileChannel = channels.computeIfAbsent( name, k ->
{
try
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SessionAwareNamedLockFactory.java b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileSystemFriendly.java
similarity index 50%
rename from maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SessionAwareNamedLockFactory.java
rename to maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileSystemFriendly.java
index 28d7961..29cac95 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/SessionAwareNamedLockFactory.java
+++ b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileSystemFriendly.java
@@ -1,4 +1,4 @@
-package org.eclipse.aether.internal.impl.synccontext.named;
+package org.eclipse.aether.named.support;
/*
* Licensed to the Apache Software Foundation (ASF) under one
@@ -19,22 +19,12 @@ package org.eclipse.aether.internal.impl.synccontext.named;
* under the License.
*/
-import org.eclipse.aether.RepositorySystemSession;
-import org.eclipse.aether.named.NamedLock;
-import org.eclipse.aether.named.NamedLockFactory;
-
/**
- * A {@link NamedLockFactory} that wants to make use of {@link RepositorySystemSession}.
+ * A marker interface that mark component "file system friendly". In case of lock factory, it would mean that
+ * passed in lock names MUST ADHERE to file path naming convention (and not use some special, non FS friendly
+ * characters in it). Essentially, component marked with this interface expects (or uses) that "name" is an absolute
+ * and valid file path.
*/
-public interface SessionAwareNamedLockFactory extends NamedLockFactory
+public interface FileSystemFriendly
{
- /**
- * Creates or reuses existing {@link NamedLock}. Returns instance MUST BE treated as "resource", best in
- * try-with-resource block.
- *
- * @param session the repository system session, must not be {@code null}
- * @param name the lock name, must not be {@code null}
- * @return named the lock instance, never {@code null}
- */
- NamedLock getLock( RepositorySystemSession session, String name );
}
diff --git a/maven-resolver-named-locks/src/test/java/org/eclipse/aether/named/FileLockNamedLockFactorySupportTest.java b/maven-resolver-named-locks/src/test/java/org/eclipse/aether/named/FileLockNamedLockFactorySupportTest.java
index 15fcb26..799d823 100644
--- a/maven-resolver-named-locks/src/test/java/org/eclipse/aether/named/FileLockNamedLockFactorySupportTest.java
+++ b/maven-resolver-named-locks/src/test/java/org/eclipse/aether/named/FileLockNamedLockFactorySupportTest.java
@@ -24,23 +24,29 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
-import org.eclipse.aether.named.support.FileLockNamedLockFactorySupport;
+import org.eclipse.aether.named.providers.FileLockNamedLockFactory;
import org.junit.BeforeClass;
public class FileLockNamedLockFactorySupportTest
extends NamedLockFactoryTestSupport {
- @BeforeClass
- public static void createNamedLockFactory() throws IOException {
+ private final Path baseDir;
+
+ public FileLockNamedLockFactorySupportTest() throws IOException
+ {
String path = System.getProperty( "java.io.tmpdir" );
Files.createDirectories(Paths.get(path) ); // hack for surefire: sets the property but directory does not exist
+ this.baseDir = Files.createTempDirectory( null );
+ }
- Path baseDir = Files.createTempDirectory( null );
- namedLockFactory = new FileLockNamedLockFactorySupport() {
- @Override
- protected Path resolveName(final String name) {
- return baseDir.resolve( name );
- }
- };
+ @Override
+ protected String lockName()
+ {
+ return baseDir.resolve(testName.getMethodName()).toAbsolutePath().toString();
+ }
+
+ @BeforeClass
+ public static void createNamedLockFactory() throws IOException {
+ namedLockFactory = new FileLockNamedLockFactory();
}
}