feat: check cyclic wrapper chain 🔄💣

foldright · Apr 5, 2024 · 422a959 · 422a959
1 parent 38242d5
commit 422a959
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 14 deletions.
diff --git a/src/main/java/io/foldright/inspectablewrappers/Inspector.java b/src/main/java/io/foldright/inspectablewrappers/Inspector.java
@@ -4,6 +4,7 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.Nullable;
 
+import java.util.IdentityHashMap;
 import java.util.Optional;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -66,6 +67,7 @@ public final class Inspector {
      *                               or any wrapper {@link Wrapper#unwrap()} returns null,
      *                               or the adaptee of {@link WrapperAdapter} is null
      * @throws IllegalStateException if the adaptee of {@link WrapperAdapter} is an instance of {@link Wrapper}
+     *                               or CYCLIC wrapper chain
      * @see Wrapper#unwrap()
      * @see WrapperAdapter#adaptee()
      */
@@ -100,6 +102,7 @@ private static boolean isInstanceOf(final Object o, final Class<?> clazz) {
      *                               or the adaptee of {@link WrapperAdapter} is null
      * @throws ClassCastException    if the return value is not type {@code <V>}
      * @throws IllegalStateException if the adaptee of {@link WrapperAdapter} is an instance of {@link Wrapper}
+     *                               or CYCLIC wrapper chain
      * @see Attachable#getAttachment(Object)
      * @see Wrapper#unwrap()
      * @see WrapperAdapter#adaptee()
@@ -134,6 +137,7 @@ public static <W, K, V> V getAttachmentFromWrapperChain(final W wrapper, final K
      *                               or any wrapper {@link Wrapper#unwrap()} returns null,
      *                               or the adaptee of {@link WrapperAdapter} is null
      * @throws IllegalStateException if the adaptee of {@link WrapperAdapter} is an instance of {@link Wrapper}
+     *                               or CYCLIC wrapper chain
      * @see Wrapper#unwrap()
      * @see WrapperAdapter#adaptee()
      */
@@ -156,6 +160,7 @@ public static <W> void verifyWrapperChainContracts(final W wrapper) {
      *                               or any wrapper {@link Wrapper#unwrap()} returns null,
      *                               or the adaptee of {@link WrapperAdapter} is null
      * @throws IllegalStateException if the adaptee of {@link WrapperAdapter} is an instance of {@link Wrapper}
+     *                               or CYCLIC wrapper chain
      * @see Wrapper#unwrap()
      * @see WrapperAdapter#adaptee()
      */
@@ -186,6 +191,7 @@ public static <W> boolean testWrapperChain(final W wrapper, final Predicate<? su
      *                               or any wrapper {@link Wrapper#unwrap()} returns null,
      *                               or the adaptee of {@link WrapperAdapter} is null
      * @throws IllegalStateException if the adaptee of {@link WrapperAdapter} is an instance of {@link Wrapper}
+     *                               or CYCLIC wrapper chain
      * @see Wrapper#unwrap()
      * @see WrapperAdapter#adaptee()
      */
@@ -195,8 +201,11 @@ public static <W, T> Optional<T> travelWrapperChain(final W wrapper, final Funct
         requireNonNull(wrapper, "wrapper is null");
         requireNonNull(process, "process is null");
 
+        final IdentityHashMap<Object, ?> history = new IdentityHashMap<>();
         Object w = wrapper;
         while (true) {
+            history.put(w, null);
+
             // process the instance on wrapper chain
             Optional<T> result = process.apply((W) w);
             if (result.isPresent()) return result;
@@ -209,6 +218,10 @@ public static <W, T> Optional<T> travelWrapperChain(final W wrapper, final Funct
 
             if (!(w instanceof Wrapper)) return Optional.empty();
             w = unwrapNonNull(w);
+            if (history.containsKey(w)) {
+                throw new IllegalStateException("CYCLIC wrapper chain," +
+                        " duplicate instance of " + w.getClass().getName());
+            }
         }
     }
 
@@ -224,7 +237,7 @@ private static Object adapteeNonWrapper(final Object wrapper) {
         if (adaptee instanceof Wrapper) {
             throw new IllegalStateException("adaptee(" + adaptee.getClass().getName() +
                     ") of WrapperAdapter(" + wrapper.getClass().getName() +
-                    ") is an instance of Wrapper, adapting a Wrapper to a Wrapper is unnecessary!");
+                    ") is an instance of Wrapper, adapting a Wrapper to a Wrapper is UNNECESSARY");
         }
 
         return adaptee;

diff --git a/src/main/java/io/foldright/inspectablewrappers/Wrapper.java b/src/main/java/io/foldright/inspectablewrappers/Wrapper.java
@@ -27,13 +27,20 @@
 public interface Wrapper<T> {
     /**
      * Returns the underlying instance that be wrapped.
-     * This method also make the wrapper instances as a wrapper chain(linked list).
      * <p>
-     * <strong>Specification contracts:</strong>
+     * This method also make the wrapper instances as a <strong>wrapper chain</strong>(linked list),
+     * The wrapper chain consists of wrapper itself, followed by the wrappers
+     * obtained by repeatedly calling this method.
      * <p>
-     * Do NOT return {@code null} which makes no sense.<br>
-     * If returns {@code null}, the inspection operations of {@link Inspector} will
-     * throw {@link NullPointerException} when touch the {@code unwrap}.
+     * <strong>Specification contracts:</strong>
+     * <ul>
+     * <li>Do NOT return {@code null} which makes no sense.<br>
+     *     If returns {@code null}, the inspection operations of {@link Inspector} will
+     *     throw {@link NullPointerException} when touch the {@code null unwrap value}.
+     * <li>The wrapper chain can NOT be CYCLIC(aka. the return value/wrapper instance
+     *     is duplicate on the wrapper chain).<br>If cyclic, the inspection operations of {@link Inspector}
+     *     will throw {@link IllegalStateException} when touch the {@code duplicate unwrap value}.
+     * </ul>
      */
     @NonNull
     T unwrap();

diff --git a/src/main/java/io/foldright/inspectablewrappers/utils/WrapperAdapterUtils.java b/src/main/java/io/foldright/inspectablewrappers/utils/WrapperAdapterUtils.java
@@ -137,7 +137,7 @@ private static <T> void checkTypeRequirements(Class<T> bizInterface, T underlyin
 
         if (adaptee instanceof Wrapper) {
             throw new IllegalArgumentException("adaptee(" + adaptee.getClass().getName() +
-                    ") is an instance of Wrapper, adapting a Wrapper to a Wrapper is unnecessary!");
+                    ") is an instance of Wrapper, adapting a Wrapper to a Wrapper is UNNECESSARY");
         }
     }
 

diff --git a/src/test/java/io/foldright/inspectablewrappers/SpecificationContractsTest.java b/src/test/java/io/foldright/inspectablewrappers/SpecificationContractsTest.java
@@ -1,12 +1,12 @@
 package io.foldright.inspectablewrappers;
 
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import java.util.concurrent.Executor;
 
 import static io.foldright.inspectablewrappers.Inspector.verifyWrapperChainContracts;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
 
 
 class SpecificationContractsTest {
@@ -18,7 +18,7 @@ class SpecificationContractsTest {
     void test_null_unwrap() {
         Executor w = new WrapperImpl(null);
 
-        NullPointerException e = Assertions.assertThrowsExactly(NullPointerException.class, () -> verifyWrapperChainContracts(w));
+        NullPointerException e = assertThrowsExactly(NullPointerException.class, () -> verifyWrapperChainContracts(w));
         String expected = "unwrap of Wrapper(io.foldright.inspectablewrappers.SpecificationContractsTest$WrapperImpl) is null";
         assertEquals(expected, e.getMessage());
     }
@@ -27,7 +27,7 @@ void test_null_unwrap() {
     void test_null_adaptee() {
         Executor w = new WrapperAdapterImpl(DUMMY, null);
 
-        NullPointerException e = Assertions.assertThrowsExactly(NullPointerException.class, () -> verifyWrapperChainContracts(w));
+        NullPointerException e = assertThrowsExactly(NullPointerException.class, () -> verifyWrapperChainContracts(w));
         String expected = "adaptee of WrapperAdapter(io.foldright.inspectablewrappers.SpecificationContractsTest$WrapperAdapterImpl) is null";
         assertEquals(expected, e.getMessage());
     }
@@ -36,10 +36,25 @@ void test_null_adaptee() {
     void test_Wrap_type_adaptee() {
         Executor w = new WrapperAdapterImpl(DUMMY, new WrapperImpl(null));
 
-        IllegalStateException e = Assertions.assertThrowsExactly(IllegalStateException.class, () -> verifyWrapperChainContracts(w));
+        IllegalStateException e = assertThrowsExactly(IllegalStateException.class, () -> verifyWrapperChainContracts(w));
         String expected = "adaptee(io.foldright.inspectablewrappers.SpecificationContractsTest$WrapperImpl)" +
                 " of WrapperAdapter(io.foldright.inspectablewrappers.SpecificationContractsTest$WrapperAdapterImpl)" +
-                " is an instance of Wrapper, adapting a Wrapper to a Wrapper is unnecessary!";
+                " is an instance of Wrapper, adapting a Wrapper to a Wrapper is UNNECESSARY";
+        assertEquals(expected, e.getMessage());
+    }
+
+    @Test
+    void testCyclicWrapperChain() {
+        MutableWrapperImpl w1 = new MutableWrapperImpl();
+        MutableWrapperImpl w2 = new MutableWrapperImpl();
+        w1.instance = w2;
+        w2.instance = w1;
+
+        IllegalStateException e = assertThrowsExactly(IllegalStateException.class, () -> {
+            verifyWrapperChainContracts(w2);
+        });
+        String expected = "CYCLIC wrapper chain, duplicate instance of" +
+                " io.foldright.inspectablewrappers.SpecificationContractsTest$MutableWrapperImpl";
         assertEquals(expected, e.getMessage());
     }
 
@@ -83,4 +98,17 @@ public Executor adaptee() {
         public void execute(Runnable command) {
         }
     }
+
+    private static class MutableWrapperImpl implements Wrapper<Executor>, Executor {
+        Executor instance;
+
+        @Override
+        public Executor unwrap() {
+            return instance;
+        }
+
+        @Override
+        public void execute(Runnable command) {
+        }
+    }
 }
diff --git a/src/test/java/io/foldright/inspectablewrappers/WrapperAdapterTest.kt b/src/test/java/io/foldright/inspectablewrappers/WrapperAdapterTest.kt
@@ -59,7 +59,7 @@ class WrapperAdapterTest : FunSpec({
 
         val errMsg = "adaptee(io.foldright.inspectablewrappers.ChattyExecutorWrapper)" +
                 " of WrapperAdapter(io.foldright.inspectablewrappers.ChattyExecutorWrapperAdapter)" +
-                " is an instance of Wrapper, adapting a Wrapper to a Wrapper is unnecessary!"
+                " is an instance of Wrapper, adapting a Wrapper to a Wrapper is UNNECESSARY"
 
         shouldThrow<IllegalStateException> {
             containsInstanceOnWrapperChain(chain, ExecutorService::class.java)

diff --git a/src/test/java/io/foldright/inspectablewrappers/utils/WrapperAdapterUtilsTest.kt b/src/test/java/io/foldright/inspectablewrappers/utils/WrapperAdapterUtilsTest.kt
@@ -76,7 +76,7 @@ class WrapperAdapterUtilsTest : FunSpec({
         shouldThrow<IllegalArgumentException> {
             createWrapperAdapter(Executor::class.java, executor, wrongAdaptee)
         }.message shouldBe "adaptee(io.foldright.inspectablewrappers.utils.WrongWrapperAdapter) is an instance of Wrapper," +
-                " adapting a Wrapper to a Wrapper is unnecessary!"
+                " adapting a Wrapper to a Wrapper is UNNECESSARY"
     }
 
     @Suppress("UNCHECKED_CAST")