michael-o commented on a change in pull request #77: URL: https://github.com/apache/maven-resolver/pull/77#discussion_r611219268
########## File path: maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/NamedLock.java ########## @@ -0,0 +1,71 @@ +package org.eclipse.aether.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 java.util.concurrent.TimeUnit; + +/** + * A named lock, functionally similar to existing JVM and other implementations. Must support boxing (reentrancy), but + * no lock upgrade is supported. Usual pattern to use this lock: + * <pre> + * try (NamedLock lock = factory.getLock("resourceName")) { + * if (lock.lockExclusively(10L, Timeunit.SECONDS)) { + * try { + * ... exclusive access to "resourceName" resource gained here + * } + * finally { + * lock.unlock(); + * } + * } + * else { + * ... failed to gain access within specified time, handle it + * } + * } + * </pre> + */ +public interface NamedLock + extends AutoCloseable +{ + /** + * Returns this instance name, never null. + */ + String name(); + + /** + * Tries to lock shared, may block for given time. If successful, returns {@code true}. + */ + boolean lockShared( long time, TimeUnit unit ) throws InterruptedException; + + /** + * Tries to lock exclusively, may block for given time. If successful, returns {@code true}. + */ + boolean lockExclusively( long time, TimeUnit unit ) throws InterruptedException; + + /** + * Unlocks the lock. + */ + void unlock(); + + /** + * Closes the lock. + */ + @Override + void close(); Review comment: One should describe whether there is a difference between `close()` and `unlock()`. ########## File path: maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/NamedLock.java ########## @@ -0,0 +1,71 @@ +package org.eclipse.aether.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 java.util.concurrent.TimeUnit; + +/** + * A named lock, functionally similar to existing JVM and other implementations. Must support boxing (reentrancy), but + * no lock upgrade is supported. Usual pattern to use this lock: + * <pre> + * try (NamedLock lock = factory.getLock("resourceName")) { + * if (lock.lockExclusively(10L, Timeunit.SECONDS)) { + * try { + * ... exclusive access to "resourceName" resource gained here + * } + * finally { + * lock.unlock(); + * } + * } + * else { + * ... failed to gain access within specified time, handle it + * } + * } + * </pre> + */ +public interface NamedLock + extends AutoCloseable +{ + /** + * Returns this instance name, never null. + */ + String name(); + + /** + * Tries to lock shared, may block for given time. If successful, returns {@code true}. + */ + boolean lockShared( long time, TimeUnit unit ) throws InterruptedException; + + /** + * Tries to lock exclusively, may block for given time. If successful, returns {@code true}. + */ + boolean lockExclusively( long time, TimeUnit unit ) throws InterruptedException; Review comment: I think that those two method should resemble Java's `Lock`: `tryLock...(long, TimeUnit)`. ########## File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NameMapper.java ########## @@ -0,0 +1,41 @@ +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 java.util.Collection; + +import org.eclipse.aether.RepositorySystemSession; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.metadata.Metadata; + +/** + * Component mapping lock names to passed in artifacts and metadata as required. + */ +public interface NameMapper +{ + /** + * Creates (opaque) names for passed in artifacts and metadata. Returned collection has max size of sum of the passed + * in artifacts and metadata collections, or less. If empty collection returned, there will be no locking happening. + * Never returns {@code null}. + */ + Collection<String> nameLocks( RepositorySystemSession session, Review comment: I wonder whether we should require the collection to be stable regardless of the input order to comply with this comment: https://github.com/apache/maven-resolver/blob/f27b51de959f316e37d081fd4d2e572e778d6813/maven-resolver-synccontext-redisson/src/main/java/org/eclipse/aether/synccontext/RedissonSyncContextFactory.java#L246-L247 WDYT? ########## File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/NamedLockFactoryAdapter.java ########## @@ -0,0 +1,189 @@ +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.SyncContext; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.metadata.Metadata; +import org.eclipse.aether.named.NamedLock; +import org.eclipse.aether.named.NamedLockFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayDeque; +import java.util.Collection; +import java.util.Objects; +import java.util.concurrent.TimeUnit; + +/** + * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link SyncContext}. + */ +public final class NamedLockFactoryAdapter +{ + private final NameMapper nameMapper; + + private final NamedLockFactory namedLockFactory; + + private final long time; + + private final TimeUnit timeUnit; + + public NamedLockFactoryAdapter( final NameMapper nameMapper, + final NamedLockFactory namedLockFactory, + final long time, + final TimeUnit timeUnit ) + { + this.nameMapper = Objects.requireNonNull( nameMapper ); + this.namedLockFactory = Objects.requireNonNull( namedLockFactory ); + if ( time < 0L ) + { + throw new IllegalArgumentException( "time cannot be negative" ); + } + this.time = time; + this.timeUnit = Objects.requireNonNull( timeUnit ); + } + + public SyncContext newInstance( final RepositorySystemSession session, final boolean shared ) + { + return new AdaptedLockSyncContext( + session, + shared, + nameMapper, + namedLockFactory, + time, + timeUnit ); + } + + public void shutdown() + { + namedLockFactory.shutdown(); + } + + private static class AdaptedLockSyncContext + implements SyncContext + { + private static final Logger LOGGER = LoggerFactory.getLogger( AdaptedLockSyncContext.class ); + + private final RepositorySystemSession session; + + private final boolean shared; + + private final NameMapper lockNaming; + + private final SessionAwareNamedLockFactory sessionAwareNamedLockFactory; + + private final NamedLockFactory namedLockFactory; + + private final long timeOut; + + private final TimeUnit timeUnit; + + private final ArrayDeque<NamedLock> locks; + + private AdaptedLockSyncContext( final RepositorySystemSession session, + final boolean shared, + final NameMapper lockNaming, + final NamedLockFactory namedLockFactory, + final long timeOut, + final TimeUnit timeUnit ) + { + this.session = session; + this.shared = shared; + this.lockNaming = lockNaming; + this.sessionAwareNamedLockFactory = namedLockFactory instanceof SessionAwareNamedLockFactory + ? (SessionAwareNamedLockFactory) namedLockFactory + : null; + this.namedLockFactory = namedLockFactory; + this.timeOut = timeOut; + this.timeUnit = timeUnit; + this.locks = new ArrayDeque<>(); + } + + @Override + public void acquire( Collection<? extends Artifact> artifacts, + Collection<? extends Metadata> metadatas ) + { + Collection<String> keys = lockNaming.nameLocks( session, artifacts, metadatas ); + if ( keys.isEmpty() ) + { + return; + } + + LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ? "read" : "write", keys ); + int acquiredLockCount = 0; + for ( String key : keys ) + { + NamedLock namedLock = sessionAwareNamedLockFactory != null + ? sessionAwareNamedLockFactory.getLock( session, key ) + : namedLockFactory.getLock( key ); + try + { + boolean locked; + if ( shared ) + { + locked = namedLock.lockShared( timeOut, timeUnit ); + } + else + { + locked = namedLock.lockExclusively( timeOut, timeUnit ); + } + + if ( !locked ) + { + namedLock.close(); + throw new IllegalStateException( "Could not lock " + + namedLock.name() + " (shared=" + shared + ")" ); + } Review comment: I wonder whether this behavior should be configurable? WDYT? ########## File path: maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java ########## @@ -0,0 +1,112 @@ +package org.eclipse.aether.named.support; + +/* + * 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.ArrayDeque; +import java.util.Deque; +import java.util.concurrent.TimeUnit; + +/** + * Named lock support implementation that is using "adapted" semaphore (to be able to use semaphores not sharing common + * API). + */ +public class AdaptedSemaphoreNamedLock + extends NamedLockSupport +{ + /** + * Wrapper for semaphore-like stuff, that do not share common ancestor. Semaphore must be created to support + * {@link Integer#MAX_VALUE} permissions. + */ + public interface AdaptedSemaphore + { + boolean tryAcquire( int perms, long timeout, TimeUnit unit ) throws InterruptedException; + + void release( int perms ); + } + + private final ThreadLocal<Deque<Integer>> threadPerms; + + private final AdaptedSemaphore semaphore; + + public AdaptedSemaphoreNamedLock( final String name, + final NamedLockFactorySupport factory, Review comment: Here too, generic type. ########## File path: maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java ########## @@ -0,0 +1,112 @@ +package org.eclipse.aether.named.support; + +/* + * 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.ArrayDeque; +import java.util.Deque; +import java.util.concurrent.TimeUnit; + +/** + * Named lock support implementation that is using "adapted" semaphore (to be able to use semaphores not sharing common + * API). + */ +public class AdaptedSemaphoreNamedLock Review comment: I think for the ease of readability you should assign static finals to 0, 1, and `Integer.MAX_VALUE` just like `org.eclipse.aether.named.support.AdaptedReadWriteLockNamedLock.Step` or have them int values and reuse them. ########## File path: maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java ########## @@ -0,0 +1,87 @@ +package org.eclipse.aether.named.support; + +/* + * 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.concurrent.atomic.AtomicInteger; + +import org.eclipse.aether.named.NamedLock; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Support class for {@link NamedLock} implementations providing reference counting. + */ +public abstract class NamedLockSupport + implements NamedLock +{ + protected final Logger log = LoggerFactory.getLogger( getClass() ); + + private final String name; + + private final NamedLockFactorySupport factory; + + private final AtomicInteger refCount; + + public NamedLockSupport( final String name, final NamedLockFactorySupport factory ) Review comment: Same here. ########## File path: maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java ########## @@ -0,0 +1,87 @@ +package org.eclipse.aether.named.support; + +/* + * 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.concurrent.atomic.AtomicInteger; + +import org.eclipse.aether.named.NamedLock; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Support class for {@link NamedLock} implementations providing reference counting. + */ +public abstract class NamedLockSupport + implements NamedLock +{ + protected final Logger log = LoggerFactory.getLogger( getClass() ); + + private final String name; + + private final NamedLockFactorySupport factory; Review comment: Eclipse complains about raw type. `private final NamedLockFactorySupport<NamedLockSupport> factory;` fixes this for me. ########## File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java ########## @@ -0,0 +1,128 @@ +package org.eclipse.aether.internal.impl.synccontext; + +/* + * 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.SyncContext; +import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper; +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.StaticNameMapper; +import org.eclipse.aether.internal.impl.synccontext.named.takari.TakariNamedLockFactory; +import org.eclipse.aether.internal.impl.synccontext.named.takari.TakariNameMapper; +import org.eclipse.aether.named.NamedLockFactory; +import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory; +import org.eclipse.aether.named.providers.LocalSemaphoreNamedLockFactory; + +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.TimeUnit; + +/** + * Named {@link SyncContextFactoryDelegate} implementation that selects underlying {@link NamedLockFactory} + * implementation at creation. + */ +@Singleton +@Named( NamedSyncContextFactory.NAME ) +public final class NamedSyncContextFactory + implements SyncContextFactoryDelegate +{ + public static final String NAME = "named"; + + private static final String FACTORY_NAME = System.getProperty( + "aether.syncContext.named.factory", LocalReadWriteLockNamedLockFactory.NAME + ); + + private static final String NAME_MAPPING = System.getProperty( + "aether.syncContext.named.nameMapping", DiscriminatingNameMapper.NAME Review comment: I think this shoud be `mameMapper` since the interface is called that way. ########## File path: maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupport.java ########## @@ -0,0 +1,120 @@ +package org.eclipse.aether.named.redisson; + +/* + * 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.support.NamedLockFactorySupport; +import org.eclipse.aether.named.support.NamedLockSupport; +import org.redisson.Redisson; +import org.redisson.api.RedissonClient; +import org.redisson.config.Config; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +/** + * Support class for factories using {@link RedissonClient}. + * + * @param <L> the actual implementation, subclass of {@link NamedLockSupport}. + */ +public abstract class RedissonNamedLockFactorySupport<L extends NamedLockSupport> + extends NamedLockFactorySupport<L> +{ + protected static final String NAME_PREFIX = "maven:resolver:"; + + private static final String DEFAULT_CONFIG_FILE_NAME = "maven-resolver-redisson.yaml"; + + private static final String DEFAULT_REDIS_ADDRESS = "redis://localhost:6379"; + + private static final String DEFAULT_CLIENT_NAME = "maven-resolver"; + + private static final String CONFIG_PROP_CONFIG_FILE = "aether.syncContext.redisson.configFile"; Review comment: Since this is now part of the named lock, the propfile should be `aether.syncContext.named.redisson.configFile`. Makes sense? ########## File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java ########## @@ -0,0 +1,128 @@ +package org.eclipse.aether.internal.impl.synccontext; + +/* + * 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.SyncContext; +import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper; +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.StaticNameMapper; +import org.eclipse.aether.internal.impl.synccontext.named.takari.TakariNamedLockFactory; +import org.eclipse.aether.internal.impl.synccontext.named.takari.TakariNameMapper; +import org.eclipse.aether.named.NamedLockFactory; +import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory; +import org.eclipse.aether.named.providers.LocalSemaphoreNamedLockFactory; + +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.TimeUnit; + +/** + * Named {@link SyncContextFactoryDelegate} implementation that selects underlying {@link NamedLockFactory} + * implementation at creation. + */ +@Singleton +@Named( NamedSyncContextFactory.NAME ) +public final class NamedSyncContextFactory + implements SyncContextFactoryDelegate +{ + public static final String NAME = "named"; + + private static final String FACTORY_NAME = System.getProperty( + "aether.syncContext.named.factory", LocalReadWriteLockNamedLockFactory.NAME + ); + + private static final String NAME_MAPPING = System.getProperty( + "aether.syncContext.named.nameMapping", DiscriminatingNameMapper.NAME Review comment: I think since `LocalReadWriteLockNamedLockFactory` is the default, then the default mapper should be `GAVNameMapper` because it is local and not distributed (discriminating). It provides no overhead to name calculation while the distributed ones does. One can also use GAV with Redisson/HC if only one process accesses the local repo. ########## File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java ########## @@ -0,0 +1,128 @@ +package org.eclipse.aether.internal.impl.synccontext; + +/* + * 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.SyncContext; +import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper; +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.StaticNameMapper; +import org.eclipse.aether.internal.impl.synccontext.named.takari.TakariNamedLockFactory; +import org.eclipse.aether.internal.impl.synccontext.named.takari.TakariNameMapper; +import org.eclipse.aether.named.NamedLockFactory; +import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory; +import org.eclipse.aether.named.providers.LocalSemaphoreNamedLockFactory; + +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.TimeUnit; + +/** + * Named {@link SyncContextFactoryDelegate} implementation that selects underlying {@link NamedLockFactory} + * implementation at creation. + */ +@Singleton +@Named( NamedSyncContextFactory.NAME ) +public final class NamedSyncContextFactory + implements SyncContextFactoryDelegate +{ + public static final String NAME = "named"; + + private static final String FACTORY_NAME = System.getProperty( + "aether.syncContext.named.factory", LocalReadWriteLockNamedLockFactory.NAME + ); + + private static final String NAME_MAPPING = System.getProperty( + "aether.syncContext.named.nameMapping", DiscriminatingNameMapper.NAME + ); + + private static final long TIME_OUT = Long.getLong( Review comment: Should be `TIMEOUT` ########## File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/DiscriminatingNameMapper.java ########## @@ -0,0 +1,141 @@ +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 java.io.File; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.charset.StandardCharsets; +import java.util.Collection; +import java.util.Collections; +import java.util.Map; +import java.util.Objects; + +import javax.inject.Inject; +import javax.inject.Named; +import javax.inject.Singleton; + +import org.eclipse.aether.RepositorySystemSession; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.metadata.Metadata; +import org.eclipse.aether.util.ChecksumUtils; +import org.eclipse.aether.util.ConfigUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static java.util.stream.Collectors.toList; + +/** + * Discriminating {@link NameMapper}, that wraps another {@link NameMapper} and adds "discriminator" as prefix, + * that makes lock names unique including local repository (by default). The discriminator may be passed in via + * {@link RepositorySystemSession} or is automatically calculated based on local repository path. + * + * The default setup wraps {@link GAVNameMapper}, but manually may be created any instance needed. + */ +@Singleton +@Named( DiscriminatingNameMapper.NAME ) +public class DiscriminatingNameMapper Review comment: This is damn good refactoring! ########## File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java ########## @@ -0,0 +1,128 @@ +package org.eclipse.aether.internal.impl.synccontext; + +/* + * 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.SyncContext; +import org.eclipse.aether.internal.impl.synccontext.named.DiscriminatingNameMapper; +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.StaticNameMapper; +import org.eclipse.aether.internal.impl.synccontext.named.takari.TakariNamedLockFactory; +import org.eclipse.aether.internal.impl.synccontext.named.takari.TakariNameMapper; +import org.eclipse.aether.named.NamedLockFactory; +import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory; +import org.eclipse.aether.named.providers.LocalSemaphoreNamedLockFactory; + +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.TimeUnit; + +/** + * Named {@link SyncContextFactoryDelegate} implementation that selects underlying {@link NamedLockFactory} + * implementation at creation. + */ +@Singleton +@Named( NamedSyncContextFactory.NAME ) +public final class NamedSyncContextFactory + implements SyncContextFactoryDelegate +{ + public static final String NAME = "named"; + + private static final String FACTORY_NAME = System.getProperty( + "aether.syncContext.named.factory", LocalReadWriteLockNamedLockFactory.NAME + ); + + private static final String NAME_MAPPING = System.getProperty( + "aether.syncContext.named.nameMapping", DiscriminatingNameMapper.NAME + ); + + private static final long TIME_OUT = Long.getLong( + "aether.syncContext.named.timeout", 30L + ); + + private static final TimeUnit TIME_UNIT = TimeUnit.valueOf( System.getProperty( Review comment: Should be `TIMEOUT_UNIT` ########## File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/DiscriminatingNameMapper.java ########## @@ -0,0 +1,141 @@ +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 java.io.File; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.charset.StandardCharsets; +import java.util.Collection; +import java.util.Collections; +import java.util.Map; +import java.util.Objects; + +import javax.inject.Inject; +import javax.inject.Named; +import javax.inject.Singleton; + +import org.eclipse.aether.RepositorySystemSession; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.metadata.Metadata; +import org.eclipse.aether.util.ChecksumUtils; +import org.eclipse.aether.util.ConfigUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static java.util.stream.Collectors.toList; + +/** + * Discriminating {@link NameMapper}, that wraps another {@link NameMapper} and adds "discriminator" as prefix, + * that makes lock names unique including local repository (by default). The discriminator may be passed in via + * {@link RepositorySystemSession} or is automatically calculated based on local repository path. Review comment: I think the key point should be mentioned here: this mapper can be used with distributed locks with the default config. ########## File path: maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java ########## @@ -0,0 +1,87 @@ +package org.eclipse.aether.named.support; + +/* + * 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.concurrent.atomic.AtomicInteger; + +import org.eclipse.aether.named.NamedLock; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Support class for {@link NamedLock} implementations providing reference counting. + */ +public abstract class NamedLockSupport + implements NamedLock +{ + protected final Logger log = LoggerFactory.getLogger( getClass() ); + + private final String name; + + private final NamedLockFactorySupport factory; + + private final AtomicInteger refCount; + + public NamedLockSupport( final String name, final NamedLockFactorySupport factory ) + { + this.name = name; + this.factory = factory; + this.refCount = new AtomicInteger( 0 ); + } + + public int incRef() + { + return refCount.incrementAndGet(); + } + + public int decRef() + { + return refCount.decrementAndGet(); + } Review comment: Can you explain why these two methods exist? Shouldn't the lock itself manage its reference count internally? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org