Merge pull request #2248 from AxonFramework/bug/2242

[#2242] Correctly support null-identifier and no-event scenarios from Command Handling constructors, `Always`, and `Create-If-Missing` creation policies

messaging/src/main/java/org/axonframework/common/lock/NoOpLock.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2010-2022. Axon Framework
+ *
+ * Licensed 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.axonframework.common.lock;
+
+/**
+ * No-op implementation of a {@link Lock}. Does nothing on {@link #release()} and returns {@code true} for
+ * {@link #isHeld()}.
+ *
+ * @author Steven van Beelen
+ * @since 4.5.11
+ */
+public class NoOpLock implements Lock {
+
+    public static final Lock INSTANCE = new NoOpLock();
+
+    private NoOpLock() {
+        // Retrieve version through static INSTANCE.
+    }
+
+    @Override
+    public void release() {
+        // Not implemented for this no-op version.
+    }
+
+    @Override
+    public boolean isHeld() {
+        return true;
+    }
+}

messaging/src/main/java/org/axonframework/common/lock/NullLockFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2018. Axon Framework
+ * Copyright (c) 2010-2022. Axon Framework
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,7 +17,7 @@
 package org.axonframework.common.lock;
 
 /**
- * LockFactory implementation that does nothing. Can be useful in cases where a Locking Repository implementation  needs
+ * LockFactory implementation that does nothing. Can be useful in cases where a Locking Repository implementation needs
  * to be configured to ignore locks, for example in scenario's where an underlying storage mechanism already performs
  * the necessary locking.
  *
@@ -38,18 +38,6 @@ public enum NullLockFactory implements LockFactory {
      */
     @Override
     public Lock obtainLock(String identifier) {
-        return NO_LOCK;
+        return NoOpLock.INSTANCE;
     }
-
-    private static final Lock NO_LOCK = new Lock() {
-        @Override
-        public void release() {
-        }
-
-        @Override
-        public boolean isHeld() {
-            return true;
-        }
-    };
-
 }

messaging/src/main/java/org/axonframework/common/lock/PessimisticLockFactory.java

@@ -1,11 +1,11 @@
 /*
- * Copyright (c) 2010-2018. Axon Framework
+ * Copyright (c) 2010-2022. Axon Framework
  *
  * Licensed 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
+ *    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,
@@ -30,6 +30,7 @@
 
 import static java.util.Collections.newSetFromMap;
 import static java.util.Collections.synchronizedSet;
+import static org.axonframework.common.Assert.nonNull;
 
 /**
  * Implementation of a {@link LockFactory} that uses a pessimistic locking strategy. Calls to
@@ -108,19 +109,19 @@ protected PessimisticLockFactory(Builder builder) {
     }
 
     /**
-     * Obtain a lock for a resource identified by the given {@code identifier}. This method will block until a
-     * lock was successfully obtained.
+     * Obtain a lock for a resource identified by the given {@code identifier}. This method will block until a lock was
+     * successfully obtained.
      * <p/>
      * Note: when an exception occurs during the locking process, the lock may or may not have been allocated.
      *
      * @param identifier the identifier of the lock to obtain.
-     * @return a handle to release the lock. If the thread that releases the lock does not hold the lock
-     * {@link IllegalMonitorStateException} is thrown
-     * {@link IllegalArgumentException} is thrown when identifier is null
+     * @return A handle to release the lock. If the thread that releases the lock does not hold the lock a
+     * {@link IllegalMonitorStateException} is thrown.
+     * @throws IllegalArgumentException Thrown when the given {@code identifier} is {@code null}.
      */
     @Override
     public Lock obtainLock(String identifier) {
-        Assert.nonNull(identifier, () -> "Aggregate identifier may not be null");
+        nonNull(identifier, () -> "The identifier to obtain a lock for may not be null.");
         boolean lockObtained = false;
         DisposableLock lock = null;
         while (!lockObtained) {

messaging/src/test/java/org/axonframework/common/lock/NoOpLockTest.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2010-2022. Axon Framework
+ *
+ * Licensed 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.axonframework.common.lock;
+
+import org.junit.jupiter.api.*;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+/**
+ * Test class validating the {@link NoOpLock}.
+ *
+ * @author Steven van Beelen
+ */
+class NoOpLockTest {
+
+    @Test
+    void testIsHeldReturnsTrue() {
+        assertTrue(NoOpLock.INSTANCE.isHeld());
+    }
+}

messaging/src/test/java/org/axonframework/common/lock/NullLockFactoryTest.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2010-2022. Axon Framework
+ *
+ * Licensed 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.axonframework.common.lock;
+
+import org.junit.jupiter.api.*;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+/**
+ * Test class validating the {@link NullLockFactory}.
+ *
+ * @author Steven van Beelen
+ */
+class NullLockFactoryTest {
+
+    @Test
+    void testObtainLockReturnsNoOpLockInstance() {
+        LockFactory testSubject = NullLockFactory.INSTANCE;
+
+        assertEquals(NoOpLock.INSTANCE, testSubject.obtainLock(null));
+    }
+}

messaging/src/test/java/org/axonframework/common/lock/PessimisticLockFactoryTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2020. Axon Framework
+ * Copyright (c) 2010-2022. Axon Framework
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,20 +16,19 @@
 
 package org.axonframework.common.lock;
 
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.*;
 
 import java.lang.reflect.Field;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 /**
+ * Test class validating the {@link PessimisticLockFactory}.
+ *
  * @author Allard Buijze
  */
 class PessimisticLockFactoryTest {
@@ -219,6 +218,7 @@ void testShouldThrowIllegalArgumentExceptionWhenIdentifierIsNull() {
         this.identifier = null;
         PessimisticLockFactory manager = PessimisticLockFactory.builder().build();
 
+        //noinspection resource
         assertThrows(IllegalArgumentException.class, () -> manager.obtainLock(identifier));
     }
 

modelling/src/main/java/org/axonframework/modelling/command/AbstractRepository.java

@@ -89,8 +89,10 @@ public A newInstance(Callable<T> factoryMethod, Consumer<Aggregate<T>> initMetho
         // a constructor may apply events, and the persistence of an aggregate must take precedence over publishing its events.
         uow.onPrepareCommit(x -> {
             A aggregate = aggregateReference.get();
-            // aggregate construction may have failed with an exception. In that case, no action is required on commit
-            if (aggregate != null) {
+            // Aggregate construction may have failed with an exception (aggregate == null)
+            //  or the handler decided not to create the aggregate (identifier == null).
+            // In that case, no action is required on commit.
+            if (aggregate != null && aggregate.identifier() != null) {
                 prepareForCommit(aggregate);
             }
         });

modelling/src/main/java/org/axonframework/modelling/command/LockingRepository.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2021. Axon Framework
+ * Copyright (c) 2010-2022. Axon Framework
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@
 import org.axonframework.common.Assert;
 import org.axonframework.common.lock.Lock;
 import org.axonframework.common.lock.LockFactory;
+import org.axonframework.common.lock.NoOpLock;
 import org.axonframework.common.lock.PessimisticLockFactory;
 import org.axonframework.messaging.annotation.HandlerDefinition;
 import org.axonframework.messaging.annotation.ParameterResolverFactory;
@@ -94,7 +95,9 @@ protected LockAwareAggregate<T, A> doCreateNew(Callable<T> factoryMethod) throws
         } else {
             // The aggregate identifier hasn't been set yet, so the lock should be created in the supplier.
             lockSupplier = sameInstanceSupplier(() -> {
-                Lock lock = lockFactory.obtainLock(aggregate.identifierAsString());
+                Lock lock = Objects.isNull(aggregate.identifierAsString())
+                        ? NoOpLock.INSTANCE
+                        : lockFactory.obtainLock(aggregate.identifierAsString());
                 unitOfWork.onCleanup(u -> lock.release());
                 return lock;
             });