diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java index 2a3c0d33f..eacbdda3f 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java @@ -218,6 +218,13 @@ public Builder primitiveItems(@Nonnull Iterable primitiveItems) { return this; } + public Builder primitiveItems(@Nonnull int[] primitiveItems) { + checkNotBuilt(); + this.primitiveItems = ConjureCollections.newNonNullIntegerList( + Preconditions.checkNotNull(primitiveItems, "primitiveItems cannot be null")); + return this; + } + public Builder addAllPrimitiveItems(@Nonnull Iterable primitiveItems) { checkNotBuilt(); ConjureCollections.addAllAndCheckNonNull( @@ -240,6 +247,13 @@ public Builder doubleItems(@Nonnull Iterable doubleItems) { return this; } + public Builder doubleItems(@Nonnull double[] doubleItems) { + checkNotBuilt(); + this.doubleItems = ConjureCollections.newNonNullDoubleList( + Preconditions.checkNotNull(doubleItems, "doubleItems cannot be null")); + return this; + } + public Builder addAllDoubleItems(@Nonnull Iterable doubleItems) { checkNotBuilt(); ConjureCollections.addAllAndCheckNonNull( diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeLongExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeLongExample.java index ba755c61a..fd3df5f25 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeLongExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeLongExample.java @@ -135,6 +135,13 @@ public Builder safeLongList(@Nonnull Iterable safeLongList) { return this; } + public Builder safeLongList(@Nonnull long[] safeLongList) { + checkNotBuilt(); + this.safeLongList = ConjureCollections.newNonNullSafeLongList( + Preconditions.checkNotNull(safeLongList, "safeLongList cannot be null")); + return this; + } + public Builder addAllSafeLongList(@Nonnull Iterable safeLongList) { checkNotBuilt(); ConjureCollections.addAllAndCheckNonNull( diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/Options.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/Options.java index ca5c878aa..55a4b8647 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/Options.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/Options.java @@ -169,6 +169,15 @@ default boolean externalFallbackTypes() { return false; } + /** + * When set, enables codegen for array setters for primitive optimized collections. + * This feature is experimental and subject to change. + */ + @Value.Default + default boolean primitiveCollectionArraySetters() { + return false; + } + Optional packagePrefix(); Optional apiVersion(); diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java index 103f51bff..b907e4c40 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java @@ -28,6 +28,7 @@ import com.google.errorprone.annotations.CheckReturnValue; import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.Options; +import com.palantir.conjure.java.lib.SafeLong; import com.palantir.conjure.java.lib.internal.ConjureCollections; import com.palantir.conjure.java.types.BeanGenerator.EnrichedField; import com.palantir.conjure.java.util.JavaNameSanitizer; @@ -57,6 +58,7 @@ import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import com.palantir.logsafe.exceptions.SafeIllegalStateException; import com.squareup.javapoet.AnnotationSpec; +import com.squareup.javapoet.ArrayTypeName; import com.squareup.javapoet.ClassName; import com.squareup.javapoet.CodeBlock; import com.squareup.javapoet.FieldSpec; @@ -671,6 +673,36 @@ private MethodSpec createCollectionSetter(String prefix, EnrichedField enriched, .build(); } + private MethodSpec createPrimitiveCollectionSetter(EnrichedField enriched) { + FieldSpec field = enriched.poetSpec(); + Type type = enriched.conjureDef().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())); + } + CollectionType collectionType = getCollectionType(type); + MethodSpec.Builder setterBuilder = BeanBuilderAuxiliarySettersUtils.publicSetter(enriched, builderClass) + .addParameter(Parameters.nonnullParameter(ArrayTypeName.of(innerTypeName), field.name)) + .addCode(verifyNotBuilt()) + .addCode(CodeBlocks.statement( + "this.$1N = $2T.newNonNull$3L($4L)", + enriched.poetSpec().name, + ConjureCollections.class, + collectionType.getConjureCollectionType().getCollectionName(), + Expressions.requireNonNull( + enriched.poetSpec().name, enriched.fieldName().get() + " cannot be null"))); + + return setterBuilder.addStatement("return this").build(); + } + @SuppressWarnings("checkstyle:CyclomaticComplexity") private CodeBlock typeAwareAssignment(EnrichedField enriched, Type type, boolean shouldClearFirst) { FieldSpec spec = enriched.poetSpec(); @@ -766,9 +798,19 @@ private List createAuxiliarySetters(EnrichedField enriched, boolean Optional safety = safetyEvaluator.getUsageTimeSafety(enriched.conjureDef()); if (type.accept(TypeVisitor.IS_LIST)) { - return ImmutableList.of( - createCollectionSetter("addAll", enriched, override), - createItemSetter(enriched, type.accept(TypeVisitor.LIST).getItemType(), override, safety)); + CollectionType collectionType = getCollectionType(type); + if (collectionType.getConjureCollectionType().isPrimitiveCollection() + && collectionType.useNonNullFactory() + && options.primitiveCollectionArraySetters()) { + return ImmutableList.of( + createPrimitiveCollectionSetter(enriched), + createCollectionSetter("addAll", enriched, override), + createItemSetter(enriched, type.accept(TypeVisitor.LIST).getItemType(), override, safety)); + } else { + return ImmutableList.of( + createCollectionSetter("addAll", enriched, override), + createItemSetter(enriched, type.accept(TypeVisitor.LIST).getItemType(), override, safety)); + } } if (type.accept(TypeVisitor.IS_SET)) { @@ -820,7 +862,9 @@ private CodeBlock optionalAssignmentStatement(EnrichedField enriched, OptionalTy private static final EnumSet 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( @@ -1039,22 +1083,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 { diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ObjectGeneratorTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ObjectGeneratorTests.java index f7a67c38e..95a47d773 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ObjectGeneratorTests.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ObjectGeneratorTests.java @@ -66,6 +66,7 @@ public void testObjectGenerator_allExamples() throws IOException { .excludeEmptyOptionals(true) .unionsWithUnknownValues(true) .jetbrainsContractAnnotations(true) + .primitiveCollectionArraySetters(true) .build()))) .emit(def, tempDir); diff --git a/conjure-java/src/main/java/com/palantir/conjure/java/cli/ConjureJavaCli.java b/conjure-java/src/main/java/com/palantir/conjure/java/cli/ConjureJavaCli.java index 101a7ee20..35cb88f3a 100644 --- a/conjure-java/src/main/java/com/palantir/conjure/java/cli/ConjureJavaCli.java +++ b/conjure-java/src/main/java/com/palantir/conjure/java/cli/ConjureJavaCli.java @@ -229,6 +229,12 @@ public static final class GenerateCommand implements Runnable { description = "Java external type imports are generated using their fallback type.") private boolean externalFallbackTypes; + @CommandLine.Option( + names = "--primitiveCollectionArraySetters", + defaultValue = "false", + description = "Ecodegen for array setters for primitive optimized collections.") + private boolean primitiveCollectionArraySetters; + @SuppressWarnings("unused") @CommandLine.Unmatched private List unmatchedOptions; @@ -297,6 +303,7 @@ CliConfiguration getConfiguration() { .excludeEmptyCollections(excludeEmptyCollections) .unionsWithUnknownValues(unionsWithUnknownValues) .externalFallbackTypes(externalFallbackTypes) + .primitiveCollectionArraySetters(primitiveCollectionArraySetters) .build()) .build(); } diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java index 92b3b7b81..457df38d3 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java @@ -18,6 +18,7 @@ import com.palantir.conjure.java.lib.SafeLong; import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashSet; @@ -140,11 +141,22 @@ public static Set newNonNullSet(Iterable 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 newNonNullDoubleList() { return new ConjureDoubleList(new DoubleArrayList()); } + // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set + public static List newNonNullDoubleList(double[] doubles) { + return new ConjureDoubleList(DoubleArrayList.newListWith(doubles.clone())); + } + // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static List newNonNullDoubleList(Iterable iterable) { List doubleList; @@ -158,17 +170,16 @@ public static List newNonNullDoubleList(Iterable 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 returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static List newNonNullIntegerList() { return new ConjureIntegerList(new IntArrayList()); } + // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set + public static List newNonNullIntegerList(int[] integers) { + return new ConjureIntegerList(IntArrayList.newListWith(integers.clone())); + } + // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static List newNonNullIntegerList(Iterable iterable) { List integerList; @@ -187,6 +198,11 @@ public static List newNonNullBooleanList() { return new ConjureBooleanList(new BooleanArrayList()); } + // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set + public static List newNonNullBooleanList(boolean[] booleans) { + return new ConjureBooleanList(BooleanArrayList.newListWith(booleans.clone())); + } + // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static List newNonNullBooleanList(Iterable iterable) { List booleanList; @@ -205,6 +221,19 @@ public static List newNonNullSafeLongList() { return new ConjureSafeLongList(new LongArrayList()); } + // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set + public static List newNonNullSafeLongList(long[] longs) { + long[] conjureCopy = longs.clone(); + for (long value : conjureCopy) { + if (!safeLongCheck(value)) { + throw new SafeIllegalArgumentException( + "number must be safely representable in javascript i.e. lie between -9007199254740991 and " + + "9007199254740991"); + } + } + return new ConjureSafeLongList(LongArrayList.newListWith(conjureCopy)); + } + // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static List newNonNullSafeLongList(Iterable iterable) { List safeLongList; @@ -217,4 +246,8 @@ public static List newNonNullSafeLongList(Iterable iterable) return safeLongList; } + + private static boolean safeLongCheck(long value) { + return SafeLong.MIN_VALUE.longValue() <= value && value <= SafeLong.MAX_VALUE.longValue(); + } } diff --git a/conjure-lib/src/test/java/com/palantir/conjure/java/lib/internal/ConjureCollectionsTest.java b/conjure-lib/src/test/java/com/palantir/conjure/java/lib/internal/ConjureCollectionsTest.java index 1e03e890e..446ca4f83 100644 --- a/conjure-lib/src/test/java/com/palantir/conjure/java/lib/internal/ConjureCollectionsTest.java +++ b/conjure-lib/src/test/java/com/palantir/conjure/java/lib/internal/ConjureCollectionsTest.java @@ -17,8 +17,10 @@ package com.palantir.conjure.java.lib.internal; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import com.palantir.conjure.java.lib.SafeLong; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import java.util.List; import org.junit.jupiter.api.Test; @@ -51,6 +53,17 @@ public void newNonNullDoubleList() { doubleList.clear(); assertThat(doubleList).hasSize(0); + + double[] rawList = new double[] {0.1, 0.2, 0.3}; + doubleList = ConjureCollections.newNonNullDoubleList(rawList); + assertThat(doubleList).hasSize(3); + assertThat(doubleList.get(0)).isEqualTo(0.1); + assertThat(doubleList.get(1)).isEqualTo(0.2); + assertThat(doubleList.get(2)).isEqualTo(0.3); + + // Check we made a copy of the array + rawList[0] = 0.4; + assertThat(doubleList.get(2)).isEqualTo(0.3); } @Test @@ -81,5 +94,19 @@ public void newNonNullSafeLongList() { safeLongList.clear(); assertThat(safeLongList).hasSize(0); + + long[] rawList = new long[] {1L, 2L, 3L}; + safeLongList = ConjureCollections.newNonNullSafeLongList(rawList); + assertThat(safeLongList).hasSize(3); + assertThat(safeLongList.get(0)).isEqualTo(SafeLong.of(1L)); + assertThat(safeLongList.get(1)).isEqualTo(SafeLong.of(2L)); + assertThat(safeLongList.get(2)).isEqualTo(SafeLong.of(3L)); + + rawList[0] = 42L; + assertThat(safeLongList.get(0)).isEqualTo(SafeLong.of(1L)); + + assertThatExceptionOfType(SafeIllegalArgumentException.class) + .isThrownBy(() -> ConjureCollections.newNonNullSafeLongList( + new long[] {1L, 2L, SafeLong.MAX_VALUE.longValue() + 1})); } }