Skip to content

<test>[testlib]: cjlib improve #4162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MatheMatrix wants to merge 1 commit into
base: 4.8.38
Choose a base branch
from
+27 −0
Open

<test>[testlib]: cjlib improve #4162

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions testlib/src/main/java/org/zstack/testlib/util/TProxy.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,33 @@ class TProxy {
}
}

TProxy(Object adaptee, Class[] argumentTypes, Object[] arguments) {
assert adaptee != null : "call TProxy(Class adapteeClass) instead"
Comment on lines +72 to +73
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

添加参数验证以防止运行时错误。

构造函数缺少对 argumentTypes 和 arguments 的验证：

  • 未检查 null 值，可能导致 NPE
  • 未验证 argumentTypes.length == arguments.length，类型与参数数量不匹配会在运行时失败
🛡️ 建议添加参数验证 
 TProxy(Object adaptee, Class[] argumentTypes, Object[] arguments) {
     assert adaptee != null : "call TProxy(Class adapteeClass) instead"
+    assert argumentTypes != null : "argumentTypes cannot be null"
+    assert arguments != null : "arguments cannot be null"
+    assert argumentTypes.length == arguments.length : "argumentTypes and arguments must have the same length"
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testlib/src/main/java/org/zstack/testlib/util/TProxy.groovy` around lines 72
- 73, 构造函数 TProxy(Object adaptee, Class[] argumentTypes, Object[] arguments) 缺少对
argumentTypes 和 arguments 的校验，会导致 NPE 或参数数量/类型不匹配的运行时错误；在方法开始处验证 argumentTypes 和
arguments 非 null，并且检查 argumentTypes.length == arguments.length（不满足时抛出
IllegalArgumentException 或 AssertionError），同时保留对 adaptee
的现有断言并在错误消息中说明期望的参数数量和类型以便调试。


Enhancer enhancer = new Enhancer()
enhancer.setSuperclass(adaptee.getClass())
enhancer.setCallback(new ProxyHandler())
try {
proxyedObject = enhancer.create(argumentTypes, arguments)
if (adaptee != null) {
FieldUtils.getAllFields(adaptee.getClass()).each { f ->
if (Modifier.isStatic(f.modifiers) || Modifier.isFinal(f.modifiers)) {
return
}

f.setAccessible(true)
f.set(proxyedObject, f.get(adaptee))
}
}
} catch (IllegalArgumentException e) {
if (e.getMessage().contains("Superclass has no null constructors" )) {
throw new Exception("the class ${adaptee.getClass()} has no non-argument constructor", e)
}

throw e
}
Comment on lines +90 to +96
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

修正异常消息以匹配参数化构造的上下文。

第 92 行的错误消息"has no non-argument constructor"在此上下文中不正确。当调用带参数的构造函数时，该消息具有误导性——实际问题应该是找不到匹配的参数化构造函数，而非无参构造函数缺失。

🐛 建议修正错误消息 
         } catch (IllegalArgumentException e) {
             if (e.getMessage().contains("Superclass has no null constructors" )) {
-                throw new Exception("the class ${adaptee.getClass()} has no non-argument constructor", e)
+                throw new Exception("the class ${adaptee.getClass()} has no matching constructor for the given argument types", e)
             }
 
             throw e
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} catch (IllegalArgumentException e) {
if (e.getMessage().contains("Superclass has no null constructors" )) {
throw new Exception("the class ${adaptee.getClass()} has no non-argument constructor", e)
}
throw e
}
} catch (IllegalArgumentException e) {
if (e.getMessage().contains("Superclass has no null constructors" )) {
throw new Exception("the class ${adaptee.getClass()} has no matching constructor for the given argument types", e)
}
throw e
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testlib/src/main/java/org/zstack/testlib/util/TProxy.groovy` around lines 90
- 96, The catch block handling IllegalArgumentException in TProxy (where
adaptee.getClass() is referenced) uses a misleading message "has no non-argument
constructor"; update that exception message to reflect the actual situation when
a parameterized constructor cannot be found (e.g., "has no matching
parameterized constructor for the provided arguments") and rethrow the new
Exception with the original cause so the message correctly describes the lookup
failure for a parameterized constructor.

}
Comment on lines +72 to +97
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

消除与现有构造函数的代码重复。

新构造函数与 TProxy(Object adaptee)（第 45-70 行）几乎完全重复，唯一区别是第 79 行调用 enhancer.create(argumentTypes, arguments) 而非无参的 enhancer.create()。字段复制和异常处理逻辑完全相同，违反 DRY 原则。

建议提取公共逻辑到私有辅助方法中，以提高可维护性并减少未来出现不一致的风险。

♻️ 建议重构以消除重复 
+    private void copyFields(Object source, Object target) {
+        FieldUtils.getAllFields(source.getClass()).each { f ->
+            if (Modifier.isStatic(f.modifiers) || Modifier.isFinal(f.modifiers)) {
+                return
+            }
+            f.setAccessible(true)
+            f.set(target, f.get(source))
+        }
+    }
+
+    private Object createProxy(Object adaptee, Closure createEnhancer) {
+        Enhancer enhancer = new Enhancer()
+        enhancer.setSuperclass(adaptee.getClass())
+        enhancer.setCallback(new ProxyHandler())
+        try {
+            def proxy = createEnhancer(enhancer)
+            if (adaptee != null) {
+                copyFields(adaptee, proxy)
+            }
+            return proxy
+        } catch (IllegalArgumentException e) {
+            if (e.getMessage().contains("Superclass has no null constructors")) {
+                throw new Exception("the class ${adaptee.getClass()} has no matching constructor", e)
+            }
+            throw e
+        }
+    }
+
     TProxy(Object adaptee) {
         assert adaptee != null : "call TProxy(Class adapteeClass) instead"
-        Enhancer enhancer = new Enhancer()
-        enhancer.setSuperclass(adaptee.getClass())
-        enhancer.setCallback(new ProxyHandler())
-        try {
-            proxyedObject = enhancer.create()
-            if (adaptee != null) {
-                FieldUtils.getAllFields(adaptee.getClass()).each { f ->
-                    if (Modifier.isStatic(f.modifiers) || Modifier.isFinal(f.modifiers)) {
-                        return
-                    }
-
-                    f.setAccessible(true)
-                    f.set(proxyedObject, f.get(adaptee))
-                }
-            }
-        } catch (IllegalArgumentException e) {
-            if (e.getMessage().contains("Superclass has no null constructors" )) {
-                throw new Exception("the class ${adaptee.getClass()} has no non-argument constructor", e)
-            }
-
-            throw e
-        }
+        proxyedObject = createProxy(adaptee) { enhancer -> enhancer.create() }
     }
 
     TProxy(Object adaptee, Class[] argumentTypes, Object[] arguments) {
         assert adaptee != null : "call TProxy(Class adapteeClass) instead"
-        Enhancer enhancer = new Enhancer()
-        enhancer.setSuperclass(adaptee.getClass())
-        enhancer.setCallback(new ProxyHandler())
-        try {
-            proxyedObject = enhancer.create(argumentTypes, arguments)
-            if (adaptee != null) {
-                FieldUtils.getAllFields(adaptee.getClass()).each { f ->
-                    if (Modifier.isStatic(f.modifiers) || Modifier.isFinal(f.modifiers)) {
-                        return
-                    }
-
-                    f.setAccessible(true)
-                    f.set(proxyedObject, f.get(adaptee))
-                }
-            }
-        } catch (IllegalArgumentException e) {
-            if (e.getMessage().contains("Superclass has no null constructors" )) {
-                throw new Exception("the class ${adaptee.getClass()} has no non-argument constructor", e)
-            }
-
-            throw e
-        }
+        proxyedObject = createProxy(adaptee) { enhancer -> enhancer.create(argumentTypes, arguments) }
     }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testlib/src/main/java/org/zstack/testlib/util/TProxy.groovy` around lines 72
- 97, The two constructors TProxy(Object adaptee) and TProxy(Object adaptee,
Class[] argumentTypes, Object[] arguments) duplicate field-copy and
exception-handling logic; refactor by extracting the shared logic into a private
helper (e.g., initProxyFields(Object proxyedObject, Object adaptee)) that
performs the FieldUtils.getAllFields loop and the IllegalArgumentException
handling, and have each constructor only create the Enhancer, call
enhancer.create(...) with the appropriate signature, then pass the created proxy
object and adaptee into that helper; ensure the helper preserves the current
behavior (skip static/final fields, setAccessible, copy values, and translate
the specific IllegalArgumentException message).


/**
*
* @param name: 要替换函数的名称
Expand Down