This is an automated email from the ASF dual-hosted git repository.
wangweipeng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-fury.git
The following commit(s) were added to refs/heads/main by this push:
new 5aba0c87 fix(java): fix immutable collection ref tracking (#1403)
5aba0c87 is described below
commit 5aba0c8780fb7fdff96a63ec60cde631eb107cb7
Author: Shawn Yang <shawn.ck.y...@gmail.com>
AuthorDate: Mon Mar 11 22:16:05 2024 +0800
fix(java): fix immutable collection ref tracking (#1403)
fix immutable collection ref tracking in #1401
---
.../ImmutableCollectionSerializersTest.java | 32 +++++++++
.../collection/GuavaCollectionSerializers.java | 29 ++------
.../collection/ImmutableCollectionSerializers.java | 3 -
.../test/java/org/apache/fury/FuryTestBase.java | 22 ++++++
.../collection/GuavaCollectionSerializersTest.java | 82 +++++++++++++---------
5 files changed, 108 insertions(+), 60 deletions(-)
diff --git
a/integration_tests/latest_jdk_tests/src/test/java/org/apache/fury/integration_tests/ImmutableCollectionSerializersTest.java
b/integration_tests/latest_jdk_tests/src/test/java/org/apache/fury/integration_tests/ImmutableCollectionSerializersTest.java
index d0e1a108..e31bafc0 100644
---
a/integration_tests/latest_jdk_tests/src/test/java/org/apache/fury/integration_tests/ImmutableCollectionSerializersTest.java
+++
b/integration_tests/latest_jdk_tests/src/test/java/org/apache/fury/integration_tests/ImmutableCollectionSerializersTest.java
@@ -24,9 +24,14 @@ import static
org.apache.fury.integration_tests.TestUtils.serDeCheck;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import lombok.AllArgsConstructor;
+import lombok.Data;
import org.apache.fury.Fury;
+import org.apache.fury.ThreadSafeFury;
+import org.apache.fury.config.Language;
import org.apache.fury.test.bean.CollectionFields;
import org.apache.fury.test.bean.MapFields;
+import org.testng.Assert;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
@@ -88,4 +93,31 @@ public class ImmutableCollectionSerializersTest {
collectionFields.map2 = Map.of("1", "2", "3", "4");
serDeCheck(fury, collectionFields);
}
+
+ @Data
+ @AllArgsConstructor
+ public static class Pojo {
+ List<List<Object>> data;
+ }
+
+ @DataProvider
+ public static Object[][] refTrackingAndCodegen() {
+ return new Object[][] {{false, false}, {true, false}, {false, true},
{true, true}};
+ }
+
+ @Test(dataProvider = "refTrackingAndCodegen")
+ void testNestedRefTracking(boolean trackingRef, boolean codegen) {
+ Pojo pojo = new Pojo(List.of(List.of(1, 2), List.of(2, 2)));
+ ThreadSafeFury fury =
+ Fury.builder()
+ .withLanguage(Language.JAVA)
+ .requireClassRegistration(false)
+ .withCodegen(codegen)
+ .withRefTracking(trackingRef)
+ .buildThreadSafeFury();
+
+ byte[] bytes = fury.serialize(pojo);
+ Pojo deserializedPojo = (Pojo) fury.deserialize(bytes);
+ Assert.assertEquals(deserializedPojo, pojo);
+ }
}
diff --git
a/java/fury-core/src/main/java/org/apache/fury/serializer/collection/GuavaCollectionSerializers.java
b/java/fury-core/src/main/java/org/apache/fury/serializer/collection/GuavaCollectionSerializers.java
index f6d0e889..532837ae 100644
---
a/java/fury-core/src/main/java/org/apache/fury/serializer/collection/GuavaCollectionSerializers.java
+++
b/java/fury-core/src/main/java/org/apache/fury/serializer/collection/GuavaCollectionSerializers.java
@@ -56,9 +56,7 @@ public class GuavaCollectionSerializers {
int size = buffer.readPositiveVarInt();
List list = new ArrayList<>();
xreadElements(fury, buffer, list, size);
- T immutableList = xnewInstance(list);
- fury.getRefResolver().reference(immutableList);
- return immutableList;
+ return xnewInstance(list);
}
protected abstract T xnewInstance(Collection collection);
@@ -82,7 +80,6 @@ public class GuavaCollectionSerializers {
public T onCollectionRead(Collection collection) {
Object[] elements = ((CollectionContainer) collection).elements;
ImmutableList list = ImmutableList.copyOf(elements);
- fury.getRefResolver().reference(list);
return (T) list;
}
@@ -133,9 +130,7 @@ public class GuavaCollectionSerializers {
@Override
public T onCollectionRead(Collection collection) {
Object[] elements = ((CollectionContainer) collection).elements;
- T t = (T) function.apply(elements);
- fury.getRefResolver().reference(t);
- return t;
+ return (T) function.apply(elements);
}
@Override
@@ -166,9 +161,7 @@ public class GuavaCollectionSerializers {
@Override
public T onCollectionRead(Collection collection) {
Object[] elements = ((CollectionContainer) collection).elements;
- T t = (T) ImmutableSet.copyOf(elements);
- fury.getRefResolver().reference(t);
- return t;
+ return (T) ImmutableSet.copyOf(elements);
}
@Override
@@ -208,9 +201,7 @@ public class GuavaCollectionSerializers {
public T onCollectionRead(Collection collection) {
SortedCollectionContainer data = (SortedCollectionContainer) collection;
Object[] elements = data.elements;
- T t = (T) new
ImmutableSortedSet.Builder<>(data.comparator).add(elements).build();
- fury.getRefResolver().reference(t);
- return t;
+ return (T) new
ImmutableSortedSet.Builder<>(data.comparator).add(elements).build();
}
}
@@ -240,9 +231,7 @@ public class GuavaCollectionSerializers {
for (int i = 0; i < size; i++) {
builder.put(keyArray[i], valueArray[i]);
}
- T t = (T) builder.build();
- fury.getRefResolver().reference(t);
- return t;
+ return (T) builder.build();
}
@Override
@@ -255,9 +244,7 @@ public class GuavaCollectionSerializers {
int size = buffer.readPositiveVarInt();
Map map = new HashMap();
xreadElements(fury, buffer, map, size);
- T immutableMap = xnewInstance(map);
- fury.getRefResolver().reference(immutableMap);
- return immutableMap;
+ return xnewInstance(map);
}
protected abstract T xnewInstance(Map map);
@@ -355,9 +342,7 @@ public class GuavaCollectionSerializers {
for (int i = 0; i < size; i++) {
builder.put(keyArray[i], valueArray[i]);
}
- T t = (T) builder.build();
- fury.getRefResolver().reference(t);
- return t;
+ return (T) builder.build();
}
}
diff --git
a/java/fury-core/src/main/java/org/apache/fury/serializer/collection/ImmutableCollectionSerializers.java
b/java/fury-core/src/main/java/org/apache/fury/serializer/collection/ImmutableCollectionSerializers.java
index 75c9ea80..5270aa78 100644
---
a/java/fury-core/src/main/java/org/apache/fury/serializer/collection/ImmutableCollectionSerializers.java
+++
b/java/fury-core/src/main/java/org/apache/fury/serializer/collection/ImmutableCollectionSerializers.java
@@ -130,7 +130,6 @@ public class ImmutableCollectionSerializers {
} else {
collection = Collections.unmodifiableList((List) collection);
}
- fury.getRefResolver().reference(collection);
return collection;
}
}
@@ -163,7 +162,6 @@ public class ImmutableCollectionSerializers {
} else {
collection = Collections.unmodifiableSet((HashSet) collection);
}
- fury.getRefResolver().reference(collection);
return collection;
}
}
@@ -200,7 +198,6 @@ public class ImmutableCollectionSerializers {
} else {
map = Collections.unmodifiableMap(map);
}
- fury.getRefResolver().reference(map);
return map;
}
}
diff --git a/java/fury-core/src/test/java/org/apache/fury/FuryTestBase.java
b/java/fury-core/src/test/java/org/apache/fury/FuryTestBase.java
index bf542b28..acf841fa 100644
--- a/java/fury-core/src/test/java/org/apache/fury/FuryTestBase.java
+++ b/java/fury-core/src/test/java/org/apache/fury/FuryTestBase.java
@@ -58,6 +58,28 @@ public abstract class FuryTestBase {
return new Object[][] {{false}, {true}};
}
+ @DataProvider
+ public static Object[][] trackingRefFury() {
+ return new Object[][] {
+ {
+ Fury.builder()
+ .withLanguage(Language.JAVA)
+ .withRefTracking(true)
+ .withCodegen(false)
+ .requireClassRegistration(false)
+ .build()
+ },
+ {
+ Fury.builder()
+ .withLanguage(Language.JAVA)
+ .withRefTracking(false)
+ .withCodegen(false)
+ .requireClassRegistration(false)
+ .build()
+ }
+ };
+ }
+
@DataProvider
public static Object[][] endian() {
return new Object[][] {{false}, {true}};
diff --git
a/java/fury-core/src/test/java/org/apache/fury/serializer/collection/GuavaCollectionSerializersTest.java
b/java/fury-core/src/test/java/org/apache/fury/serializer/collection/GuavaCollectionSerializersTest.java
index 5a974636..56d585c1 100644
---
a/java/fury-core/src/test/java/org/apache/fury/serializer/collection/GuavaCollectionSerializersTest.java
+++
b/java/fury-core/src/test/java/org/apache/fury/serializer/collection/GuavaCollectionSerializersTest.java
@@ -25,6 +25,9 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedSet;
+import java.util.List;
+import lombok.AllArgsConstructor;
+import lombok.Data;
import org.apache.fury.Fury;
import org.apache.fury.FuryTestBase;
import org.apache.fury.config.Language;
@@ -33,68 +36,63 @@ import org.testng.annotations.Test;
public class GuavaCollectionSerializersTest extends FuryTestBase {
- @Test
- public void testImmutableListSerializer() {
- serDe(getJavaFury(), ImmutableList.of(1));
+ @Test(dataProvider = "trackingRefFury")
+ public void testImmutableListSerializer(Fury fury) {
+ serDe(fury, ImmutableList.of(1));
Assert.assertEquals(
-
getJavaFury().getClassResolver().getSerializerClass(ImmutableList.of(1).getClass()),
+
fury.getClassResolver().getSerializerClass(ImmutableList.of(1).getClass()),
GuavaCollectionSerializers.ImmutableListSerializer.class);
- serDe(getJavaFury(), ImmutableList.of(1, 2));
+ serDe(fury, ImmutableList.of(1, 2));
Assert.assertEquals(
-
getJavaFury().getClassResolver().getSerializerClass(ImmutableList.of(1,
2).getClass()),
+ fury.getClassResolver().getSerializerClass(ImmutableList.of(1,
2).getClass()),
GuavaCollectionSerializers.RegularImmutableListSerializer.class);
}
- @Test
- public void testImmutableSetSerializer() {
- serDe(getJavaFury(), ImmutableSet.of(1));
+ @Test(dataProvider = "trackingRefFury")
+ public void testImmutableSetSerializer(Fury fury) {
+ serDe(fury, ImmutableSet.of(1));
Assert.assertEquals(
-
getJavaFury().getClassResolver().getSerializerClass(ImmutableSet.of(1).getClass()),
+
fury.getClassResolver().getSerializerClass(ImmutableSet.of(1).getClass()),
GuavaCollectionSerializers.ImmutableSetSerializer.class);
- serDe(getJavaFury(), ImmutableSet.of(1, 2));
+ serDe(fury, ImmutableSet.of(1, 2));
Assert.assertEquals(
- getJavaFury().getClassResolver().getSerializerClass(ImmutableSet.of(1,
2).getClass()),
+ fury.getClassResolver().getSerializerClass(ImmutableSet.of(1,
2).getClass()),
GuavaCollectionSerializers.ImmutableSetSerializer.class);
}
- @Test
- public void testImmutableSortedSetSerializer() {
- serDe(getJavaFury(), ImmutableSortedSet.of(1, 2));
+ @Test(dataProvider = "trackingRefFury")
+ public void testImmutableSortedSetSerializer(Fury fury) {
+ serDe(fury, ImmutableSortedSet.of(1, 2));
Assert.assertEquals(
-
getJavaFury().getClassResolver().getSerializerClass(ImmutableSortedSet.of(1,
2).getClass()),
+ fury.getClassResolver().getSerializerClass(ImmutableSortedSet.of(1,
2).getClass()),
GuavaCollectionSerializers.ImmutableSortedSetSerializer.class);
}
- @Test
- public void testImmutableMapSerializer() {
- serDe(getJavaFury(), ImmutableMap.of("k1", 1, "k2", 2));
+ @Test(dataProvider = "trackingRefFury")
+ public void testImmutableMapSerializer(Fury fury) {
+ serDe(fury, ImmutableMap.of("k1", 1, "k2", 2));
Assert.assertEquals(
- getJavaFury()
- .getClassResolver()
- .getSerializerClass(ImmutableMap.of("k1", 1, "k2", 2).getClass()),
+ fury.getClassResolver().getSerializerClass(ImmutableMap.of("k1", 1,
"k2", 2).getClass()),
GuavaCollectionSerializers.ImmutableMapSerializer.class);
}
- @Test
- public void testImmutableBiMapSerializer() {
- serDe(getJavaFury(), ImmutableBiMap.of("k1", 1));
+ @Test(dataProvider = "trackingRefFury")
+ public void testImmutableBiMapSerializer(Fury fury) {
+ serDe(fury, ImmutableBiMap.of("k1", 1));
Assert.assertEquals(
-
getJavaFury().getClassResolver().getSerializerClass(ImmutableBiMap.of("k1",
1).getClass()),
+ fury.getClassResolver().getSerializerClass(ImmutableBiMap.of("k1",
1).getClass()),
GuavaCollectionSerializers.ImmutableBiMapSerializer.class);
- serDe(getJavaFury(), ImmutableBiMap.of("k1", 1, "k2", 2));
+ serDe(fury, ImmutableBiMap.of("k1", 1, "k2", 2));
Assert.assertEquals(
- getJavaFury()
- .getClassResolver()
- .getSerializerClass(ImmutableBiMap.of("k1", 1, "k2",
2).getClass()),
+ fury.getClassResolver().getSerializerClass(ImmutableBiMap.of("k1", 1,
"k2", 2).getClass()),
GuavaCollectionSerializers.ImmutableBiMapSerializer.class);
}
- @Test
- public void testImmutableSortedMapSerializer() {
- serDe(getJavaFury(), ImmutableSortedMap.of("k1", 1, "k2", 2));
+ @Test(dataProvider = "trackingRefFury")
+ public void testImmutableSortedMapSerializer(Fury fury) {
+ serDe(fury, ImmutableSortedMap.of("k1", 1, "k2", 2));
Assert.assertEquals(
- getJavaFury()
- .getClassResolver()
+ fury.getClassResolver()
.getSerializerClass(ImmutableSortedMap.of("k1", 1, "k2",
2).getClass()),
GuavaCollectionSerializers.ImmutableSortedMapSerializer.class);
}
@@ -114,4 +112,18 @@ public class GuavaCollectionSerializersTest extends
FuryTestBase {
serDe(fury, ImmutableSet.of(1));
serDe(fury, ImmutableSet.of(1, 2, 3, 4));
}
+
+ @Data
+ @AllArgsConstructor
+ public static class Pojo {
+ List<List<Object>> data;
+ }
+
+ @Test(dataProvider = "javaFury")
+ void testNestedRefTracking(Fury fury) {
+ Pojo pojo = new Pojo(ImmutableList.of(ImmutableList.of(1, 2),
ImmutableList.of(2, 2)));
+ byte[] bytes = fury.serialize(pojo);
+ Pojo deserializedPojo = (Pojo) fury.deserialize(bytes);
+ System.out.println(deserializedPojo);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@fury.apache.org
For additional commands, e-mail: commits-h...@fury.apache.org
