This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fury.git
The following commit(s) were added to refs/heads/main by this push:
new 2d51e044 fix(java): row encoder incorrectly interprets type parameters
as cycles (#2265)
2d51e044 is described below
commit 2d51e044e8ff8ea0aeebe33affece574df41ff97
Author: Steven Schlansker <[email protected]>
AuthorDate: Thu May 29 09:50:27 2025 -0700
fix(java): row encoder incorrectly interprets type parameters as cycles
(#2265)
## What does this PR do?
Given a typed-identifier type `class Id<T> { int id; }` and a `class T {
Id<T> id; }`, Fury incorrectly reports a type cycle of T, but the Id
type does not actually have any property of type T.
This is due to over-eager expansion of bean types in the type utils -
all generic types are assumed to be used as property types, which is not
always correct.
---
.../main/java/org/apache/fory/type/TypeUtils.java | 82 +++++++++-------------
.../org/apache/fory/format/encoder/Encoders.java | 1 +
.../fory/format/encoder/GenericTypeTest.java | 57 +++++++++++++++
3 files changed, 90 insertions(+), 50 deletions(-)
diff --git a/java/fory-core/src/main/java/org/apache/fory/type/TypeUtils.java
b/java/fory-core/src/main/java/org/apache/fory/type/TypeUtils.java
index 8162ac56..15b8d9ca 100644
--- a/java/fory-core/src/main/java/org/apache/fory/type/TypeUtils.java
+++ b/java/fory-core/src/main/java/org/apache/fory/type/TypeUtils.java
@@ -664,14 +664,12 @@ public class TypeUtils {
return isSupported(Objects.requireNonNull(typeRef.getComponentType()));
} else if (ITERABLE_TYPE.isSupertypeOf(typeRef)) {
TypeRef<?> elementType = getElementType(typeRef);
- if (ctx.getCustomTypeRegistry()
- .canConstructCollection(typeRef.getRawType(),
elementType.getRawType())) {
- return true;
- }
boolean isSuperOfArrayList = cls.isAssignableFrom(ArrayList.class);
boolean isSuperOfHashSet = cls.isAssignableFrom(HashSet.class);
if ((!isSuperOfArrayList && !isSuperOfHashSet)
- && (cls.isInterface() || Modifier.isAbstract(cls.getModifiers()))) {
+ && (cls.isInterface() || Modifier.isAbstract(cls.getModifiers()))
+ && !ctx.getCustomTypeRegistry()
+ .canConstructCollection(typeRef.getRawType(),
elementType.getRawType())) {
return false;
}
return isSupported(getElementType(typeRef));
@@ -707,58 +705,42 @@ public class TypeUtils {
if (beanClass.isInterface()) {
ctx = ctx.withSynthesizedBeanType(beanClass);
}
- return listBeansRecursiveInclusive(beanClass, ctx);
+ return listBeansRecursiveInclusive(TypeRef.of(beanClass), ctx);
}
private static LinkedHashSet<Class<?>> listBeansRecursiveInclusive(
- Class<?> beanClass, TypeResolutionContext ctx) {
+ TypeRef<?> typeRef, TypeResolutionContext ctx) {
LinkedHashSet<Class<?>> beans = new LinkedHashSet<>();
Class<?> enclosingType = ctx.getEnclosingType().getRawType();
- if (ctx.getCustomTypeRegistry().hasCodec(enclosingType, beanClass)) {
- return beans;
- }
- if (isBean(beanClass, ctx)) {
- beans.add(beanClass);
- }
- LinkedHashSet<TypeRef<?>> typeRefs = new LinkedHashSet<>();
- List<Descriptor> descriptors = Descriptor.getDescriptors(beanClass);
+ Class<?> type = typeRef.getRawType();
TypeResolutionContext newCtx = ctx;
- for (Descriptor descriptor : descriptors) {
- TypeRef<?> typeRef = descriptor.getTypeRef();
- Class<?> rawType = typeRef.getRawType();
- typeRefs.add(descriptor.getTypeRef());
- typeRefs.addAll(getAllTypeArguments(typeRef));
- if (beanClass.isInterface() && typeRef.getRawType().isInterface() &&
!isContainer(rawType)) {
- newCtx = newCtx.withSynthesizedBeanType(typeRef.getRawType());
- }
- }
-
- for (TypeRef<?> typeToken : typeRefs) {
- Class<?> type = getRawType(typeToken);
- if (type == Optional.class) {
- TypeRef<?> elemType = getTypeArguments(typeToken).get(0);
- beans.addAll(listBeansRecursiveInclusive(elemType.getRawType(),
newCtx));
- } else if (isCollection(type)) {
- TypeRef<?> elementType = getElementType(typeToken);
- while (isContainer(elementType.getRawType())) {
- elementType = getElementType(elementType);
+ if (ctx.getCustomTypeRegistry().hasCodec(enclosingType, type)) {
+ return beans;
+ } else if (type == Optional.class) {
+ TypeRef<?> elemType = getTypeArguments(typeRef).get(0);
+ beans.addAll(listBeansRecursiveInclusive(elemType, newCtx));
+ } else if (isCollection(type) || Iterable.class == type) {
+ TypeRef<?> elementType = getElementType(typeRef);
+ beans.addAll(listBeansRecursiveInclusive(elementType, newCtx));
+ } else if (isMap(type)) {
+ Tuple2<TypeRef<?>, TypeRef<?>> mapKeyValueType =
getMapKeyValueType(typeRef);
+ TypeResolutionContext mapCtx = newCtx;
+ beans.addAll(listBeansRecursiveInclusive(mapKeyValueType.f0, mapCtx));
+ beans.addAll(listBeansRecursiveInclusive(mapKeyValueType.f1, mapCtx));
+ } else if (type.isArray()) {
+ Class<?> arrayComponent = getArrayComponent(type);
+ beans.addAll(listBeansRecursiveInclusive(TypeRef.of(arrayComponent),
newCtx));
+ } else if (isBean(type, newCtx)) {
+ List<Descriptor> descriptors = Descriptor.getDescriptors(type);
+ beans.add(type);
+ for (Descriptor descriptor : descriptors) {
+ ctx.checkNoCycle(typeRef);
+ TypeRef<?> propertyTypeRef = descriptor.getTypeRef();
+ Class<?> propertyType = propertyTypeRef.getRawType();
+ if (propertyType.isInterface()) {
+ newCtx = newCtx.withSynthesizedBeanType(propertyType);
}
- beans.addAll(
- listBeansRecursiveInclusive(
- elementType.getRawType(), newCtx.appendTypePath(elementType)));
- } else if (isMap(type)) {
- Tuple2<TypeRef<?>, TypeRef<?>> mapKeyValueType =
getMapKeyValueType(typeToken);
- TypeResolutionContext mapCtx =
- newCtx.appendTypePath(mapKeyValueType.f0, mapKeyValueType.f1);
-
beans.addAll(listBeansRecursiveInclusive(mapKeyValueType.f0.getRawType(),
mapCtx));
-
beans.addAll(listBeansRecursiveInclusive(mapKeyValueType.f1.getRawType(),
mapCtx));
- } else if (type.isArray()) {
- Class<?> arrayComponent = getArrayComponent(type);
- beans.addAll(
- listBeansRecursiveInclusive(arrayComponent,
newCtx.appendTypePath(arrayComponent)));
- } else if (isBean(type, newCtx)) {
- ctx.checkNoCycle(typeToken);
- beans.addAll(listBeansRecursiveInclusive(type,
newCtx.appendTypePath(typeToken)));
+ beans.addAll(listBeansRecursiveInclusive(propertyTypeRef,
newCtx.appendTypePath(typeRef)));
}
}
return beans;
diff --git
a/java/fory-format/src/main/java/org/apache/fory/format/encoder/Encoders.java
b/java/fory-format/src/main/java/org/apache/fory/format/encoder/Encoders.java
index 8103d19a..8a7bd6b3 100644
---
a/java/fory-format/src/main/java/org/apache/fory/format/encoder/Encoders.java
+++
b/java/fory-format/src/main/java/org/apache/fory/format/encoder/Encoders.java
@@ -656,6 +656,7 @@ public class Encoders {
if (TypeUtils.isBean(typeRef)) {
set.add(typeRef);
}
+ findBeanToken(typeRef, set);
} else {
Tuple2<TypeRef<?>, TypeRef<?>> tuple2 =
TypeUtils.getMapKeyValueType(typeRef);
if (TypeUtils.isBean(tuple2.f0)) {
diff --git
a/java/fory-format/src/test/java/org/apache/fory/format/encoder/GenericTypeTest.java
b/java/fory-format/src/test/java/org/apache/fory/format/encoder/GenericTypeTest.java
new file mode 100644
index 00000000..6649d3d6
--- /dev/null
+++
b/java/fory-format/src/test/java/org/apache/fory/format/encoder/GenericTypeTest.java
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fory.format.encoder;
+
+import lombok.Data;
+import org.apache.fory.format.row.binary.BinaryRow;
+import org.apache.fory.memory.MemoryBuffer;
+import org.apache.fory.memory.MemoryUtils;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class GenericTypeTest {
+
+ @Data
+ public static class TypedId<T> {
+ int id;
+
+ public TypedId() {}
+ }
+
+ @Data
+ public static class TestEntity {
+ TypedId<TestEntity> id;
+
+ public TestEntity() {}
+ }
+
+ @Test
+ public void testRecursiveGenericType() {
+ final TestEntity bean = new TestEntity();
+ bean.id = new TypedId<>();
+ bean.id.id = 42;
+ final RowEncoder<TestEntity> encoder = Encoders.bean(TestEntity.class);
+ final BinaryRow row = encoder.toRow(bean);
+ final MemoryBuffer buffer = MemoryUtils.wrap(row.toBytes());
+ row.pointTo(buffer, 0, buffer.size());
+ final TestEntity deserializedBean = encoder.fromRow(row);
+ Assert.assertEquals(deserializedBean, bean);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
