Skip to content

Commit

Permalink
fix immutable collection ref tracking
Browse files Browse the repository at this point in the history
  • Loading branch information
@chaokunyang
chaokunyang committed Mar 11, 2024
1 parent 27b6249 commit 27b7c25
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 60 deletions.
32 changes: 32 additions & 0 deletions ...s/src/test/java/org/apache/fury/integration_tests/ImmutableCollectionSerializersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@
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;

Expand Down Expand Up @@ -88,4 +93,31 @@ public void testImmutableMapStruct() {
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);
}
}
29 changes: 7 additions & 22 deletions ...-core/src/main/java/org/apache/fury/serializer/collection/GuavaCollectionSerializers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ public T xread(MemoryBuffer buffer) {
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);
Expand All @@ -82,7 +80,6 @@ public Collection newCollection(MemoryBuffer buffer) {
public T onCollectionRead(Collection collection) {
Object[] elements = ((CollectionContainer) collection).elements;
ImmutableList list = ImmutableList.copyOf(elements);
fury.getRefResolver().reference(list);
return (T) list;
}

Expand Down Expand Up @@ -133,9 +130,7 @@ public Collection newCollection(MemoryBuffer buffer) {
@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
Expand Down Expand Up @@ -166,9 +161,7 @@ public Collection newCollection(MemoryBuffer buffer) {
@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
Expand Down Expand Up @@ -208,9 +201,7 @@ public Collection newCollection(MemoryBuffer buffer) {
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();
}
}

Expand Down Expand Up @@ -240,9 +231,7 @@ public T onMapRead(Map map) {
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
Expand All @@ -255,9 +244,7 @@ public T xread(MemoryBuffer buffer) {
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);
Expand Down Expand Up @@ -355,9 +342,7 @@ public T onMapRead(Map map) {
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();
}
}

Expand Down
3 changes: 0 additions & 3 deletions ...e/src/main/java/org/apache/fury/serializer/collection/ImmutableCollectionSerializers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ public Collection onCollectionRead(Collection collection) {
} else {
collection = Collections.unmodifiableList((List) collection);
}
fury.getRefResolver().reference(collection);
return collection;
}
}
Expand Down Expand Up @@ -163,7 +162,6 @@ public Collection onCollectionRead(Collection collection) {
} else {
collection = Collections.unmodifiableSet((HashSet) collection);
}
fury.getRefResolver().reference(collection);
return collection;
}
}
Expand Down Expand Up @@ -200,7 +198,6 @@ public Map onMapRead(Map map) {
} else {
map = Collections.unmodifiableMap(map);
}
fury.getRefResolver().reference(map);
return map;
}
}
Expand Down
22 changes: 22 additions & 0 deletions java/fury-core/src/test/java/org/apache/fury/FuryTestBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,28 @@ public static Object[][] referenceTrackingConfig() {
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}};
Expand Down
82 changes: 47 additions & 35 deletions ...e/src/test/java/org/apache/fury/serializer/collection/GuavaCollectionSerializersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
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;
Expand All @@ -33,68 +36,63 @@

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);
}
Expand All @@ -114,4 +112,18 @@ public void tesXlangSerialize() {
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(List.of(List.of(1, 2), List.of(2, 2)));
byte[] bytes = fury.serialize(pojo);
Pojo deserializedPojo = (Pojo) fury.deserialize(bytes);
System.out.println(deserializedPojo);
}
}

0 comments on commit 27b7c25

Please sign in to comment.