From 6de1b20d286ec38f47de6a9d1a7b7c9e776b1fe4 Mon Sep 17 00:00:00 2001
From: Jerry Lee <oldratlee@gmail.com>
Date: Sat, 23 Mar 2024 22:43:42 +0800
Subject: [PATCH] =?UTF-8?q?refactor:=20move=20the=20`WrapperAdapter.adapte?=
 =?UTF-8?q?e`=20process=20into=20`Wrapper.travel`=20=F0=9F=94=8C?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../inspectablewrappers/Wrapper.java          | 27 +++++++++++----
 .../inspectablewrappers/WrapperAdapterTest.kt | 33 +++++++++++++++++--
 2 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/src/main/java/io/foldright/inspectablewrappers/Wrapper.java b/src/main/java/io/foldright/inspectablewrappers/Wrapper.java
index 00377e4..874d5d8 100644
--- a/src/main/java/io/foldright/inspectablewrappers/Wrapper.java
+++ b/src/main/java/io/foldright/inspectablewrappers/Wrapper.java
@@ -48,13 +48,7 @@ public interface Wrapper<T> {
     static <W> boolean isInstanceOf(final W wrapper, final Class<?> clazz) {
         requireNonNull(wrapper, "wrapper is null");
         requireNonNull(clazz, "clazz is null");
-        return inspect(wrapper, w -> {
-            if (w instanceof WrapperAdapter) {
-                Object adaptee = ((WrapperAdapter<?>) w).adaptee();
-                if (clazz.isAssignableFrom(adaptee.getClass())) return true;
-            }
-            return clazz.isAssignableFrom(w.getClass());
-        });
+        return inspect(wrapper, w -> clazz.isAssignableFrom(w.getClass()));
     }
 
     /**
@@ -128,16 +122,35 @@ static <W, K, V> V getAttachment(final W wrapper, final K key) {
      * @param <T>     the return data type of process function
      * @return the first non-empty({@link Optional#empty()}) result of the process function,
      * otherwise returns {@link Optional#empty()}.
+     * @throws NullPointerException  if any arguments is null or any wrapper {{@link #unwrap()}} returns null
+     * @throws IllegalStateException if the adaptee of {@link WrapperAdapter} is a wrapper instance,
+     *                               the use of WrapperAdapter is unnecessary!
      */
+    @NonNull
     @SuppressWarnings("unchecked")
     static <W, T> Optional<T> travel(final W wrapper, final Function<W, Optional<T>> process) {
         requireNonNull(wrapper, "wrapper is null");
         requireNonNull(process, "process is null");
+
         Object w = wrapper;
         while (true) {
+            // process the instance on wrapper chain
             Optional<T> result = process.apply((W) w);
             if (result.isPresent()) return result;
 
+            // also process the adaptee if it's a WrapperAdapter
+            if (w instanceof WrapperAdapter) {
+                final Object adaptee = ((WrapperAdapter<?>) w).adaptee();
+                if (adaptee instanceof Wrapper) {
+                    throw new IllegalStateException("adaptee(" + adaptee.getClass().getName() +
+                            ") of WrapperAdapter(" + w.getClass().getName() +
+                            ") is a wrapper instance, the use of WrapperAdapter is unnecessary!");
+                }
+
+                Optional<T> r = process.apply((W) adaptee);
+                if (r.isPresent()) return r;
+            }
+
             if (!(w instanceof Wrapper)) return Optional.empty();
             w = unwrapNonNull(w);
         }
diff --git a/src/test/java/io/foldright/inspectablewrappers/WrapperAdapterTest.kt b/src/test/java/io/foldright/inspectablewrappers/WrapperAdapterTest.kt
index 6659faa..9dcf264 100644
--- a/src/test/java/io/foldright/inspectablewrappers/WrapperAdapterTest.kt
+++ b/src/test/java/io/foldright/inspectablewrappers/WrapperAdapterTest.kt
@@ -46,7 +46,7 @@ class WrapperAdapterTest : FunSpec({
     @Suppress("NULLABILITY_MISMATCH_BASED_ON_JAVA_ANNOTATIONS")
     test("argument null") {
         shouldThrow<NullPointerException> {
-            Wrapper.getAttachment<Executor, String, String?>(null , ADAPTED_MSG_KEY)
+            Wrapper.getAttachment<Executor, String, String?>(null, ADAPTED_MSG_KEY)
         }.message shouldBe "wrapper is null"
 
         shouldThrow<NullPointerException> {
@@ -54,6 +54,26 @@ class WrapperAdapterTest : FunSpec({
         }.message shouldBe "key is null"
     }
 
+    test("travel IllegalStateException - the adaptee of WrapperAdapter is a wrapper instance") {
+        val chain: Executor = ChattyExecutorWrapper { runnable -> runnable.run() }
+            .let(::ChattyExecutorWrapperAdapter)
+
+        val errMsg = "adaptee(io.foldright.inspectablewrappers.ChattyExecutorWrapper)" +
+                " of WrapperAdapter(io.foldright.inspectablewrappers.ChattyExecutorWrapperAdapter)" +
+                " is a wrapper instance, the use of WrapperAdapter is unnecessary!"
+
+        shouldThrow<IllegalStateException> {
+            Wrapper.isInstanceOf(chain, ExecutorService::class.java)
+        }.message shouldBe errMsg
+        // first instance is ok, not trigger the check logic yet...
+        Wrapper.isInstanceOf(chain, Executor::class.java).shouldBeTrue()
+        Wrapper.isInstanceOf(chain, ChattyExecutorWrapperAdapter::class.java).shouldBeTrue()
+
+
+        shouldThrow<IllegalStateException> {
+            Wrapper.getAttachment(chain, "k1")
+        }.message shouldBe errMsg
+    }
 })
 
 /**
@@ -62,7 +82,6 @@ class WrapperAdapterTest : FunSpec({
 private class ExistedExecutorWrapperAdapter(private val adaptee: ExistedExecutorWrapper) :
         Executor by adaptee, WrapperAdapter<Executor>, Attachable<String, String> by AttachableDelegate() {
     override fun unwrap(): Executor = adaptee.executor
-
     override fun adaptee(): Executor = adaptee
 }
 
@@ -72,3 +91,13 @@ class ExistedExecutorWrapper(val executor: Executor) : Executor {
         executor.execute(command)
     }
 }
+
+/**
+ * Wrong use the [WrapperAdapter], the adaptee is [Wrapper].
+ */
+private class ChattyExecutorWrapperAdapter(private val adaptee: ChattyExecutorWrapper) :
+        Executor by adaptee, WrapperAdapter<Executor>, Attachable<String, String> by AttachableDelegate() {
+    override fun unwrap(): Executor = adaptee.unwrap()
+    override fun adaptee(): Executor = adaptee
+}
+