Skip to content

Commit

Permalink
add serialization optimization for primitive collection types
Browse files Browse the repository at this point in the history
  • Loading branch information
bmarcaur committed Nov 25, 2024
1 parent 74269fc commit 42e78f4
Show file tree
Hide file tree
Showing 9 changed files with 345 additions and 14 deletions.
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-2386.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: '[FR] Add Serialization Optimization for Primitive Collection Types'
links:
- https://github.com/palantir/conjure-java/pull/2386

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,19 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.exc.InvalidNullException;
import com.palantir.conjure.java.lib.SafeLong;
import com.palantir.conjure.java.serialization.ObjectMappers;
import com.palantir.product.CovariantListExample;
import com.palantir.product.ListExample;
import com.palantir.product.PrimitiveExample;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import org.junit.jupiter.api.Test;

public class NonNullCollectionsTest {
private static final ObjectMapper objectMapper = ObjectMappers.newClientObjectMapper();
private static final ObjectMapper clientMapper = ObjectMappers.newClientObjectMapper();
private static final ObjectMapper serverMapper = ObjectMappers.newServerJsonMapper();

@Test
public void throwsNpe() {
Expand All @@ -55,7 +59,7 @@ public void testOptionalSerialization() throws JsonProcessingException {
.optionalItems(Collections.singleton(Optional.empty()))
.build();

assertThat(objectMapper.readValue(objectMapper.writeValueAsString(listExample), ListExample.class))
assertThat(clientMapper.readValue(clientMapper.writeValueAsString(listExample), ListExample.class))
.isEqualTo(listExample);

// non-null collections will add "contentNulls = Nulls.FAIL" to the JsonSetter annotation. This will cause deser
Expand All @@ -64,12 +68,31 @@ public void testOptionalSerialization() throws JsonProcessingException {
.addAllItems(Collections.singleton(Optional.empty()))
.build();
assertThatExceptionOfType(InvalidNullException.class)
.isThrownBy(() -> objectMapper.readValue(
objectMapper.writeValueAsString(covariantListExample), CovariantListExample.class));
.isThrownBy(() -> clientMapper.readValue(
clientMapper.writeValueAsString(covariantListExample), CovariantListExample.class));

// Similarly, setting a null in the builder also breaks
assertThatExceptionOfType(NullPointerException.class).isThrownBy(() -> CovariantListExample.builder()
.addAllItems(Collections.singleton(null))
.build());
}

@Test
public void testSerDeOptimizationRespectsConjureEmptyCollections() throws JsonProcessingException {
PrimitiveExample expected = PrimitiveExample.builder().build();
assertThat(clientMapper.writeValueAsString(expected))
.describedAs("Does not serialize any empty collections, even when optimizing for primitives")
.isEqualTo("{}");
}

@Test
public void testSerializationRoundtrip() throws JsonProcessingException {
PrimitiveExample expected = PrimitiveExample.builder()
.ints(List.of(1, 2, 3))
.doubles(List.of(1.1, 2.2, 3.3))
.longs(List.of(SafeLong.of(1L), SafeLong.of(2L)))
.build();
String serialized = serverMapper.writeValueAsString(expected);
assertThat(expected).isEqualTo(clientMapper.readValue(serialized, PrimitiveExample.class));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,21 @@ public void testObjectGenerator_excludeEmptyCollections() throws IOException {
assertThatFilesAreTheSame(files, REFERENCE_FILES_FOLDER);
}

@Test
public void testObjectGenerator_primitiveCollections() throws IOException {
ConjureDefinition def =
Conjure.parse(ImmutableList.of(new File("src/test/resources/primitive-collections.yml")));
List<Path> files = new GenerationCoordinator(
MoreExecutors.directExecutor(),
ImmutableSet.of(new ObjectGenerator(Options.builder()
.excludeEmptyCollections(true)
.nonNullCollections(true)
.build())))
.emit(def, tempDir);

assertThatFilesAreTheSame(files, REFERENCE_FILES_FOLDER);
}

@Test
public void testConjureImports() throws IOException {
ConjureDefinition conjure = Conjure.parse(ImmutableList.of(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
types:
definitions:
default-package: com.palantir.product
objects:
PrimitiveExample:
fields:
ints: list<integer>
doubles: list<double>
longs: list<safelong>
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,33 @@ private ConjureCollections() {
// cannot instantiate
}

/*
* This is bizarre. Allow me to explain...
*
* We do _not_ want to expose the Conjure*List types externally
* but we also want the optimizations they provide to make it thru
* to jackson for serialization. So the runtime type needs to be
* preserved while also not exposing the type :phew:.
*
* To achieve this we have to do some gymnastics surrounding the type
* system. We need this to return the type of the list given, but also
* return specific Conjure types when detected. This requires that we
* erase the type info, but we know this is safe because we are directly
* returning the same type which is by definition the identity function.
* Therefore the input List<T> is the same types as the output List<T>.
*/
public static <T> List<T> unmodifiableList(List<T> list) {
return Collections.unmodifiableList(list);
// Return the unmodifiable version of the Eclipse types
if (list instanceof ConjureIntegerList) {
return (List<T>) ((ConjureIntegerList) list).asUnmodifiable();
} else if (list instanceof ConjureDoubleList) {
return (List<T>) ((ConjureDoubleList) list).asUnmodifiable();
} else if (list instanceof ConjureSafeLongList) {
return (List<T>) ((ConjureSafeLongList) list).asUnmodifiable();
} else {
// Otherwise use the JDK types
return Collections.unmodifiableList(list);
}
}

@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,21 @@

package com.palantir.conjure.java.lib.internal;

import com.fasterxml.jackson.annotation.JsonValue;
import java.util.AbstractList;
import java.util.Collection;
import java.util.RandomAccess;
import org.eclipse.collections.impl.list.mutable.primitive.DoubleArrayList;
import org.eclipse.collections.api.list.primitive.MutableDoubleList;
import org.eclipse.collections.impl.utility.Iterate;

/**
* ConjureDoubleList is a boxed list wrapper for the eclipse-collections DoubleArrayList. In eclipse-collections 12,
* a BoxedMutableDoubleList will be released. Once available, ConjureDoubleList should be replaced with that.
*/
final class ConjureDoubleList extends AbstractList<Double> implements RandomAccess {
private final DoubleArrayList delegate;
private final MutableDoubleList delegate;

ConjureDoubleList(DoubleArrayList delegate) {
ConjureDoubleList(MutableDoubleList delegate) {
this.delegate = delegate;
}

Expand Down Expand Up @@ -69,4 +70,15 @@ public void clear() {
public Double set(int index, Double element) {
return delegate.set(index, element);
}

public ConjureDoubleList asUnmodifiable() {
return new ConjureDoubleList(delegate.asUnmodifiable());
}

// Cannot be named 'toArray' as that conflicts with the #toArray in AbstractList
// This is a serialization optimization that avoids boxing, but does copy
@JsonValue
double[] jacksonSerialize() {
return delegate.toArray();
}
}
Loading

0 comments on commit 42e78f4

Please sign in to comment.