From 479b6987f015403350dd405ef51b400b2e399699 Mon Sep 17 00:00:00 2001 From: Jerry Lee Date: Thu, 4 Apr 2024 18:58:28 +0800 Subject: [PATCH] =?UTF-8?q?feat:=20check=20cyclic=20wrapper=20chain=20?= =?UTF-8?q?=F0=9F=94=84=F0=9F=92=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../inspectablewrappers/Inspector.java | 12 +++++- .../inspectablewrappers/Wrapper.java | 17 ++++++--- .../utils/WrapperAdapterUtils.java | 2 +- .../SpecificationContractsTest.java | 37 ++++++++++++++++--- .../inspectablewrappers/WrapperAdapterTest.kt | 2 +- .../utils/WrapperAdapterUtilsTest.kt | 2 +- 6 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/main/java/io/foldright/inspectablewrappers/Inspector.java b/src/main/java/io/foldright/inspectablewrappers/Inspector.java index 5055843..8bc0da2 100644 --- 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 } * @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 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 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 boolean testWrapperChain(final W wrapper, final Predicate Optional travelWrapperChain(final W wrapper, final Funct requireNonNull(wrapper, "wrapper is null"); requireNonNull(process, "process is null"); + final IdentityHashMap history = new IdentityHashMap<>(); Object w = wrapper; while (true) { + history.put(w, null); + // process the instance on wrapper chain Optional result = process.apply((W) w); if (result.isPresent()) return result; @@ -209,6 +218,7 @@ public static Optional 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"); } } @@ -224,7 +234,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 index 7712cee..06644fb 100644 --- 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 { /** * Returns the underlying instance that be wrapped. - * This method also make the wrapper instances as a wrapper chain(linked list). *

- * Specification contracts: + * This method also make the wrapper instances as a wrapper chain(linked list), + * The wrapper chain consists of wrapper itself, followed by the wrappers + * obtained by repeatedly calling this method. *

- * Do NOT return {@code null} which makes no sense.
- * If returns {@code null}, the inspection operations of {@link Inspector} will - * throw {@link NullPointerException} when touch the {@code unwrap}. + * Specification contracts: + *

    + *
  • Do NOT return {@code null} which makes no sense.
    + * If returns {@code null}, the inspection operations of {@link Inspector} will + * throw {@link NullPointerException} when touch the {@code null unwrap value}. + *
  • The wrapper chain can NOT be CYCLIC(aka. the return value/wrapper instance + * is duplicate on the wrapper chain).
    If cyclic, the inspection operations of {@link Inspector} + * will throw {@link IllegalStateException} when touch the {@code duplicate unwrap value}. + *
*/ @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 index 1c9d546..7e56eda 100644 --- 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 void checkTypeRequirements(Class 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 index 22e5c35..90e1a79 100644 --- 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,24 @@ 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"; assertEquals(expected, e.getMessage()); } @@ -83,4 +97,17 @@ public Executor adaptee() { public void execute(Runnable command) { } } + + private static class MutableWrapperImpl implements Wrapper, 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 index 4405b95..9d2f6e9 100644 --- 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 { 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 index caa68f6..d3a1141 100644 --- 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 { 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")