Skip to content

Commit

Permalink
efficient primitive item addition
Browse files Browse the repository at this point in the history
  • Loading branch information
bmarcaur committed Oct 22, 2024
1 parent cbf9fcf commit 8242023
Show file tree
Hide file tree
Showing 10 changed files with 175 additions and 15 deletions.

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

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 @@ -17,6 +17,7 @@
package com.palantir.conjure.java.types;

import com.palantir.conjure.java.ConjureAnnotations;
import com.palantir.conjure.java.lib.SafeLong;
import com.palantir.conjure.java.types.BeanGenerator.EnrichedField;
import com.palantir.conjure.java.util.Javadoc;
import com.palantir.conjure.java.util.Primitives;
Expand All @@ -28,6 +29,7 @@
import com.palantir.conjure.spec.PrimitiveType;
import com.palantir.conjure.spec.Type;
import com.palantir.conjure.visitor.TypeVisitor;
import com.squareup.javapoet.ArrayTypeName;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
import com.squareup.javapoet.FieldSpec;
Expand All @@ -43,6 +45,39 @@ public final class BeanBuilderAuxiliarySettersUtils {

private BeanBuilderAuxiliarySettersUtils() {}

public static MethodSpec.Builder createPrimitiveCollectionSetterBuilder(
String prefix,
EnrichedField enriched,
TypeMapper typeMapper,
ClassName returnClass,
SafetyEvaluator safetyEvaluator) {
FieldSpec field = enriched.poetSpec();
FieldDefinition definition = enriched.conjureDef();
Type type = definition.getType();
Type innerType = type.accept(TypeVisitor.LIST).getItemType();
TypeName boxedTypeName = typeMapper.getClassName(innerType);
TypeName innerTypeName;
// SafeLong is just a special case of long
if (boxedTypeName.equals(ClassName.get(SafeLong.class))) {
innerTypeName = ConjureAnnotations.withSafety(
TypeName.LONG, safetyEvaluator.getUsageTimeSafety(enriched.conjureDef()));
} else {
innerTypeName = ConjureAnnotations.withSafety(
Primitives.unbox(boxedTypeName), safetyEvaluator.getUsageTimeSafety(enriched.conjureDef()));
}

return MethodSpec.methodBuilder(prefix + StringUtils.capitalize(field.name))
.addJavadoc(Javadoc.render(definition.getDocs(), definition.getDeprecated())
.map(rendered -> CodeBlock.of("$L", rendered))
.orElseGet(() -> CodeBlock.builder().build()))
.addAnnotations(ConjureAnnotations.deprecation(definition.getDeprecated()))
.addModifiers(Modifier.PUBLIC)
// Var arg of the primitive type
.varargs()
.addParameter(Parameters.nonnullParameter(ArrayTypeName.of(innerTypeName), field.name))
.returns(returnClass);
}

public static MethodSpec.Builder createCollectionSetterBuilder(
String prefix,
EnrichedField enriched,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,32 @@ private MethodSpec createCollectionSetter(String prefix, EnrichedField enriched,
.build();
}

private MethodSpec createPrimitiveCollectionSetter(EnrichedField enriched, boolean override) {
FieldSpec field = enriched.poetSpec();
Type type = enriched.conjureDef().getType();

CollectionType collectionType = getCollectionType(type);
return BeanBuilderAuxiliarySettersUtils.createPrimitiveCollectionSetterBuilder(
"addAll", enriched, typeMapper, builderClass, safetyEvaluator)
.addAnnotations(ConjureAnnotations.override(override))
.addCode(verifyNotBuilt())
.addCode(CodeBlocks.statement(
"$1T.addAllTo$2L(this.$3N, $4L)",
ConjureCollections.class,
collectionType.getConjureCollectionType().getCollectionName(),
field.name,
Expressions.requireNonNull(
field.name, enriched.fieldName().get() + " cannot be null")))
.addStatement("return this")
.build();
/*
ConjureCollections.class,
spec.name,
Expressions.requireNonNull(
spec.name, enriched.fieldName().get() + " cannot be null"));
*/
}

@SuppressWarnings("checkstyle:CyclomaticComplexity")
private CodeBlock typeAwareAssignment(EnrichedField enriched, Type type, boolean shouldClearFirst) {
FieldSpec spec = enriched.poetSpec();
Expand Down Expand Up @@ -765,10 +791,19 @@ private List<MethodSpec> createAuxiliarySetters(EnrichedField enriched, boolean
Type type = enriched.conjureDef().getType();
Optional<LogSafety> safety = safetyEvaluator.getUsageTimeSafety(enriched.conjureDef());

ImmutableList.Builder<MethodSpec> builder = ImmutableList.builder();

if (type.accept(TypeVisitor.IS_LIST)) {
return ImmutableList.of(
CollectionType collectionType = getCollectionType(type);
if (collectionType.getConjureCollectionType().isPrimitiveCollection()
&& collectionType.useNonNullFactory()) {
builder.add(createPrimitiveCollectionSetter(enriched, override));
}

builder.add(
createCollectionSetter("addAll", enriched, override),
createItemSetter(enriched, type.accept(TypeVisitor.LIST).getItemType(), override, safety));
return builder.build();
}

if (type.accept(TypeVisitor.IS_SET)) {
Expand Down Expand Up @@ -820,7 +855,9 @@ private CodeBlock optionalAssignmentStatement(EnrichedField enriched, OptionalTy
private static final EnumSet<PrimitiveType.Value> OPTIONAL_PRIMITIVES =
EnumSet.of(PrimitiveType.Value.INTEGER, PrimitiveType.Value.DOUBLE, PrimitiveType.Value.BOOLEAN);

/** Check if the optionalType contains a primitive boolean, double or integer. */
/**
* Check if the optionalType contains a primitive boolean, double or integer.
*/
private boolean isPrimitiveOptional(OptionalType optionalType) {
return optionalType.getItemType().accept(TypeVisitor.IS_PRIMITIVE)
&& OPTIONAL_PRIMITIVES.contains(
Expand Down Expand Up @@ -1039,22 +1076,28 @@ public boolean useNonNullFactory() {
}

private enum ConjureCollectionType {
LIST("List"),
DOUBLE_LIST("DoubleList"),
INTEGER_LIST("IntegerList"),
BOOLEAN_LIST("BooleanList"),
SAFE_LONG_LIST("SafeLongList"),
SET("Set");
LIST("List", false),
DOUBLE_LIST("DoubleList", true),
INTEGER_LIST("IntegerList", true),
BOOLEAN_LIST("BooleanList", true),
SAFE_LONG_LIST("SafeLongList", true),
SET("Set", false);

private final String collectionName;
private final Boolean primitiveCollection;

ConjureCollectionType(String collectionName) {
ConjureCollectionType(String collectionName, Boolean primitiveCollection) {
this.collectionName = collectionName;
this.primitiveCollection = primitiveCollection;
}

public String getCollectionName() {
return collectionName;
}

public Boolean isPrimitiveCollection() {
return primitiveCollection;
}
}

private enum ConjureCollectionNullHandlingMode {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public long longValue() {
return longValue;
}

private static long check(long value) {
public static long check(long value) {
Preconditions.checkArgument(
MIN_SAFE_VALUE <= value && value <= MAX_SAFE_VALUE,
"number must be safely representable in javascript i.e. "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ public boolean addAll(int index, Collection<? extends Boolean> collection) {
return delegate.addAllAtIndex(index, target);
}

public void addAll(boolean... source) {
this.delegate.addAll(source);
}

@Override
public Boolean remove(int index) {
return delegate.removeAtIndex(index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.palantir.conjure.java.lib.SafeLong;
import com.palantir.logsafe.Preconditions;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -140,6 +141,12 @@ public static <T> Set<T> newNonNullSet(Iterable<? extends T> iterable) {
return set;
}

/**
* The following Conjure boxed list wrappers for the eclipse-collections [type]ArrayList are temporary (except
* ConjureSafeLongList). In eclipse-collections 12, a BoxedMutable[type]List will be released. Once available,
* Conjure[type]List should be replaced with that.
*/

// This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
public static List<Double> newNonNullDoubleList() {
return new ConjureDoubleList(new DoubleArrayList());
Expand All @@ -158,11 +165,14 @@ public static List<Double> newNonNullDoubleList(Iterable<Double> iterable) {
return doubleList;
}

/**
* The following Conjure boxed list wrappers for the eclipse-collections [type]ArrayList are temporary (except
* ConjureSafeLongList). In eclipse-collections 12, a BoxedMutable[type]List will be released. Once available,
* Conjure[type]List should be replaced with that.
*/
// This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
public static void addAllToDoubleList(Collection<Double> addTo, double[] elementsToAdd) {
if (addTo instanceof ConjureDoubleList) {
((ConjureDoubleList) addTo).addAll(elementsToAdd);
} else {
addAll(addTo, () -> Arrays.stream(elementsToAdd).iterator());
}
}

// This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
public static List<Integer> newNonNullIntegerList() {
Expand All @@ -182,6 +192,15 @@ public static List<Integer> newNonNullIntegerList(Iterable<Integer> iterable) {
return integerList;
}

// This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
public static void addAllToIntegerList(Collection<Integer> addTo, int[] elementsToAdd) {
if (addTo instanceof ConjureIntegerList) {
((ConjureIntegerList) addTo).addAll(elementsToAdd);
} else {
addAll(addTo, () -> Arrays.stream(elementsToAdd).iterator());
}
}

// This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
public static List<Boolean> newNonNullBooleanList() {
return new ConjureBooleanList(new BooleanArrayList());
Expand All @@ -200,6 +219,19 @@ public static List<Boolean> newNonNullBooleanList(Iterable<Boolean> iterable) {
return booleanList;
}

// This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
public static void addAllToBooleanList(Collection<Boolean> addTo, boolean[] elementsToAdd) {
if (addTo instanceof ConjureBooleanList) {
((ConjureBooleanList) addTo).addAll(elementsToAdd);
} else {
// Arrays.stream doesn't have a method for boolean, lol.
// Maybe because an array of booleans makes no sense?
for (boolean bool : elementsToAdd) {
addTo.add(bool);
}
}
}

// This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
public static List<SafeLong> newNonNullSafeLongList() {
return new ConjureSafeLongList(new LongArrayList());
Expand All @@ -217,4 +249,13 @@ public static List<SafeLong> newNonNullSafeLongList(Iterable<SafeLong> iterable)

return safeLongList;
}

// This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
public static void addAllToSafeLongList(Collection<SafeLong> addTo, long[] elementsToAdd) {
if (addTo instanceof ConjureSafeLongList) {
((ConjureSafeLongList) addTo).addAll(elementsToAdd);
} else {
addAll(addTo, Arrays.stream(elementsToAdd).boxed().map(SafeLong::of).toList());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ public boolean addAll(int index, Collection<? extends Double> collection) {
return delegate.addAllAtIndex(index, target);
}

public void addAll(double... source) {
this.delegate.addAll(source);
}

@Override
public Double remove(int index) {
return delegate.removeAtIndex(index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ public boolean addAll(int index, Collection<? extends Integer> collection) {
return delegate.addAllAtIndex(index, target);
}

public void addAll(int... source) {
this.delegate.addAll(source);
}

@Override
public Integer remove(int index) {
return delegate.removeAtIndex(index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ public boolean addAll(int index, Collection<? extends SafeLong> collection) {
return delegate.addAllAtIndex(index, target);
}

public void addAll(long... source) {
for (long value : source) {
// Doesn't use SafeLong creation because this causes unnecessary boxing
SafeLong.check(value);
}
this.delegate.addAll(source);
}

@Override
public SafeLong remove(int index) {
return SafeLong.of(delegate.removeAtIndex(index));
Expand Down

0 comments on commit 8242023

Please sign in to comment.