[MRESOLVER-220] Refuse proactively unsupported operation (#301)

Instead to silently "send result" (that is false/code expect true=success) to caller, as this makes totally strange outcome: "failed to lock within 30sec" but caller did not wait anything. Moreover, the bug where upgrade is attempted slips in unnoticed.

In short: it was totally wrong that lock implementation immediately returns "false", hiding the fact that user attempted non supported operation.

---

https://issues.apache.org/jira/browse/MRESOLVER-220

maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/NamedLockFactoryAdapterTestSupport.java

@@ -31,6 +31,7 @@
 import org.eclipse.aether.artifact.DefaultArtifact;
 import org.eclipse.aether.internal.impl.synccontext.named.*;
 import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.support.LockUpgradeNotSupportedException;
 import org.eclipse.aether.repository.LocalRepository;
 import org.eclipse.aether.spi.synccontext.SyncContextFactory;
 import org.junit.AfterClass;
@@ -266,8 +267,7 @@ public void run() {
                         chained.run();
                     }
                     loser.await();
-                } catch (IllegalStateException e) {
-                    e.printStackTrace(); // for ref purposes
+                } catch (IllegalStateException | LockUpgradeNotSupportedException e) {
                     loser.countDown();
                     winner.await();
                 }

maven-resolver-named-locks-hazelcast/src/test/java/org/eclipse/aether/named/hazelcast/NamedLockFactoryAdapterTestSupport.java

@@ -33,6 +33,7 @@
 import org.eclipse.aether.internal.impl.synccontext.named.GAVNameMapper;
 import org.eclipse.aether.internal.impl.synccontext.named.NamedLockFactoryAdapter;
 import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.support.LockUpgradeNotSupportedException;
 import org.eclipse.aether.repository.LocalRepository;
 import org.eclipse.aether.spi.synccontext.SyncContextFactory;
 import org.junit.AfterClass;
@@ -230,7 +231,7 @@ public void run() {
                         chained.run();
                     }
                     loser.await();
-                } catch (IllegalStateException e) {
+                } catch (IllegalStateException | LockUpgradeNotSupportedException e) {
                     loser.countDown();
                     winner.await();
                 }

maven-resolver-named-locks-hazelcast/src/test/java/org/eclipse/aether/named/hazelcast/NamedLockFactoryTestSupport.java

@@ -23,6 +23,7 @@
 
 import org.eclipse.aether.named.NamedLock;
 import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.support.LockUpgradeNotSupportedException;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.Rule;
@@ -114,7 +115,11 @@ public void rwBoxing() throws InterruptedException {
         final String name = testName.getMethodName();
         try (NamedLock one = namedLockFactory.getLock(name)) {
             assertThat(one.lockShared(1L, TimeUnit.MILLISECONDS), is(true));
-            assertThat(one.lockExclusively(1L, TimeUnit.MILLISECONDS), is(false));
+            try {
+                one.lockExclusively(1L, TimeUnit.MILLISECONDS);
+            } catch (LockUpgradeNotSupportedException e) {
+                // good
+            }
             one.unlock();
         }
     }

maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/AdaptedSemaphoreNamedLock.java

@@ -87,7 +87,7 @@ protected boolean doLockExclusively(final long time, final TimeUnit unit) throws
                 perms.push(NONE);
                 return true;
             } else {
-                return false; // Lock upgrade not supported
+                throw new LockUpgradeNotSupportedException(this); // Lock upgrade not supported
             }
         }
         if (semaphore.tryAcquire(EXCLUSIVE, time, unit)) {

maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/FileLockNamedLock.java

@@ -137,7 +137,7 @@ private Boolean doLockExclusively() {
                         // if we own shared, that's attempted upgrade
                         boolean weOwnShared = steps.contains(Boolean.TRUE);
                         if (weOwnShared) {
-                            return false; // Lock upgrade not supported
+                            throw new LockUpgradeNotSupportedException(this); // Lock upgrade not supported
                         } else {
                             // someone else owns shared, let's wait
                             return null;

maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/LockUpgradeNotSupportedException.java

@@ -0,0 +1,36 @@
+/*
+ * 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.
+ */
+package org.eclipse.aether.named.support;
+
+/**
+ * Exception thrown when lock upgrade attempted that we do not support. This exception when used within {@link Retry}
+ * helper should never be reattempted, hence is marked with {@link Retry.DoNotRetry} marker.
+ *
+ * @since 1.9.13
+ */
+public final class LockUpgradeNotSupportedException extends RuntimeException implements Retry.DoNotRetry {
+    /**
+     * Constructor for case, when current thread attempts lock upgrade on given lock instance.
+     */
+    public LockUpgradeNotSupportedException(NamedLockSupport namedLock) {
+        super("Thread " + Thread.currentThread().getName()
+                + " already holds shared lock for '" + namedLock.name()
+                + "', but asks for exclusive lock; lock upgrade not supported");
+    }
+}

maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/ReadWriteLockNamedLock.java

@@ -67,7 +67,7 @@ protected boolean doLockExclusively(final long time, final TimeUnit unit) throws
         Deque<Step> steps = threadSteps.get();
         if (!steps.isEmpty()) { // we already own shared or exclusive lock
             if (!steps.contains(Step.EXCLUSIVE)) {
-                return false; // Lock upgrade not supported
+                throw new LockUpgradeNotSupportedException(this); // Lock upgrade not supported
             }
         }
         if (readWriteLock.writeLock().tryLock(time, unit)) {

maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/Retry.java

@@ -37,6 +37,14 @@
 public final class Retry {
     private static final Logger LOGGER = LoggerFactory.getLogger(Retry.class);
 
+    /**
+     * Marker interface to apply onto exceptions to make them "never retried" when thrown. This shortcuts checks with
+     * predicate, if used.
+     *
+     * @since 1.9.13
+     */
+    public interface DoNotRetry {}
+
     private Retry() {
         // no instances
     }
@@ -71,6 +79,13 @@ public static <R> R retry(
                 throw e;
             } catch (Exception e) {
                 LOGGER.trace("Retry attempt {}: operation failure", attempt, e);
+                if (e instanceof DoNotRetry) {
+                    if (e instanceof RuntimeException) {
+                        throw (RuntimeException) e;
+                    } else {
+                        throw new IllegalStateException(e);
+                    }
+                }
                 if (retryPredicate != null && !retryPredicate.test(e)) {
                     throw new IllegalStateException(e);
                 }
@@ -111,6 +126,13 @@ public static <R> R retry(
                 throw e;
             } catch (Exception e) {
                 LOGGER.trace("Retry attempt {}: operation failure", attempt, e);
+                if (e instanceof DoNotRetry) {
+                    if (e instanceof RuntimeException) {
+                        throw (RuntimeException) e;
+                    } else {
+                        throw new IllegalStateException(e);
+                    }
+                }
                 if (retryPredicate != null && !retryPredicate.test(e)) {
                     throw new IllegalStateException(e);
                 }

maven-resolver-named-locks/src/test/java/org/eclipse/aether/named/NamedLockFactoryTestSupport.java

@@ -21,6 +21,7 @@
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
+import org.eclipse.aether.named.support.LockUpgradeNotSupportedException;
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
@@ -132,7 +133,11 @@ public void rwBoxing() throws InterruptedException {
         final String name = lockName();
         try (NamedLock one = namedLockFactory.getLock(name)) {
             assertThat(one.lockShared(1L, TimeUnit.MILLISECONDS), is(true));
-            assertThat(one.lockExclusively(1L, TimeUnit.MILLISECONDS), is(false));
+            try {
+                one.lockExclusively(1L, TimeUnit.MILLISECONDS);
+            } catch (LockUpgradeNotSupportedException e) {
+                // good
+            }
             one.unlock();
         }
     }