You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by ch...@apache.org on 2026/08/31 09:17:08 UTC
(kyuubi) branch master updated: [KYUUBI #7690] Fix DynFields javadoc copied from the method variants
This is an automated email from the ASF dual-hosted git repository.
pan3793 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kyuubi.git
The following commit(s) were added to refs/heads/master by this push:
new 797ba2bd43 [KYUUBI #7690] Fix DynFields javadoc copied from the method variants
797ba2bd43 is described below
commit 797ba2bd43bf44dd2db94ac35e756c8d69abbce6
Author: YangJie <ya...@baidu.com>
AuthorDate: Mon Aug 31 17:16:58 2026 +0800
[KYUUBI #7690] Fix DynFields javadoc copied from the method variants
### Why are the changes needed?
Closes #7690.
`DynFields` is vendored from iceberg-common and its javadoc came across with the `DynMethods` wording still in it, so a field API ends up documented in method vocabulary. `bind` said it returns a `BoundMethod`, a type this file does not contain. Six `throws` tags said "if the method is static" or "if the method is not static" for a check that reads `Modifier.isStatic` on a `Field`. Three `param fieldName` tags carried `(different from constructor)`, which separates a method from a con [...]
Two of the fourteen corrections change an exception type. `build()` was documented as throwing `NoSuchFieldException` when it has no `throws` clause and raises `RuntimeException`; the `throws RuntimeException` tag in the same comment already contradicted that summary. `buildChecked(Object)` was documented as returning a `BoundMethod` and throwing `NoSuchMethodException` while its signature declares `throws NoSuchFieldException`. In both cases the documented exception is one the compil [...]
The `UnboundField` class doc claimed "all Exceptions wrapped by RuntimeException, or with a single Exception catch block". `get` and `set` catch only `IllegalAccessException`, and this class has no checked-throwing accessor for the second clause to refer to; that clause describes `DynMethods.UnboundMethod.invokeChecked`. The class doc now names the one throwable that is wrapped and says the rest are not.
Upstream iceberg-common carries the same wrong text, so this is not local drift that a resync would repair. Thirteen of the fourteen lines have a direct upstream counterpart and are worth sending to iceberg separately.
Left for a follow-up: the builder's `throws` tags are written unconditionally and stop holding once `defaultAlwaysNull()` is set, because the sentinel is returned instead of an exception and its `bind` override skips the static and receiver checks. Both `hiddenImpl` overloads also point `see` at `Class#getField(String)` while the code calls `getDeclaredField`. `DynMethods` has both defects too, so the two vendored copies should change together.
### How was this patch tested?
Every changed line sits inside a `/** */` block, so there is no behaviour to test, and a javadoc assertion would pass with the patch reverted.
`build/mvn -o test -pl kyuubi-util -am -DwildcardSuites=none` gives 33/33 green. `build/mvn -o spotless:check -pl kyuubi-util` is clean.
Existing tests in `DynFieldsTest` already cover the behaviour these sentences describe: `testBuildWithoutAlwaysNullFallbackThrows` for `build()` raising `RuntimeException`, `testBindRejectsStaticField` and `testBindRejectsIncompatibleTarget` for `bind`'s two conditions, and `testAlwaysNullBindsThroughEveryEntryPoint` for the sentinel path behind the deferred wording. Two of the behavioural claims have no test and were checked by reading the code: the class doc's wrapping sentence, and [...]
### Was this patch assisted by generative AI tooling?
Assisted-by: Claude Opus 5
Closes #7691 from LuciferYang/kyuubi-util-dynfields-javadoc.
Closes #7690
ec814231d [yangjie01] [KYUUBI #7690] Fix DynFields javadoc copied from the method variants
Authored-by: YangJie <ya...@baidu.com>
Signed-off-by: Cheng Pan <ch...@apache.org>
---
.../org/apache/kyuubi/util/reflect/DynFields.java | 31 +++++++++++-----------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynFields.java b/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynFields.java
index 1ab64b4221..51fb72e556 100644
--- a/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynFields.java
+++ b/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynFields.java
@@ -34,8 +34,9 @@ public class DynFields {
/**
* Convenience wrapper class around {@link Field}.
*
- * <p>Allows callers to invoke the wrapped method with all Exceptions wrapped by RuntimeException,
- * or with a single Exception catch block.
+ * <p>Allows callers to access the wrapped field with {@code IllegalAccessException} wrapped by
+ * RuntimeException. Other throwables {@link Field} raises, such as the {@code
+ * IllegalArgumentException} for an incompatible target, are not wrapped.
*/
public static class UnboundField<T> {
private final Field field;
@@ -70,11 +71,11 @@ public class DynFields {
}
/**
- * Returns this method as a BoundMethod for the given receiver.
+ * Returns this field as a BoundField for the given receiver.
*
* @param target an Object on which to get or set this field
* @return a {@link BoundField} for this field and the target
- * @throws IllegalStateException if the method is static
+ * @throws IllegalStateException if the field is static
* @throws IllegalArgumentException if the receiver's class is incompatible
*/
public BoundField<T> bind(Object target) {
@@ -94,7 +95,7 @@ public class DynFields {
* Returns this field as a StaticField.
*
* @return a {@link StaticField} for this field
- * @throws IllegalStateException if the method is not static
+ * @throws IllegalStateException if the field is not static
*/
public StaticField<T> asStatic() {
if (!isStatic()) {
@@ -262,7 +263,7 @@ public class DynFields {
* Checks for an implementation.
*
* @param targetClass a class instance
- * @param fieldName name of a field (different from constructor)
+ * @param fieldName name of a field
* @return this Builder for method chaining
* @see Class#forName(String)
* @see Class#getField(String)
@@ -286,7 +287,7 @@ public class DynFields {
* Checks for a hidden implementation, first finding the class by name.
*
* @param className name of a class
- * @param fieldName name of a field (different from constructor)
+ * @param fieldName name of a field
* @return this Builder for method chaining
* @see Class#forName(String)
* @see Class#getField(String)
@@ -311,7 +312,7 @@ public class DynFields {
* Checks for a hidden implementation.
*
* @param targetClass a class instance
- * @param fieldName name of a field (different from constructor)
+ * @param fieldName name of a field
* @return this Builder for method chaining
* @see Class#forName(String)
* @see Class#getField(String)
@@ -355,13 +356,13 @@ public class DynFields {
}
/**
- * Returns the first valid implementation as a BoundMethod or throws a NoSuchMethodException if
+ * Returns the first valid implementation as a BoundField or throws a NoSuchFieldException if
* there is none.
*
* @param target an Object on which to get and set the field
* @param <T> Java class stored in the field
* @return a {@link BoundField} with a valid implementation and target
- * @throws IllegalStateException if the method is static
+ * @throws IllegalStateException if the field is static
* @throws IllegalArgumentException if the receiver's class is incompatible
* @throws NoSuchFieldException if no implementation was found
*/
@@ -370,7 +371,7 @@ public class DynFields {
}
/**
- * Returns the first valid implementation as a UnboundField or throws a NoSuchFieldException if
+ * Returns the first valid implementation as a UnboundField or throws a RuntimeException if
* there is none.
*
* @param <T> Java class stored in the field
@@ -390,13 +391,13 @@ public class DynFields {
}
/**
- * Returns the first valid implementation as a BoundMethod or throws a RuntimeException if there
+ * Returns the first valid implementation as a BoundField or throws a RuntimeException if there
* is none.
*
* @param target an Object on which to get and set the field
* @param <T> Java class stored in the field
* @return a {@link BoundField} with a valid implementation and target
- * @throws IllegalStateException if the method is static
+ * @throws IllegalStateException if the field is static
* @throws IllegalArgumentException if the receiver's class is incompatible
* @throws RuntimeException if no implementation was found
*/
@@ -410,7 +411,7 @@ public class DynFields {
*
* @param <T> Java class stored in the field
* @return a {@link StaticField} with a valid implementation
- * @throws IllegalStateException if the method is not static
+ * @throws IllegalStateException if the field is not static
* @throws NoSuchFieldException if no implementation was found
*/
public <T> StaticField<T> buildStaticChecked() throws NoSuchFieldException {
@@ -423,7 +424,7 @@ public class DynFields {
*
* @param <T> Java class stored in the field
* @return a {@link StaticField} with a valid implementation
- * @throws IllegalStateException if the method is not static
+ * @throws IllegalStateException if the field is not static
* @throws RuntimeException if no implementation was found
*/
public <T> StaticField<T> buildStatic() {