From b8294194c715adda743d40b21b73affbbc8e17af Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Wed, 1 Feb 2023 18:33:26 -0500 Subject: [PATCH 01/16] WIP --- .../palantir/conjure/java/ConjureTags.java | 14 +- .../conjure/java/types/AliasGenerator.java | 8 +- .../BeanBuilderAuxiliarySettersUtils.java | 5 +- .../java/types/BeanBuilderGenerator.java | 5 +- .../conjure/java/types/BeanGenerator.java | 11 +- .../conjure/java/types/SafetyEvaluator.java | 12 +- .../conjure/java/types/UnionGenerator.java | 10 +- .../conjure/java/util/SafetyUtils.java | 50 +++++ .../java/types/SafetyEvaluatorTest.java | 194 +++++++++++++++++- versions.lock | 62 +++--- versions.props | 2 +- 11 files changed, 302 insertions(+), 71 deletions(-) create mode 100644 conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/ConjureTags.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/ConjureTags.java index f4b88a0b1..6cf0ebc32 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/ConjureTags.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/ConjureTags.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableSet; import com.palantir.conjure.java.types.SafetyEvaluator; +import com.palantir.conjure.java.util.SafetyUtils; import com.palantir.conjure.spec.ArgumentDefinition; import com.palantir.conjure.spec.EndpointDefinition; import com.palantir.conjure.spec.LogSafety; @@ -80,8 +81,12 @@ public static Optional validateArgument(ArgumentDefinition argument, public static Optional safety(ArgumentDefinition argument) { validateTags(argument); - if (argument.getSafety().isPresent()) { - return argument.getSafety(); + + // Always prefer the external import's safety. If EI are supported, this is correct. If not, there won't be an + // annotation anywhere so it doesn't matter. + Optional argumentSafety = SafetyUtils.getSafety(argument); + if (argumentSafety.isPresent()) { + return argumentSafety; } Set tags = argument.getTags(); if (isSafe(tags)) { @@ -104,7 +109,8 @@ public static void validateTags(ArgumentDefinition argument) { isSafe(tags) ? "safe" : "unsafe", argument.getArgName())); } - if (argument.getSafety().isPresent()) { + Optional argumentSafety = SafetyUtils.getSafety(argument); + if (argumentSafety.isPresent()) { if (markerSafety.isPresent()) { throw new IllegalStateException(String.format( "Unexpected 'safety: %s' value in addition to a '%s' marker on argument '%s'", @@ -115,7 +121,7 @@ public static void validateTags(ArgumentDefinition argument) { if (isSafe(tags) || isUnsafe(tags)) { throw new IllegalStateException(String.format( "Unexpected 'safety: %s' value in addition to a '%s' tag on argument '%s'", - argument.getSafety().get().accept(DefFormatSafetyVisitor.INSTANCE), + argumentSafety.get().accept(DefFormatSafetyVisitor.INSTANCE), isSafe(tags) ? "safe" : "unsafe", argument.getArgName())); } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/AliasGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/AliasGenerator.java index 96874e950..4747f6e90 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/AliasGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/AliasGenerator.java @@ -23,6 +23,7 @@ import com.palantir.conjure.java.lib.SafeLong; import com.palantir.conjure.java.util.Javadoc; import com.palantir.conjure.java.util.Packages; +import com.palantir.conjure.java.util.SafetyUtils; import com.palantir.conjure.java.visitor.MoreVisitors; import com.palantir.conjure.spec.AliasDefinition; import com.palantir.conjure.spec.ExternalReference; @@ -64,13 +65,12 @@ public static JavaFile generateAliasType( TypeMapper typeMapper, SafetyEvaluator safetyEvaluator, AliasDefinition typeDef, Options options) { com.palantir.conjure.spec.TypeName prefixedTypeName = Packages.getPrefixedName(typeDef.getTypeName(), options.packagePrefix()); - TypeName aliasTypeName = - ConjureAnnotations.withSafety(typeMapper.getClassName(typeDef.getAlias()), typeDef.getSafety()); + Optional safety = SafetyUtils.getSafety(typeDef); + TypeName aliasTypeName = ConjureAnnotations.withSafety(typeMapper.getClassName(typeDef.getAlias()), safety); ClassName thisClass = ClassName.get(prefixedTypeName.getPackage(), prefixedTypeName.getName()); - Optional safety = typeDef.getSafety(); ImmutableList safetyAnnotations = ConjureAnnotations.safety(safety); - Optional computedSafety = safetyEvaluator.evaluate(typeDef.getAlias(), typeDef.getSafety()); + Optional computedSafety = safetyEvaluator.evaluate(typeDef.getAlias(), safety); ImmutableList computedSafetyAnnotations = ConjureAnnotations.safety(computedSafety); TypeSpec.Builder spec = TypeSpec.classBuilder(prefixedTypeName.getName()) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java index e6c61dbbc..a4986053b 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java @@ -19,6 +19,7 @@ import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.types.BeanGenerator.EnrichedField; import com.palantir.conjure.java.util.Javadoc; +import com.palantir.conjure.java.util.SafetyUtils; import com.palantir.conjure.java.visitor.DefaultTypeVisitor; import com.palantir.conjure.spec.FieldDefinition; import com.palantir.conjure.spec.MapType; @@ -57,7 +58,7 @@ public static MethodSpec.Builder createCollectionSetterBuilder( .addParameter(Parameters.nonnullParameter( widenParameterIfPossible(field.type, type, typeMapper), field.name, - enriched.conjureDef().getSafety())); + SafetyUtils.getSafety(enriched.conjureDef()))); } public static MethodSpec.Builder createOptionalSetterBuilder( @@ -68,7 +69,7 @@ public static MethodSpec.Builder createOptionalSetterBuilder( .addParameter(Parameters.nonnullParameter( typeMapper.getClassName(type.getItemType()), field.name, - enriched.conjureDef().getSafety())); + SafetyUtils.getSafety(enriched.conjureDef()))); } public static MethodSpec.Builder createItemSetterBuilder( 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 d10fe9852..5e0ed8ac2 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 @@ -27,6 +27,7 @@ import com.palantir.conjure.java.lib.internal.ConjureCollections; import com.palantir.conjure.java.types.BeanGenerator.EnrichedField; import com.palantir.conjure.java.util.JavaNameSanitizer; +import com.palantir.conjure.java.util.SafetyUtils; import com.palantir.conjure.java.util.TypeFunctions; import com.palantir.conjure.java.visitor.DefaultTypeVisitor; import com.palantir.conjure.java.visitor.DefaultableTypeVisitor; @@ -263,7 +264,7 @@ private MethodSpec createFromObject(Collection enrichedFields, bo private EnrichedField createField(FieldName fieldName, FieldDefinition field) { Type type = field.getType(); - TypeName typeName = ConjureAnnotations.withSafety(typeMapper.getClassName(type), field.getSafety()); + TypeName typeName = ConjureAnnotations.withSafety(typeMapper.getClassName(type), SafetyUtils.getSafety(field)); FieldSpec.Builder spec = FieldSpec.builder(typeName, JavaNameSanitizer.sanitize(fieldName), Modifier.PRIVATE); if (type.accept(TypeVisitor.IS_LIST) || type.accept(TypeVisitor.IS_SET) || type.accept(TypeVisitor.IS_MAP)) { spec.initializer("new $T<>()", type.accept(COLLECTION_CONCRETE_TYPE)); @@ -328,7 +329,7 @@ private MethodSpec createSetter( .addParameter(Parameters.nonnullParameter( BeanBuilderAuxiliarySettersUtils.widenParameterIfPossible(field.type, type, typeMapper), field.name, - enriched.conjureDef().getSafety())) + SafetyUtils.getSafety(enriched.conjureDef()))) .addCode(verifyNotBuilt()) .addCode(typeAwareAssignment(enriched, type, shouldClearFirst)); diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java index 5c4eb967f..3c8cef4e9 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java @@ -32,6 +32,7 @@ import com.palantir.conjure.java.util.JavaNameSanitizer; import com.palantir.conjure.java.util.Javadoc; import com.palantir.conjure.java.util.Packages; +import com.palantir.conjure.java.util.SafetyUtils; import com.palantir.conjure.java.util.TypeFunctions; import com.palantir.conjure.java.visitor.DefaultTypeVisitor; import com.palantir.conjure.java.visitor.MoreVisitors; @@ -461,7 +462,7 @@ private static MethodSpec createGetter( .addAnnotation(AnnotationSpec.builder(JsonProperty.class) .addMember("value", "$S", field.fieldName().get()) .build()) - .addAnnotations(ConjureAnnotations.safety(field.conjureDef().getSafety())) + .addAnnotations(ConjureAnnotations.safety(SafetyUtils.getSafety(field.conjureDef()))) .returns(field.poetSpec().type); Type conjureDefType = field.conjureDef().getType(); if (featureFlags.excludeEmptyOptionals()) { @@ -538,10 +539,10 @@ private static MethodSpec createStaticFactoryMethod(ImmutableList .addCode("return $L;", SINGLETON_INSTANCE_NAME); } else { builder.addCode("return builder()"); - fields.forEach(field -> builder.addParameter(ParameterSpec.builder( - getTypeNameWithoutOptional(field.poetSpec()), field.poetSpec().name) - .addAnnotations(ConjureAnnotations.safety(field.conjureDef().getSafety())) - .build())); + fields.forEach(field -> builder.addParameter( + ParameterSpec.builder(getTypeNameWithoutOptional(field.poetSpec()), field.poetSpec().name) + .addAnnotations(ConjureAnnotations.safety(SafetyUtils.getSafety(field.conjureDef()))) + .build())); // Follow order on adding methods on builder to comply with staged builders option if set sortedEnrichedFields(fields).map(EnrichedField::poetSpec).forEach(spec -> { if (isOptional(spec)) { diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java index 3e3dfa9fa..08a025a47 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java @@ -16,6 +16,7 @@ package com.palantir.conjure.java.types; +import com.palantir.conjure.java.util.SafetyUtils; import com.palantir.conjure.java.util.TypeFunctions; import com.palantir.conjure.spec.AliasDefinition; import com.palantir.conjure.spec.ConjureDefinition; @@ -89,7 +90,7 @@ private TypeDefinitionSafetyVisitor(Map definitionMap, @Override public Optional visitAlias(AliasDefinition value) { - return with(value.getTypeName(), () -> getSafety(value.getAlias(), value.getSafety())); + return with(value.getTypeName(), () -> getSafety(value.getAlias(), SafetyUtils.getSafety(value))); } @Override @@ -102,7 +103,7 @@ public Optional visitObject(ObjectDefinition value) { return with(value.getTypeName(), () -> { Optional safety = Optional.of(LogSafety.SAFE); for (FieldDefinition field : value.getFields()) { - safety = combine(safety, getSafety(field.getType(), field.getSafety())); + safety = combine(safety, getSafety(field.getType(), SafetyUtils.getSafety(field))); } return safety; }); @@ -113,7 +114,7 @@ public Optional visitUnion(UnionDefinition value) { return with(value.getTypeName(), () -> { Optional safety = UNKNOWN_UNION_VARINT_SAFETY; for (FieldDefinition variant : value.getUnion()) { - safety = combine(safety, getSafety(variant.getType(), variant.getSafety())); + safety = combine(safety, getSafety(variant.getType(), SafetyUtils.getSafety(variant))); } return safety; }); @@ -187,9 +188,8 @@ public Optional visitReference(TypeName value) { } @Override - public Optional visitExternal(ExternalReference _value) { - // External types have unknown safety for now - return Optional.empty(); + public Optional visitExternal(ExternalReference value) { + return value.getSafety(); } @Override diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/UnionGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/UnionGenerator.java index 1467c8158..e4035938e 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/UnionGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/UnionGenerator.java @@ -34,6 +34,7 @@ import com.palantir.conjure.java.util.JavaNameSanitizer; import com.palantir.conjure.java.util.Javadoc; import com.palantir.conjure.java.util.Packages; +import com.palantir.conjure.java.util.SafetyUtils; import com.palantir.conjure.java.util.StableCollectors; import com.palantir.conjure.java.util.TypeFunctions; import com.palantir.conjure.java.visitor.DefaultableTypeVisitor; @@ -104,7 +105,7 @@ public static JavaFile generateUnionType( .collect(StableCollectors.toLinkedMap( Function.identity(), entry -> ConjureAnnotations.withSafety( - typeMapper.getClassName(entry.getType()), entry.getSafety()))); + typeMapper.getClassName(entry.getType()), SafetyUtils.getSafety(entry)))); List fields = ImmutableList.of(FieldSpec.builder(baseClass, VALUE_FIELD_NAME, Modifier.PRIVATE, Modifier.FINAL) .build()); @@ -174,7 +175,7 @@ private static List generateStaticFactories( MethodSpec.Builder builder = MethodSpec.methodBuilder(JavaNameSanitizer.sanitize(memberName)) .addModifiers(Modifier.PUBLIC, Modifier.STATIC) .addParameter(ParameterSpec.builder(memberType, variableName) - .addAnnotations(ConjureAnnotations.safety(memberTypeDef.getSafety())) + .addAnnotations(ConjureAnnotations.safety(SafetyUtils.getSafety(memberTypeDef))) .build()) .addStatement( "return new $T(new $T($L))", @@ -309,8 +310,7 @@ private static List generateMemberVisitMethods(Map sortedStageNameTypePairs(Map new NameTypeMetadata( sanitizeUnknown(entry.getKey().getFieldName().get()), entry.getValue(), - entry.getKey().getSafety())) + SafetyUtils.getSafety(entry.getKey()))) .sorted(Comparator.comparing(p -> p.memberName)), Stream.of(NameTypeMetadata.UNKNOWN)); } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java new file mode 100644 index 000000000..8352604ae --- /dev/null +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java @@ -0,0 +1,50 @@ +/* + * (c) Copyright 2023 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.conjure.java.util; + +import com.palantir.conjure.java.visitor.MoreVisitors; +import com.palantir.conjure.spec.AliasDefinition; +import com.palantir.conjure.spec.ArgumentDefinition; +import com.palantir.conjure.spec.FieldDefinition; +import com.palantir.conjure.spec.LogSafety; +import java.util.Optional; + +public final class SafetyUtils { + + private SafetyUtils() {} + + public static Optional getSafety(AliasDefinition alias) { + if (alias.getAlias().accept(MoreVisitors.IS_EXTERNAL)) { + return alias.getAlias().accept(MoreVisitors.EXTERNAL).getSafety(); + } + return alias.getSafety(); + } + + public static Optional getSafety(FieldDefinition field) { + if (field.getType().accept(MoreVisitors.IS_EXTERNAL)) { + return field.getType().accept(MoreVisitors.EXTERNAL).getSafety(); + } + return field.getSafety(); + } + + public static Optional getSafety(ArgumentDefinition argument) { + if (argument.getType().accept(MoreVisitors.IS_EXTERNAL)) { + return argument.getType().accept(MoreVisitors.EXTERNAL).getSafety(); + } + return argument.getSafety(); + } +} diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java index 9f77efeef..59f9455fb 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java @@ -19,11 +19,13 @@ import static org.assertj.core.api.Assertions.assertThat; import com.google.common.collect.Iterables; +import com.palantir.conjure.defs.SafetyDeclarationRequirements; import com.palantir.conjure.defs.validator.ConjureDefinitionValidator; import com.palantir.conjure.spec.AliasDefinition; import com.palantir.conjure.spec.ConjureDefinition; import com.palantir.conjure.spec.Documentation; import com.palantir.conjure.spec.EnumDefinition; +import com.palantir.conjure.spec.ExternalReference; import com.palantir.conjure.spec.FieldDefinition; import com.palantir.conjure.spec.FieldName; import com.palantir.conjure.spec.LogSafety; @@ -35,7 +37,12 @@ import com.palantir.conjure.spec.TypeName; import com.palantir.conjure.spec.UnionDefinition; import java.util.Optional; +import java.util.stream.Stream; +import org.junit.jupiter.api.Named; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; class SafetyEvaluatorTest { private static final String PACKAGE = "package"; @@ -130,11 +137,85 @@ void testMapSafeKey() { .types(object) .types(SAFE_ALIAS) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + // there's an ABI break between 4.28.0 -> 4.36.0 + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(object)).isEmpty(); } + private static Stream providesExternalRefTypes_ImportTime() { + Type external = Type.external(ExternalReference.builder() + .externalReference(TypeName.of("Long", "java.lang")) + .fallback(Type.primitive(PrimitiveType.STRING)) + .safety(LogSafety.DO_NOT_LOG) + .build()); + return getTypes_SafetyAtImportTime(external); + } + + @ParameterizedTest + @MethodSource("providesExternalRefTypes_ImportTime") + void testExternalRefType_AtImportTime(TypeDefinition typeDefinition, ConjureDefinition conjureDef) { + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); + SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); + assertThat(evaluator.evaluate(typeDefinition)).hasValue(LogSafety.DO_NOT_LOG); + } + + private static Stream providesExternalRefTypes_NoSafety() { + Type external = Type.external(ExternalReference.builder() + .externalReference(TypeName.of("Long", "java.lang")) + .fallback(Type.primitive(PrimitiveType.STRING)) + .build()); + return getTypes_SafetyAtImportTime(external); + } + + @ParameterizedTest + @MethodSource("providesExternalRefTypes_NoSafety") + void testExternalRefType_NoSafety(TypeDefinition typeDefinition, ConjureDefinition conjureDef) { + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); + SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); + assertThat(evaluator.evaluate(typeDefinition)).isEmpty(); + } + + private static Stream providesExternalRefTypes_ImportAndUsageTime() { + Type external = Type.external(ExternalReference.builder() + .externalReference(TypeName.of("Long", "java.lang")) + .fallback(Type.primitive(PrimitiveType.STRING)) + .safety(LogSafety.DO_NOT_LOG) + .build()); + return getTypes_SafetyAtUsageTime(external); + } + + // We should never hit this case because validation should enforce that external imports declare safety + // at import time, not usage time + @ParameterizedTest + @MethodSource("providesExternalRefTypes_ImportAndUsageTime") + void testExternalRefType_AtImportAndUsageTime(TypeDefinition typeDefinition, ConjureDefinition conjureDef) { + // ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); + SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); + assertThat(evaluator.evaluate(typeDefinition)).hasValue(LogSafety.DO_NOT_LOG); + } + + private static Stream providesExternalRefTypes_OnlyAtUsageTime() { + Type external = Type.external(ExternalReference.builder() + .externalReference(TypeName.of("Long", "java.lang")) + .fallback(Type.primitive(PrimitiveType.STRING)) + .build()); + return getTypes_SafetyAtUsageTime(external); + } + + // We should never hit this case because validation should enforce that external imports declare safety + // at import time, not usage time + @ParameterizedTest + @MethodSource("providesExternalRefTypes_OnlyAtUsageTime") + void testExternalRefType_OnlyAtUsageTime(TypeDefinition typeDefinition, ConjureDefinition conjureDef) { + // ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); + SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); + assertThat(evaluator.evaluate(typeDefinition)).isEmpty(); + } + + // TODO: how will I test endpoints + // and the generation logic?? like an E2E test + @Test void testMapSafetyUnsafeValue() { TypeDefinition object = TypeDefinition.object(ObjectDefinition.builder() @@ -150,7 +231,7 @@ void testMapSafetyUnsafeValue() { .types(SAFE_ALIAS) .types(UNSAFE_ALIAS) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(object)).hasValue(LogSafety.UNSAFE); } @@ -170,7 +251,7 @@ void testMapSafetySafeKeyAndValue() { .types(SAFE_ALIAS) .types(UNSAFE_ALIAS) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(object)).hasValue(LogSafety.SAFE); } @@ -203,7 +284,7 @@ void testNested() { .types(secondObject) .types(UNSAFE_ALIAS) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(firstObject)).hasValue(LogSafety.UNSAFE); } @@ -222,7 +303,7 @@ void testPrimitiveSafety() { .build()) .build())) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(Iterables.getOnlyElement(conjureDef.getTypes()))) .hasValue(LogSafety.UNSAFE); @@ -241,7 +322,7 @@ void testNoSafety() { .build()) .build())) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(Iterables.getOnlyElement(conjureDef.getTypes()))) .isEmpty(); @@ -254,7 +335,7 @@ void testEmptyUnion() { .types(TypeDefinition.union( UnionDefinition.builder().typeName(FOO).build())) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(Iterables.getOnlyElement(conjureDef.getTypes()))) .as("No guarantees can be made about future union values, " @@ -275,7 +356,7 @@ void testUnsafeElementUnion() { .build()) .build())) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(Iterables.getOnlyElement(conjureDef.getTypes()))) .hasValue(LogSafety.UNSAFE); @@ -294,7 +375,7 @@ void testDoNotLogElementUnion() { .build()) .build())) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(Iterables.getOnlyElement(conjureDef.getTypes()))) .hasValue(LogSafety.DO_NOT_LOG); @@ -307,7 +388,7 @@ void testEmptyObject() { .types(TypeDefinition.object( ObjectDefinition.builder().typeName(FOO).build())) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(Iterables.getOnlyElement(conjureDef.getTypes()))) .hasValue(LogSafety.SAFE); @@ -320,9 +401,100 @@ void testEmptyEnum() { .types(TypeDefinition.enum_( EnumDefinition.builder().typeName(FOO).build())) .build(); - ConjureDefinitionValidator.validateAll(conjureDef); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(Iterables.getOnlyElement(conjureDef.getTypes()))) .hasValue(LogSafety.SAFE); } + + private static Stream getTypes_SafetyAtImportTime(Type externalReference) { + TypeDefinition objectType = TypeDefinition.object(ObjectDefinition.builder() + .typeName(FOO) + .fields(FieldDefinition.builder() + .fieldName(FieldName.of("import")) + .type(externalReference) + .docs(DOCS) + .build()) + .build()); + ConjureDefinition conjureObjectDef = ConjureDefinition.builder() + .version(1) + .types(objectType) + .types(UNSAFE_ALIAS) + .build(); + + TypeDefinition aliasType = TypeDefinition.alias( + AliasDefinition.builder().typeName(FOO).alias(externalReference).build()); + ConjureDefinition conjureAliasDef = ConjureDefinition.builder() + .version(1) + .types(aliasType) + .types(UNSAFE_ALIAS) + .build(); + + TypeDefinition unionType = TypeDefinition.union(UnionDefinition.builder() + .union(FieldDefinition.builder() + .fieldName(FieldName.of("importOne")) + .type(externalReference) + .docs(DOCS) + .build()) + .typeName(FOO) + .build()); + ConjureDefinition conjureUnionDef = ConjureDefinition.builder() + .version(1) + .types(unionType) + .types(UNSAFE_ALIAS) + .build(); + + return Stream.of( + Arguments.of(Named.of("Object", objectType), conjureObjectDef), + Arguments.of(Named.of("Alias", aliasType), conjureAliasDef), + Arguments.of(Named.of("Union", unionType), conjureUnionDef)); + } + + private static Stream getTypes_SafetyAtUsageTime(Type externalReference) { + TypeDefinition objectType = TypeDefinition.object(ObjectDefinition.builder() + .typeName(FOO) + .fields(FieldDefinition.builder() + .fieldName(FieldName.of("import")) + .type(externalReference) + .docs(DOCS) + .safety(LogSafety.UNSAFE) + .build()) + .build()); + ConjureDefinition conjureObjectDef = ConjureDefinition.builder() + .version(1) + .types(objectType) + .types(UNSAFE_ALIAS) + .build(); + + TypeDefinition aliasType = TypeDefinition.alias(AliasDefinition.builder() + .typeName(FOO) + .alias(externalReference) + .safety(LogSafety.UNSAFE) + .build()); + ConjureDefinition conjureAliasDef = ConjureDefinition.builder() + .version(1) + .types(aliasType) + .types(UNSAFE_ALIAS) + .build(); + + TypeDefinition unionType = TypeDefinition.union(UnionDefinition.builder() + .union(FieldDefinition.builder() + .fieldName(FieldName.of("importOne")) + .type(externalReference) + .safety(LogSafety.DO_NOT_LOG) + .docs(DOCS) + .build()) + .typeName(FOO) + .build()); + ConjureDefinition conjureUnionDef = ConjureDefinition.builder() + .version(1) + .types(unionType) + .types(UNSAFE_ALIAS) + .build(); + + return Stream.of( + Arguments.of(Named.of("Object", objectType), conjureObjectDef), + Arguments.of(Named.of("Alias", aliasType), conjureAliasDef), + Arguments.of(Named.of("Union", unionType), conjureUnionDef)); + } } diff --git a/versions.lock b/versions.lock index 0ea86c15c..dfcdde75e 100644 --- a/versions.lock +++ b/versions.lock @@ -1,27 +1,27 @@ # Run ./gradlew --write-locks to regenerate this file com.atlassian.commonmark:commonmark:0.12.1 (1 constraints: 36052a3b) -com.fasterxml.jackson.core:jackson-annotations:2.13.3 (10 constraints: 31b5d8fe) -com.fasterxml.jackson.core:jackson-core:2.13.3 (14 constraints: 9d2fef04) -com.fasterxml.jackson.core:jackson-databind:2.13.3 (27 constraints: 28156aa3) -com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.13.3 (2 constraints: e033da00) -com.fasterxml.jackson.dataformat:jackson-dataformat-smile:2.13.3 (1 constraints: 7d1c9aa4) -com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.13.3 (3 constraints: 3825d50f) -com.fasterxml.jackson.datatype:jackson-datatype-guava:2.13.3 (3 constraints: d841f79d) -com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.13.3 (4 constraints: 253fb28f) -com.fasterxml.jackson.datatype:jackson-datatype-joda:2.13.3 (2 constraints: 2b2b94af) -com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.13.3 (2 constraints: 2b2b94af) -com.fasterxml.jackson.module:jackson-module-afterburner:2.13.3 (2 constraints: 472b27b6) +com.fasterxml.jackson.core:jackson-annotations:2.14.1 (10 constraints: 5db5319b) +com.fasterxml.jackson.core:jackson-core:2.14.1 (14 constraints: 912f1cf0) +com.fasterxml.jackson.core:jackson-databind:2.14.1 (27 constraints: 5f1441dc) +com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.14.1 (2 constraints: df338d00) +com.fasterxml.jackson.dataformat:jackson-dataformat-smile:2.14.1 (1 constraints: 7d1c9aa4) +com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.14.1 (3 constraints: 3825d70f) +com.fasterxml.jackson.datatype:jackson-datatype-guava:2.14.1 (3 constraints: d841f79d) +com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.14.1 (4 constraints: 253fb48f) +com.fasterxml.jackson.datatype:jackson-datatype-joda:2.14.1 (2 constraints: 2b2b94af) +com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.14.1 (2 constraints: 2b2b94af) +com.fasterxml.jackson.module:jackson-module-afterburner:2.14.1 (2 constraints: 472b27b6) com.github.ben-manes.caffeine:caffeine:3.1.1 (10 constraints: 39b0aad1) com.google.auto:auto-common:1.2.1 (2 constraints: ec16041a) com.google.code.findbugs:jsr305:3.0.2 (20 constraints: a13e998f) com.google.errorprone:error_prone_annotations:2.7.1 (23 constraints: 98725532) com.google.guava:failureaccess:1.0.1 (1 constraints: 140ae1b4) -com.google.guava:guava:31.1-jre (33 constraints: fa4c49b2) +com.google.guava:guava:31.1-jre (33 constraints: 644e79e3) com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava (1 constraints: bd17c918) com.google.j2objc:j2objc-annotations:1.3 (1 constraints: b809eda0) com.palantir.common:streams:2.0.0 (1 constraints: 0405f535) -com.palantir.conjure:conjure-api-objects:4.28.0 (2 constraints: 6322d2e4) -com.palantir.conjure:conjure-generator-common:4.28.0 (2 constraints: 07149683) +com.palantir.conjure:conjure-api-objects:4.36.0 (2 constraints: 612297e4) +com.palantir.conjure:conjure-generator-common:4.36.0 (2 constraints: 05146783) com.palantir.conjure.java.api:errors:2.27.0 (6 constraints: cd6d3cc4) com.palantir.conjure.java.api:service-config:2.27.0 (4 constraints: 3948174d) com.palantir.conjure.java.api:ssl-config:2.27.0 (2 constraints: a5252239) @@ -40,13 +40,13 @@ com.palantir.goethe:goethe:0.8.0 (1 constraints: 0a050336) com.palantir.human-readable-types:human-readable-types:1.5.0 (1 constraints: 0805ff35) com.palantir.refreshable:refreshable:2.2.0 (2 constraints: 642473b7) com.palantir.ri:resource-identifier:2.5.0 (3 constraints: c124fff2) -com.palantir.safe-logging:logger:1.28.0 (20 constraints: f86238a7) -com.palantir.safe-logging:logger-slf4j:1.28.0 (1 constraints: 360e9750) -com.palantir.safe-logging:logger-spi:1.28.0 (2 constraints: 7b1e44b1) -com.palantir.safe-logging:preconditions:1.28.0 (25 constraints: e9beeaab) -com.palantir.safe-logging:safe-logging:1.28.0 (20 constraints: c34d7b98) +com.palantir.safe-logging:logger:3.0.0 (20 constraints: f86238a7) +com.palantir.safe-logging:logger-slf4j:3.0.0 (1 constraints: fe0d5342) +com.palantir.safe-logging:logger-spi:3.0.0 (2 constraints: 0b1e2f7a) +com.palantir.safe-logging:preconditions:3.0.0 (26 constraints: b9d1724d) +com.palantir.safe-logging:safe-logging:3.0.0 (21 constraints: eb5fa859) com.palantir.safethreadlocalrandom:safe-thread-local-random:0.1.0 (3 constraints: 933225f9) -com.palantir.syntactic-paths:syntactic-paths:0.6.1 (2 constraints: 99131761) +com.palantir.syntactic-paths:syntactic-paths:0.9.0 (2 constraints: 9b131f61) com.palantir.tokens:auth-tokens:3.14.0 (5 constraints: 52512cde) com.palantir.tracing:tracing:6.12.0 (8 constraints: c67ce633) com.palantir.tracing:tracing-api:6.12.0 (6 constraints: 0560a340) @@ -64,7 +64,7 @@ jakarta.annotation:jakarta.annotation-api:1.3.5 (3 constraints: ff2657ce) jakarta.validation:jakarta.validation-api:2.0.2 (4 constraints: 933c55bb) jakarta.ws.rs:jakarta.ws.rs-api:2.1.6 (12 constraints: 2fdea59b) joda-time:joda-time:2.10.14 (3 constraints: 1d29a8e3) -org.apache.commons:commons-lang3:3.12.0 (6 constraints: 9a5204a2) +org.apache.commons:commons-lang3:3.12.0 (5 constraints: ec3fa4cd) org.apache.httpcomponents.client5:httpclient5:5.1.3 (1 constraints: cd13956e) org.apache.httpcomponents.core5:httpcore5:5.1.4 (3 constraints: 6139a70d) org.apache.httpcomponents.core5:httpcore5-h2:5.1.3 (1 constraints: 3d13093c) @@ -79,24 +79,24 @@ org.jboss.logging:jboss-logging:3.4.1.Final (4 constraints: a24555a8) org.jboss.threads:jboss-threads:3.1.0.Final (2 constraints: 561a9b42) org.jboss.xnio:xnio-api:3.8.7.Final (2 constraints: 771a3146) org.jboss.xnio:xnio-nio:3.8.7.Final (1 constraints: c80dcb30) -org.jetbrains:annotations:23.0.0 (1 constraints: 37053c3b) +org.jetbrains:annotations:23.0.0 (2 constraints: 9d16fd24) org.mpierce.metrics.reservoir:hdrhistogram-metrics-reservoir:1.1.3 (1 constraints: 0d10f991) org.slf4j:slf4j-api:1.7.36 (29 constraints: d9af3053) org.slf4j:slf4j-simple:1.7.36 (1 constraints: 43054b3b) org.wildfly.client:wildfly-client-config:1.0.1.Final (1 constraints: 940c6308) org.wildfly.common:wildfly-common:1.5.4.Final (2 constraints: 741cfbf1) -org.yaml:snakeyaml:1.30 (1 constraints: 6c17f527) +org.yaml:snakeyaml:1.33 (1 constraints: 6f17f827) [Test dependencies] ch.qos.logback:logback-access:1.2.11 (1 constraints: b41148e2) ch.qos.logback:logback-classic:1.2.11 (4 constraints: 0133bde0) ch.qos.logback:logback-core:1.2.11 (3 constraints: 78284bbc) com.fasterxml:classmate:1.3.4 (1 constraints: 9b122713) -com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:2.13.3 (2 constraints: db2e8069) -com.fasterxml.jackson.jaxrs:jackson-jaxrs-cbor-provider:2.13.3 (1 constraints: 3f1977bb) -com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:2.13.3 (1 constraints: b70ea16b) -com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.13.3 (2 constraints: db2e8069) -com.fasterxml.jackson.module:jackson-module-parameter-names:2.13.3 (1 constraints: af0e575e) +com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:2.14.1 (2 constraints: d92e4169) +com.fasterxml.jackson.jaxrs:jackson-jaxrs-cbor-provider:2.14.1 (1 constraints: 3f1977bb) +com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:2.14.1 (1 constraints: b70ea16b) +com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.14.1 (2 constraints: d92e4169) +com.fasterxml.jackson.module:jackson-module-parameter-names:2.14.1 (1 constraints: af0e575e) com.github.stefanbirkner:system-lambda:1.2.0 (1 constraints: 0505f635) com.google.auto.value:auto-value:1.7.4 (1 constraints: 1f1221fb) com.google.auto.value:auto-value-annotations:1.7.4 (1 constraints: 640a29b9) @@ -108,7 +108,7 @@ com.netflix.concurrency-limits:concurrency-limits-core:0.2.2 (1 constraints: 7d1 com.netflix.feign:feign-core:8.18.0 (4 constraints: ec4c4333) com.netflix.feign:feign-jackson:8.18.0 (1 constraints: c718909e) com.netflix.feign:feign-jaxrs:8.18.0 (1 constraints: c718909e) -com.palantir.conjure:conjure-core:4.28.0 (1 constraints: 4005573b) +com.palantir.conjure:conjure-core:4.36.0 (1 constraints: 3f05553b) com.palantir.conjure.java.api:test-utils:2.27.0 (1 constraints: 3d05483b) com.palantir.conjure.java.runtime:conjure-java-annotations:7.33.0 (1 constraints: c318839e) com.palantir.conjure.java.runtime:conjure-java-jaxrs-client:7.33.0 (1 constraints: 3f055e3b) @@ -117,7 +117,7 @@ com.palantir.conjure.java.runtime:conjure-java-legacy-clients:7.33.0 (2 constrai com.palantir.conjure.java.runtime:conjure-java-retrofit2-client:7.33.0 (1 constraints: 3f055e3b) com.palantir.conjure.java.runtime:okhttp-clients:7.33.0 (1 constraints: 3c1acc0c) com.palantir.conjure.java.runtime:refresh-utils:7.33.0 (2 constraints: fe320abb) -com.palantir.safe-logging:preconditions-assertj:1.28.0 (1 constraints: 3d05453b) +com.palantir.safe-logging:preconditions-assertj:3.0.0 (1 constraints: 3d05453b) com.palantir.tracing:tracing-jersey:6.12.0 (1 constraints: 401988bb) com.palantir.tracing:tracing-okhttp3:6.12.0 (2 constraints: e72e1a8e) com.palantir.websecurity:dropwizard-web-security:1.1.0 (1 constraints: 0405f335) @@ -149,7 +149,7 @@ io.dropwizard.metrics:metrics-json:4.1.2 (1 constraints: e61063b8) io.dropwizard.metrics:metrics-jvm:4.1.2 (2 constraints: 261e8a46) io.dropwizard.metrics:metrics-logback:4.1.2 (1 constraints: 7f0edf4f) io.dropwizard.metrics:metrics-servlets:4.1.2 (1 constraints: 410d3a1f) -jakarta.activation:jakarta.activation-api:1.2.2 (4 constraints: 724514b4) +jakarta.activation:jakarta.activation-api:1.2.2 (4 constraints: 734594b4) jakarta.el:jakarta.el-api:3.0.3 (1 constraints: fe135465) jakarta.servlet:jakarta.servlet-api:4.0.3 (5 constraints: 52540445) jakarta.xml.bind:jakarta.xml.bind-api:2.3.3 (4 constraints: fb4f6a4f) diff --git a/versions.props b/versions.props index 6b44670a3..1d50b931b 100644 --- a/versions.props +++ b/versions.props @@ -7,7 +7,7 @@ com.palantir.common:streams = 2.0.0 com.palantir.conjure.java.api:* = 2.27.0 com.palantir.conjure.java.runtime:* = 7.33.0 com.palantir.conjure.verification:* = 0.19.0 -com.palantir.conjure:* = 4.28.0 +com.palantir.conjure:* = 4.36.0 com.palantir.dialogue:* = 3.67.0 com.palantir.ri:resource-identifier = 2.5.0 com.palantir.safe-logging:* = 1.28.0 From 6ba76b7fd2a1a305793a8a9a3793016db2f85a54 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Thu, 2 Feb 2023 14:30:13 -0500 Subject: [PATCH 02/16] adding E2E tests --- .../ExternalLongTestServiceEndpoints.java | 82 ++++ .../product/ExternalLongUnionExample.java | 354 ++++++++++++++++++ .../product/SafeExternalAliasExample.java | 49 +++ .../product/SafeExternalLongAlias.java | 48 +++ .../product/SafeExternalLongExample.java | 114 ++++++ .../UndertowExternalLongTestService.java | 13 + .../product/ExternalLongUnionExample.java | 344 +++++++++++++++++ .../product/SafeExternalAliasExample.java | 49 +++ .../product/SafeExternalLongAlias.java | 48 +++ .../product/SafeExternalLongExample.java | 116 ++++++ .../conjure/java/ConjureTagsTest.java | 68 ++++ .../conjure/java/UndertowServiceEteTest.java | 3 +- .../java/types/SafetyEvaluatorTest.java | 18 +- .../src/test/resources/example-types.yml | 17 + .../resources/external-long-test-service.yml | 20 + 15 files changed, 1334 insertions(+), 9 deletions(-) create mode 100644 conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongTestServiceEndpoints.java create mode 100644 conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java create mode 100644 conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java create mode 100644 conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongAlias.java create mode 100644 conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java create mode 100644 conjure-java-core/src/integrationInput/java/com/palantir/product/UndertowExternalLongTestService.java create mode 100644 conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ExternalLongUnionExample.java create mode 100644 conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java create mode 100644 conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongAlias.java create mode 100644 conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java create mode 100644 conjure-java-core/src/test/resources/external-long-test-service.yml diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongTestServiceEndpoints.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongTestServiceEndpoints.java new file mode 100644 index 000000000..9d5636493 --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongTestServiceEndpoints.java @@ -0,0 +1,82 @@ +package com.palantir.product; + +import com.google.common.collect.ImmutableList; +import com.palantir.conjure.java.undertow.lib.Deserializer; +import com.palantir.conjure.java.undertow.lib.Endpoint; +import com.palantir.conjure.java.undertow.lib.TypeMarker; +import com.palantir.conjure.java.undertow.lib.UndertowRuntime; +import com.palantir.conjure.java.undertow.lib.UndertowService; +import com.palantir.tokens.auth.AuthHeader; +import io.undertow.server.HttpHandler; +import io.undertow.server.HttpServerExchange; +import io.undertow.util.HttpString; +import io.undertow.util.Methods; +import io.undertow.util.StatusCodes; +import java.io.IOException; +import java.util.List; +import javax.annotation.processing.Generated; + +@Generated("com.palantir.conjure.java.services.UndertowServiceHandlerGenerator") +public final class ExternalLongTestServiceEndpoints implements UndertowService { + private final UndertowExternalLongTestService delegate; + + private ExternalLongTestServiceEndpoints(UndertowExternalLongTestService delegate) { + this.delegate = delegate; + } + + public static UndertowService of(UndertowExternalLongTestService delegate) { + return new ExternalLongTestServiceEndpoints(delegate); + } + + @Override + public List endpoints(UndertowRuntime runtime) { + return ImmutableList.of(new TestExternalLongArgEndpoint(runtime, delegate)); + } + + private static final class TestExternalLongArgEndpoint implements HttpHandler, Endpoint { + private final UndertowRuntime runtime; + + private final UndertowExternalLongTestService delegate; + + private final Deserializer deserializer; + + TestExternalLongArgEndpoint(UndertowRuntime runtime, UndertowExternalLongTestService delegate) { + this.runtime = runtime; + this.delegate = delegate; + this.deserializer = runtime.bodySerDe().deserializer(new TypeMarker() {}, this); + } + + @Override + public void handleRequest(HttpServerExchange exchange) throws IOException { + AuthHeader authHeader = runtime.auth().header(exchange); + Long externalLong = deserializer.deserialize(exchange); + delegate.testExternalLongArg(authHeader, externalLong); + exchange.setStatusCode(StatusCodes.NO_CONTENT); + } + + @Override + public HttpString method() { + return Methods.POST; + } + + @Override + public String template() { + return "/external-long/test"; + } + + @Override + public String serviceName() { + return "ExternalLongTestService"; + } + + @Override + public String name() { + return "testExternalLongArg"; + } + + @Override + public HttpHandler handler() { + return this; + } + } +} diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java new file mode 100644 index 000000000..21659a397 --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java @@ -0,0 +1,354 @@ +package com.palantir.product; + +import com.fasterxml.jackson.annotation.JsonAnyGetter; +import com.fasterxml.jackson.annotation.JsonAnySetter; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.annotation.JsonTypeName; +import com.fasterxml.jackson.annotation.JsonValue; +import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.function.BiFunction; +import java.util.function.Function; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import javax.annotation.processing.Generated; + +/** + * A union of a safe and unsafe external long. + */ +@Generated("com.palantir.conjure.java.types.UnionGenerator") +public final class ExternalLongUnionExample { + private final Base value; + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + private ExternalLongUnionExample(Base value) { + this.value = value; + } + + @JsonValue + private Base getValue() { + return value; + } + + public static ExternalLongUnionExample safeLong(@Safe long value) { + return new ExternalLongUnionExample(new SafeLongWrapper(value)); + } + + public static ExternalLongUnionExample unsafeLong(long value) { + return new ExternalLongUnionExample(new UnsafeLongWrapper(value)); + } + + public static ExternalLongUnionExample unknown(@Safe String type, Object value) { + switch (Preconditions.checkNotNull(type, "Type is required")) { + case "safeLong": + throw new SafeIllegalArgumentException( + "Unknown type cannot be created as the provided type is known: safeLong"); + case "unsafeLong": + throw new SafeIllegalArgumentException( + "Unknown type cannot be created as the provided type is known: unsafeLong"); + default: + return new ExternalLongUnionExample(new UnknownWrapper(type, Collections.singletonMap(type, value))); + } + } + + public T accept(Visitor visitor) { + return value.accept(visitor); + } + + @Override + public boolean equals(Object other) { + return this == other + || (other instanceof ExternalLongUnionExample && equalTo((ExternalLongUnionExample) other)); + } + + private boolean equalTo(ExternalLongUnionExample other) { + return this.value.equals(other.value); + } + + @Override + public int hashCode() { + return this.value.hashCode(); + } + + @Override + public String toString() { + return "ExternalLongUnionExample{value: " + value + '}'; + } + + public interface Visitor { + T visitSafeLong(@Safe long value); + + T visitUnsafeLong(long value); + + T visitUnknown(@Safe String unknownType, Object unknownValue); + + static SafeLongStageVisitorBuilder builder() { + return new VisitorBuilder(); + } + } + + private static final class VisitorBuilder + implements SafeLongStageVisitorBuilder, + UnsafeLongStageVisitorBuilder, + UnknownStageVisitorBuilder, + Completed_StageVisitorBuilder { + private Function<@Safe Long, T> safeLongVisitor; + + private Function unsafeLongVisitor; + + private BiFunction<@Safe String, Object, T> unknownVisitor; + + @Override + public UnsafeLongStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor) { + Preconditions.checkNotNull(safeLongVisitor, "safeLongVisitor cannot be null"); + this.safeLongVisitor = safeLongVisitor; + return this; + } + + @Override + public UnknownStageVisitorBuilder unsafeLong(@Nonnull Function unsafeLongVisitor) { + Preconditions.checkNotNull(unsafeLongVisitor, "unsafeLongVisitor cannot be null"); + this.unsafeLongVisitor = unsafeLongVisitor; + return this; + } + + @Override + public Completed_StageVisitorBuilder unknown(@Nonnull BiFunction<@Safe String, Object, T> unknownVisitor) { + Preconditions.checkNotNull(unknownVisitor, "unknownVisitor cannot be null"); + this.unknownVisitor = unknownVisitor; + return this; + } + + @Override + public Completed_StageVisitorBuilder unknown(@Nonnull Function<@Safe String, T> unknownVisitor) { + Preconditions.checkNotNull(unknownVisitor, "unknownVisitor cannot be null"); + this.unknownVisitor = (unknownType, _unknownValue) -> unknownVisitor.apply(unknownType); + return this; + } + + @Override + public Completed_StageVisitorBuilder throwOnUnknown() { + this.unknownVisitor = (unknownType, _unknownValue) -> { + throw new SafeIllegalArgumentException( + "Unknown variant of the 'ExternalLongUnionExample' union", + SafeArg.of("unknownType", unknownType)); + }; + return this; + } + + @Override + public Visitor build() { + final Function<@Safe Long, T> safeLongVisitor = this.safeLongVisitor; + final Function unsafeLongVisitor = this.unsafeLongVisitor; + final BiFunction<@Safe String, Object, T> unknownVisitor = this.unknownVisitor; + return new Visitor() { + @Override + public T visitSafeLong(long value) { + return safeLongVisitor.apply(value); + } + + @Override + public T visitUnsafeLong(long value) { + return unsafeLongVisitor.apply(value); + } + + @Override + public T visitUnknown(String unknownType, Object unknownValue) { + return unknownVisitor.apply(unknownType, unknownValue); + } + }; + } + } + + public interface SafeLongStageVisitorBuilder { + UnsafeLongStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor); + } + + public interface UnsafeLongStageVisitorBuilder { + UnknownStageVisitorBuilder unsafeLong(@Nonnull Function unsafeLongVisitor); + } + + public interface UnknownStageVisitorBuilder { + Completed_StageVisitorBuilder unknown(@Nonnull BiFunction<@Safe String, Object, T> unknownVisitor); + + Completed_StageVisitorBuilder unknown(@Nonnull Function<@Safe String, T> unknownVisitor); + + Completed_StageVisitorBuilder throwOnUnknown(); + } + + public interface Completed_StageVisitorBuilder { + Visitor build(); + } + + @JsonTypeInfo( + use = JsonTypeInfo.Id.NAME, + include = JsonTypeInfo.As.EXISTING_PROPERTY, + property = "type", + visible = true, + defaultImpl = UnknownWrapper.class) + @JsonSubTypes({@JsonSubTypes.Type(SafeLongWrapper.class), @JsonSubTypes.Type(UnsafeLongWrapper.class)}) + @JsonIgnoreProperties(ignoreUnknown = true) + private interface Base { + T accept(Visitor visitor); + } + + @JsonTypeName("safeLong") + private static final class SafeLongWrapper implements Base { + private final long value; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + private SafeLongWrapper(@JsonSetter("safeLong") @Nonnull long value) { + Preconditions.checkNotNull(value, "safeLong cannot be null"); + this.value = value; + } + + @JsonProperty(value = "type", index = 0) + private String getType() { + return "safeLong"; + } + + @JsonProperty("safeLong") + private long getValue() { + return value; + } + + @Override + public T accept(Visitor visitor) { + return visitor.visitSafeLong(value); + } + + @Override + public boolean equals(@Nullable Object other) { + return this == other || (other instanceof SafeLongWrapper && equalTo((SafeLongWrapper) other)); + } + + private boolean equalTo(SafeLongWrapper other) { + return this.value == other.value; + } + + @Override + public int hashCode() { + return Long.hashCode(this.value); + } + + @Override + public String toString() { + return "SafeLongWrapper{value: " + value + '}'; + } + } + + @JsonTypeName("unsafeLong") + private static final class UnsafeLongWrapper implements Base { + private final long value; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + private UnsafeLongWrapper(@JsonSetter("unsafeLong") @Nonnull long value) { + Preconditions.checkNotNull(value, "unsafeLong cannot be null"); + this.value = value; + } + + @JsonProperty(value = "type", index = 0) + private String getType() { + return "unsafeLong"; + } + + @JsonProperty("unsafeLong") + private long getValue() { + return value; + } + + @Override + public T accept(Visitor visitor) { + return visitor.visitUnsafeLong(value); + } + + @Override + public boolean equals(@Nullable Object other) { + return this == other || (other instanceof UnsafeLongWrapper && equalTo((UnsafeLongWrapper) other)); + } + + private boolean equalTo(UnsafeLongWrapper other) { + return this.value == other.value; + } + + @Override + public int hashCode() { + return Long.hashCode(this.value); + } + + @Override + public String toString() { + return "UnsafeLongWrapper{value: " + value + '}'; + } + } + + private static final class UnknownWrapper implements Base { + private final String type; + + private final Map value; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + private UnknownWrapper(@JsonProperty("type") String type) { + this(type, new HashMap()); + } + + private UnknownWrapper(@Nonnull String type, @Nonnull Map value) { + Preconditions.checkNotNull(type, "type cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + this.type = type; + this.value = value; + } + + @JsonProperty + private String getType() { + return type; + } + + @JsonAnyGetter + private Map getValue() { + return value; + } + + @JsonAnySetter + private void put(String key, Object val) { + value.put(key, val); + } + + @Override + public T accept(Visitor visitor) { + return visitor.visitUnknown(type, value.get(type)); + } + + @Override + public boolean equals(@Nullable Object other) { + return this == other || (other instanceof UnknownWrapper && equalTo((UnknownWrapper) other)); + } + + private boolean equalTo(UnknownWrapper other) { + return this.type.equals(other.type) && this.value.equals(other.value); + } + + @Override + public int hashCode() { + int hash = 1; + hash = 31 * hash + this.type.hashCode(); + hash = 31 * hash + this.value.hashCode(); + return hash; + } + + @Override + public String toString() { + return "UnknownWrapper{type: " + type + ", value: " + value + '}'; + } + } +} diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java new file mode 100644 index 000000000..95129b69e --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java @@ -0,0 +1,49 @@ +package com.palantir.product; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonValue; +import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; +import javax.annotation.Nonnull; +import javax.annotation.processing.Generated; + +@Safe +@Generated("com.palantir.conjure.java.types.AliasGenerator") +public final class SafeExternalAliasExample { + private final SafeExternalLongAlias value; + + private SafeExternalAliasExample(@Nonnull SafeExternalLongAlias value) { + this.value = Preconditions.checkNotNull(value, "value cannot be null"); + } + + @JsonValue + public SafeExternalLongAlias get() { + return value; + } + + @Override + public String toString() { + return value.toString(); + } + + @Override + public boolean equals(Object other) { + return this == other + || (other instanceof SafeExternalAliasExample + && this.value.equals(((SafeExternalAliasExample) other).value)); + } + + @Override + public int hashCode() { + return value.hashCode(); + } + + public static SafeExternalAliasExample valueOf(String value) { + return of(SafeExternalLongAlias.valueOf(value)); + } + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public static SafeExternalAliasExample of(@Nonnull SafeExternalLongAlias value) { + return new SafeExternalAliasExample(value); + } +} diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongAlias.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongAlias.java new file mode 100644 index 000000000..57d90260b --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongAlias.java @@ -0,0 +1,48 @@ +package com.palantir.product; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonValue; +import com.palantir.logsafe.Safe; +import javax.annotation.processing.Generated; + +@Safe +@Generated("com.palantir.conjure.java.types.AliasGenerator") +public final class SafeExternalLongAlias { + private final long value; + + private SafeExternalLongAlias(long value) { + this.value = value; + } + + @JsonValue + @Safe + public long get() { + return value; + } + + @Override + public String toString() { + return String.valueOf(value); + } + + @Override + public boolean equals(Object other) { + return this == other + || (other instanceof SafeExternalLongAlias && this.value == ((SafeExternalLongAlias) other).value); + } + + @Override + public int hashCode() { + return Long.hashCode(value); + } + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public static SafeExternalLongAlias valueOf(@Safe String value) { + return of(Long.valueOf(value)); + } + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public static SafeExternalLongAlias of(@Safe long value) { + return new SafeExternalLongAlias(value); + } +} diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java new file mode 100644 index 000000000..fee2b2945 --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java @@ -0,0 +1,114 @@ +package com.palantir.product; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; +import java.util.ArrayList; +import java.util.List; +import javax.annotation.Nullable; +import javax.annotation.processing.Generated; + +@Safe +@JsonDeserialize(builder = SafeExternalLongExample.Builder.class) +@Generated("com.palantir.conjure.java.types.BeanGenerator") +public final class SafeExternalLongExample { + private final long safeExternalLongValue; + + private SafeExternalLongExample(long safeExternalLongValue) { + this.safeExternalLongValue = safeExternalLongValue; + } + + @JsonProperty("safeExternalLongValue") + @Safe + public long getSafeExternalLongValue() { + return this.safeExternalLongValue; + } + + @Override + public boolean equals(@Nullable Object other) { + return this == other || (other instanceof SafeExternalLongExample && equalTo((SafeExternalLongExample) other)); + } + + private boolean equalTo(SafeExternalLongExample other) { + return this.safeExternalLongValue == other.safeExternalLongValue; + } + + @Override + public int hashCode() { + return Long.hashCode(this.safeExternalLongValue); + } + + @Override + public String toString() { + return "SafeExternalLongExample{safeExternalLongValue: " + safeExternalLongValue + '}'; + } + + public static SafeExternalLongExample of(@Safe long safeExternalLongValue) { + return builder().safeExternalLongValue(safeExternalLongValue).build(); + } + + public static Builder builder() { + return new Builder(); + } + + @Generated("com.palantir.conjure.java.types.BeanBuilderGenerator") + public static final class Builder { + boolean _buildInvoked; + + private long safeExternalLongValue; + + private boolean _safeExternalLongValueInitialized = false; + + private Builder() {} + + public Builder from(SafeExternalLongExample other) { + checkNotBuilt(); + safeExternalLongValue(other.getSafeExternalLongValue()); + return this; + } + + @JsonSetter("safeExternalLongValue") + public Builder safeExternalLongValue(@Safe long safeExternalLongValue) { + checkNotBuilt(); + this.safeExternalLongValue = safeExternalLongValue; + this._safeExternalLongValueInitialized = true; + return this; + } + + private void validatePrimitiveFieldsHaveBeenInitialized() { + List missingFields = null; + missingFields = + addFieldIfMissing(missingFields, _safeExternalLongValueInitialized, "safeExternalLongValue"); + if (missingFields != null) { + throw new SafeIllegalArgumentException( + "Some required fields have not been set", SafeArg.of("missingFields", missingFields)); + } + } + + private static List addFieldIfMissing(List prev, boolean initialized, String fieldName) { + List missingFields = prev; + if (!initialized) { + if (missingFields == null) { + missingFields = new ArrayList<>(1); + } + missingFields.add(fieldName); + } + return missingFields; + } + + public SafeExternalLongExample build() { + checkNotBuilt(); + this._buildInvoked = true; + validatePrimitiveFieldsHaveBeenInitialized(); + return new SafeExternalLongExample(safeExternalLongValue); + } + + private void checkNotBuilt() { + Preconditions.checkState(!_buildInvoked, "Build has already been called"); + } + } +} diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/UndertowExternalLongTestService.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/UndertowExternalLongTestService.java new file mode 100644 index 000000000..8b289d352 --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/UndertowExternalLongTestService.java @@ -0,0 +1,13 @@ +package com.palantir.product; + +import com.palantir.logsafe.DoNotLog; +import com.palantir.tokens.auth.AuthHeader; +import javax.annotation.processing.Generated; + +@Generated("com.palantir.conjure.java.services.UndertowServiceInterfaceGenerator") +public interface UndertowExternalLongTestService { + /** + * @apiNote {@code POST /external-long/test} + */ + void testExternalLongArg(AuthHeader authHeader, @DoNotLog long externalLong); +} diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ExternalLongUnionExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ExternalLongUnionExample.java new file mode 100644 index 000000000..624442ef9 --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ExternalLongUnionExample.java @@ -0,0 +1,344 @@ +package test.prefix.com.palantir.product; + +import com.fasterxml.jackson.annotation.JsonAnyGetter; +import com.fasterxml.jackson.annotation.JsonAnySetter; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.annotation.JsonTypeName; +import com.fasterxml.jackson.annotation.JsonValue; +import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Function; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import javax.annotation.processing.Generated; + +/** + * A union of a safe and unsafe external long. + */ +@Generated("com.palantir.conjure.java.types.UnionGenerator") +public final class ExternalLongUnionExample { + private final Base value; + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + private ExternalLongUnionExample(Base value) { + this.value = value; + } + + @JsonValue + private Base getValue() { + return value; + } + + public static ExternalLongUnionExample safeLong(@Safe long value) { + return new ExternalLongUnionExample(new SafeLongWrapper(value)); + } + + public static ExternalLongUnionExample unsafeLong(long value) { + return new ExternalLongUnionExample(new UnsafeLongWrapper(value)); + } + + public static ExternalLongUnionExample unknown(@Safe String type, Object value) { + switch (Preconditions.checkNotNull(type, "Type is required")) { + case "safeLong": + throw new SafeIllegalArgumentException( + "Unknown type cannot be created as the provided type is known: safeLong"); + case "unsafeLong": + throw new SafeIllegalArgumentException( + "Unknown type cannot be created as the provided type is known: unsafeLong"); + default: + return new ExternalLongUnionExample(new UnknownWrapper(type, Collections.singletonMap(type, value))); + } + } + + public T accept(Visitor visitor) { + return value.accept(visitor); + } + + @Override + public boolean equals(Object other) { + return this == other + || (other instanceof ExternalLongUnionExample && equalTo((ExternalLongUnionExample) other)); + } + + private boolean equalTo(ExternalLongUnionExample other) { + return this.value.equals(other.value); + } + + @Override + public int hashCode() { + return this.value.hashCode(); + } + + @Override + public String toString() { + return "ExternalLongUnionExample{value: " + value + '}'; + } + + public interface Visitor { + T visitSafeLong(@Safe long value); + + T visitUnsafeLong(long value); + + T visitUnknown(@Safe String unknownType); + + static SafeLongStageVisitorBuilder builder() { + return new VisitorBuilder(); + } + } + + private static final class VisitorBuilder + implements SafeLongStageVisitorBuilder, + UnsafeLongStageVisitorBuilder, + UnknownStageVisitorBuilder, + Completed_StageVisitorBuilder { + private Function<@Safe Long, T> safeLongVisitor; + + private Function unsafeLongVisitor; + + private Function unknownVisitor; + + @Override + public UnsafeLongStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor) { + Preconditions.checkNotNull(safeLongVisitor, "safeLongVisitor cannot be null"); + this.safeLongVisitor = safeLongVisitor; + return this; + } + + @Override + public UnknownStageVisitorBuilder unsafeLong(@Nonnull Function unsafeLongVisitor) { + Preconditions.checkNotNull(unsafeLongVisitor, "unsafeLongVisitor cannot be null"); + this.unsafeLongVisitor = unsafeLongVisitor; + return this; + } + + @Override + public Completed_StageVisitorBuilder unknown(@Nonnull Function unknownVisitor) { + Preconditions.checkNotNull(unknownVisitor, "unknownVisitor cannot be null"); + this.unknownVisitor = unknownVisitor; + return this; + } + + @Override + public Completed_StageVisitorBuilder throwOnUnknown() { + this.unknownVisitor = unknownType -> { + throw new SafeIllegalArgumentException( + "Unknown variant of the 'ExternalLongUnionExample' union", + SafeArg.of("unknownType", unknownType)); + }; + return this; + } + + @Override + public Visitor build() { + final Function<@Safe Long, T> safeLongVisitor = this.safeLongVisitor; + final Function unsafeLongVisitor = this.unsafeLongVisitor; + final Function unknownVisitor = this.unknownVisitor; + return new Visitor() { + @Override + public T visitSafeLong(long value) { + return safeLongVisitor.apply(value); + } + + @Override + public T visitUnsafeLong(long value) { + return unsafeLongVisitor.apply(value); + } + + @Override + public T visitUnknown(String value) { + return unknownVisitor.apply(value); + } + }; + } + } + + public interface SafeLongStageVisitorBuilder { + UnsafeLongStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor); + } + + public interface UnsafeLongStageVisitorBuilder { + UnknownStageVisitorBuilder unsafeLong(@Nonnull Function unsafeLongVisitor); + } + + public interface UnknownStageVisitorBuilder { + Completed_StageVisitorBuilder unknown(@Nonnull Function unknownVisitor); + + Completed_StageVisitorBuilder throwOnUnknown(); + } + + public interface Completed_StageVisitorBuilder { + Visitor build(); + } + + @JsonTypeInfo( + use = JsonTypeInfo.Id.NAME, + include = JsonTypeInfo.As.EXISTING_PROPERTY, + property = "type", + visible = true, + defaultImpl = UnknownWrapper.class) + @JsonSubTypes({@JsonSubTypes.Type(SafeLongWrapper.class), @JsonSubTypes.Type(UnsafeLongWrapper.class)}) + @JsonIgnoreProperties(ignoreUnknown = true) + private interface Base { + T accept(Visitor visitor); + } + + @JsonTypeName("safeLong") + private static final class SafeLongWrapper implements Base { + private final long value; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + private SafeLongWrapper(@JsonSetter("safeLong") @Nonnull long value) { + Preconditions.checkNotNull(value, "safeLong cannot be null"); + this.value = value; + } + + @JsonProperty(value = "type", index = 0) + private String getType() { + return "safeLong"; + } + + @JsonProperty("safeLong") + private long getValue() { + return value; + } + + @Override + public T accept(Visitor visitor) { + return visitor.visitSafeLong(value); + } + + @Override + public boolean equals(@Nullable Object other) { + return this == other || (other instanceof SafeLongWrapper && equalTo((SafeLongWrapper) other)); + } + + private boolean equalTo(SafeLongWrapper other) { + return this.value == other.value; + } + + @Override + public int hashCode() { + return Long.hashCode(this.value); + } + + @Override + public String toString() { + return "SafeLongWrapper{value: " + value + '}'; + } + } + + @JsonTypeName("unsafeLong") + private static final class UnsafeLongWrapper implements Base { + private final long value; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + private UnsafeLongWrapper(@JsonSetter("unsafeLong") @Nonnull long value) { + Preconditions.checkNotNull(value, "unsafeLong cannot be null"); + this.value = value; + } + + @JsonProperty(value = "type", index = 0) + private String getType() { + return "unsafeLong"; + } + + @JsonProperty("unsafeLong") + private long getValue() { + return value; + } + + @Override + public T accept(Visitor visitor) { + return visitor.visitUnsafeLong(value); + } + + @Override + public boolean equals(@Nullable Object other) { + return this == other || (other instanceof UnsafeLongWrapper && equalTo((UnsafeLongWrapper) other)); + } + + private boolean equalTo(UnsafeLongWrapper other) { + return this.value == other.value; + } + + @Override + public int hashCode() { + return Long.hashCode(this.value); + } + + @Override + public String toString() { + return "UnsafeLongWrapper{value: " + value + '}'; + } + } + + private static final class UnknownWrapper implements Base { + private final String type; + + private final Map value; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + private UnknownWrapper(@JsonProperty("type") String type) { + this(type, new HashMap()); + } + + private UnknownWrapper(@Nonnull String type, @Nonnull Map value) { + Preconditions.checkNotNull(type, "type cannot be null"); + Preconditions.checkNotNull(value, "value cannot be null"); + this.type = type; + this.value = value; + } + + @JsonProperty + private String getType() { + return type; + } + + @JsonAnyGetter + private Map getValue() { + return value; + } + + @JsonAnySetter + private void put(String key, Object val) { + value.put(key, val); + } + + @Override + public T accept(Visitor visitor) { + return visitor.visitUnknown(type); + } + + @Override + public boolean equals(@Nullable Object other) { + return this == other || (other instanceof UnknownWrapper && equalTo((UnknownWrapper) other)); + } + + private boolean equalTo(UnknownWrapper other) { + return this.type.equals(other.type) && this.value.equals(other.value); + } + + @Override + public int hashCode() { + int hash = 1; + hash = 31 * hash + this.type.hashCode(); + hash = 31 * hash + this.value.hashCode(); + return hash; + } + + @Override + public String toString() { + return "UnknownWrapper{type: " + type + ", value: " + value + '}'; + } + } +} diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java new file mode 100644 index 000000000..2800b4d49 --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java @@ -0,0 +1,49 @@ +package test.prefix.com.palantir.product; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonValue; +import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; +import javax.annotation.Nonnull; +import javax.annotation.processing.Generated; + +@Safe +@Generated("com.palantir.conjure.java.types.AliasGenerator") +public final class SafeExternalAliasExample { + private final SafeExternalLongAlias value; + + private SafeExternalAliasExample(@Nonnull SafeExternalLongAlias value) { + this.value = Preconditions.checkNotNull(value, "value cannot be null"); + } + + @JsonValue + public SafeExternalLongAlias get() { + return value; + } + + @Override + public String toString() { + return value.toString(); + } + + @Override + public boolean equals(Object other) { + return this == other + || (other instanceof SafeExternalAliasExample + && this.value.equals(((SafeExternalAliasExample) other).value)); + } + + @Override + public int hashCode() { + return value.hashCode(); + } + + public static SafeExternalAliasExample valueOf(String value) { + return of(SafeExternalLongAlias.valueOf(value)); + } + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public static SafeExternalAliasExample of(@Nonnull SafeExternalLongAlias value) { + return new SafeExternalAliasExample(value); + } +} diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongAlias.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongAlias.java new file mode 100644 index 000000000..46d137c45 --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongAlias.java @@ -0,0 +1,48 @@ +package test.prefix.com.palantir.product; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonValue; +import com.palantir.logsafe.Safe; +import javax.annotation.processing.Generated; + +@Safe +@Generated("com.palantir.conjure.java.types.AliasGenerator") +public final class SafeExternalLongAlias { + private final long value; + + private SafeExternalLongAlias(long value) { + this.value = value; + } + + @JsonValue + @Safe + public long get() { + return value; + } + + @Override + public String toString() { + return String.valueOf(value); + } + + @Override + public boolean equals(Object other) { + return this == other + || (other instanceof SafeExternalLongAlias && this.value == ((SafeExternalLongAlias) other).value); + } + + @Override + public int hashCode() { + return Long.hashCode(value); + } + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public static SafeExternalLongAlias valueOf(@Safe String value) { + return of(Long.valueOf(value)); + } + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public static SafeExternalLongAlias of(@Safe long value) { + return new SafeExternalLongAlias(value); + } +} diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java new file mode 100644 index 000000000..9cca4a14c --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java @@ -0,0 +1,116 @@ +package test.prefix.com.palantir.product; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; +import java.util.ArrayList; +import java.util.List; +import javax.annotation.Nullable; +import javax.annotation.processing.Generated; + +@Safe +@JsonDeserialize(builder = SafeExternalLongExample.Builder.class) +@Generated("com.palantir.conjure.java.types.BeanGenerator") +public final class SafeExternalLongExample { + private final long safeExternalLongValue; + + private SafeExternalLongExample(long safeExternalLongValue) { + this.safeExternalLongValue = safeExternalLongValue; + } + + @JsonProperty("safeExternalLongValue") + @Safe + public long getSafeExternalLongValue() { + return this.safeExternalLongValue; + } + + @Override + public boolean equals(@Nullable Object other) { + return this == other || (other instanceof SafeExternalLongExample && equalTo((SafeExternalLongExample) other)); + } + + private boolean equalTo(SafeExternalLongExample other) { + return this.safeExternalLongValue == other.safeExternalLongValue; + } + + @Override + public int hashCode() { + return Long.hashCode(this.safeExternalLongValue); + } + + @Override + public String toString() { + return "SafeExternalLongExample{safeExternalLongValue: " + safeExternalLongValue + '}'; + } + + public static SafeExternalLongExample of(@Safe long safeExternalLongValue) { + return builder().safeExternalLongValue(safeExternalLongValue).build(); + } + + public static Builder builder() { + return new Builder(); + } + + @Generated("com.palantir.conjure.java.types.BeanBuilderGenerator") + @JsonIgnoreProperties(ignoreUnknown = true) + public static final class Builder { + boolean _buildInvoked; + + private long safeExternalLongValue; + + private boolean _safeExternalLongValueInitialized = false; + + private Builder() {} + + public Builder from(SafeExternalLongExample other) { + checkNotBuilt(); + safeExternalLongValue(other.getSafeExternalLongValue()); + return this; + } + + @JsonSetter("safeExternalLongValue") + public Builder safeExternalLongValue(@Safe long safeExternalLongValue) { + checkNotBuilt(); + this.safeExternalLongValue = safeExternalLongValue; + this._safeExternalLongValueInitialized = true; + return this; + } + + private void validatePrimitiveFieldsHaveBeenInitialized() { + List missingFields = null; + missingFields = + addFieldIfMissing(missingFields, _safeExternalLongValueInitialized, "safeExternalLongValue"); + if (missingFields != null) { + throw new SafeIllegalArgumentException( + "Some required fields have not been set", SafeArg.of("missingFields", missingFields)); + } + } + + private static List addFieldIfMissing(List prev, boolean initialized, String fieldName) { + List missingFields = prev; + if (!initialized) { + if (missingFields == null) { + missingFields = new ArrayList<>(1); + } + missingFields.add(fieldName); + } + return missingFields; + } + + public SafeExternalLongExample build() { + checkNotBuilt(); + this._buildInvoked = true; + validatePrimitiveFieldsHaveBeenInitialized(); + return new SafeExternalLongExample(safeExternalLongValue); + } + + private void checkNotBuilt() { + Preconditions.checkState(!_buildInvoked, "Build has already been called"); + } + } +} diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/ConjureTagsTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/ConjureTagsTest.java index 1fd6b0aff..95ce1b2a4 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/ConjureTagsTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/ConjureTagsTest.java @@ -192,6 +192,74 @@ void testTagAndTypeSafetyDisagreement() { .hasMessageContaining("Declared argument safety is incompatible with the provided type"); } + @Test + void testExternalImport_AtImportTime() { + SafetyEvaluator safetyEvaluator = new SafetyEvaluator(ImmutableMap.of()); + Type external = Type.external(ExternalReference.builder() + .externalReference(TypeName.of("Long", "java.lang")) + .fallback(Type.primitive(PrimitiveType.STRING)) + .safety(LogSafety.DO_NOT_LOG) + .build()); + ArgumentDefinition argument = ArgumentDefinition.builder() + .argName(ArgumentName.of("testArgument")) + .type(external) + .paramType(ParameterType.body(BodyParameterType.of())) + .build(); + assertThat(ConjureTags.validateArgument(argument, safetyEvaluator)).hasValue(LogSafety.DO_NOT_LOG); + } + + @Test + void testExternalImport_NoSafety() { + SafetyEvaluator safetyEvaluator = new SafetyEvaluator(ImmutableMap.of()); + Type external = Type.external(ExternalReference.builder() + .externalReference(TypeName.of("Long", "java.lang")) + .fallback(Type.primitive(PrimitiveType.STRING)) + .build()); + ArgumentDefinition argument = ArgumentDefinition.builder() + .argName(ArgumentName.of("testArgument")) + .type(external) + .paramType(ParameterType.body(BodyParameterType.of())) + .build(); + assertThat(ConjureTags.validateArgument(argument, safetyEvaluator)).isEmpty(); + } + + // upstream validation logic should catch this degenerate case, but codegen should still prefer the import time + // annotations + @Test + void testExternalImport_ImportAndUsageTime() { + SafetyEvaluator safetyEvaluator = new SafetyEvaluator(ImmutableMap.of()); + Type external = Type.external(ExternalReference.builder() + .externalReference(TypeName.of("Long", "java.lang")) + .fallback(Type.primitive(PrimitiveType.STRING)) + .safety(LogSafety.DO_NOT_LOG) + .build()); + ArgumentDefinition argument = ArgumentDefinition.builder() + .argName(ArgumentName.of("testArgument")) + .type(external) + .safety(LogSafety.UNSAFE) + .paramType(ParameterType.body(BodyParameterType.of())) + .build(); + assertThat(ConjureTags.validateArgument(argument, safetyEvaluator)).hasValue(LogSafety.DO_NOT_LOG); + } + + // upstream validation logic should catch this degenerate case, but codegen should still prefer the import time + // annotations + @Test + void testExternalImport_UsageTimeOnly() { + SafetyEvaluator safetyEvaluator = new SafetyEvaluator(ImmutableMap.of()); + Type external = Type.external(ExternalReference.builder() + .externalReference(TypeName.of("Long", "java.lang")) + .fallback(Type.primitive(PrimitiveType.STRING)) + .build()); + ArgumentDefinition argument = ArgumentDefinition.builder() + .argName(ArgumentName.of("testArgument")) + .type(external) + .safety(LogSafety.UNSAFE) + .paramType(ParameterType.body(BodyParameterType.of())) + .build(); + assertThat(ConjureTags.validateArgument(argument, safetyEvaluator)).isEmpty(); + } + private static ArgumentDefinition tags(String... tags) { return ArgumentDefinition.builder() .argName(ArgumentName.of("name")) diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/UndertowServiceEteTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/UndertowServiceEteTest.java index cd2bc5588..2326f42c2 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/UndertowServiceEteTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/UndertowServiceEteTest.java @@ -569,7 +569,8 @@ public static void beforeClass() throws IOException { ConjureDefinition def = Conjure.parse(ImmutableList.of( new File("src/test/resources/ete-service.yml"), new File("src/test/resources/ete-binary.yml"), - new File("src/test/resources/alias-test-service.yml"))); + new File("src/test/resources/alias-test-service.yml"), + new File("src/test/resources/external-long-test-service.yml"))); Options options = Options.builder() .undertowServicePrefix(true) .nonNullCollections(true) diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java index 59f9455fb..01b6b683f 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java @@ -17,6 +17,7 @@ package com.palantir.conjure.java.types; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.google.common.collect.Iterables; import com.palantir.conjure.defs.SafetyDeclarationRequirements; @@ -185,12 +186,13 @@ private static Stream providesExternalRefTypes_ImportAndUsageTime() { return getTypes_SafetyAtUsageTime(external); } - // We should never hit this case because validation should enforce that external imports declare safety - // at import time, not usage time @ParameterizedTest @MethodSource("providesExternalRefTypes_ImportAndUsageTime") void testExternalRefType_AtImportAndUsageTime(TypeDefinition typeDefinition, ConjureDefinition conjureDef) { - // ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); + // upstream validation logic should catch this degenerate case, but codegen should still prefer the import time + // annotations + assertThatThrownBy( + () -> ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED)); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(typeDefinition)).hasValue(LogSafety.DO_NOT_LOG); } @@ -203,18 +205,18 @@ private static Stream providesExternalRefTypes_OnlyAtUsageTime() { return getTypes_SafetyAtUsageTime(external); } - // We should never hit this case because validation should enforce that external imports declare safety - // at import time, not usage time @ParameterizedTest @MethodSource("providesExternalRefTypes_OnlyAtUsageTime") void testExternalRefType_OnlyAtUsageTime(TypeDefinition typeDefinition, ConjureDefinition conjureDef) { - // ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); + // upstream validation logic should catch this degenerate case, but codegen should still prefer the import time + // annotations + assertThatThrownBy( + () -> ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED)); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(typeDefinition)).isEmpty(); } - // TODO: how will I test endpoints - // and the generation logic?? like an E2E test + // TODO test the generation logic?? like an E2E test @Test void testMapSafetyUnsafeValue() { diff --git a/conjure-java-core/src/test/resources/example-types.yml b/conjure-java-core/src/test/resources/example-types.yml index 9fd88b30f..142ddc767 100644 --- a/conjure-java-core/src/test/resources/example-types.yml +++ b/conjure-java-core/src/test/resources/example-types.yml @@ -13,6 +13,11 @@ types: base-type: string external: java: java.lang.String + SafeExternalLong: + base-type: safelong + external: + java: java.lang.Long + safety: safe definitions: default-package: com.palantir.product objects: @@ -25,6 +30,9 @@ types: SafeLongExample: fields: safeLongValue: safelong + SafeExternalLongExample: + fields: + safeExternalLongValue: SafeExternalLong RidExample: fields: ridValue: rid @@ -131,6 +139,8 @@ types: alias: datetime BinaryAliasExample: alias: binary + SafeExternalAliasExample: + alias: SafeExternalLongAlias PrimitiveOptionalsExample: fields: num: optional @@ -214,6 +224,11 @@ types: unknown: string EmptyUnionTypeExample: union: {} + ExternalLongUnionExample: + docs: A union of a safe and unsafe external long. + union: + safeLong: SafeExternalLong + unsafeLong: ExternalLong EmptyObjectExample: fields: {} AliasAsMapKeyExample: @@ -272,6 +287,8 @@ types: alias: ExternalLong ExternalLongAliasTwo: alias: ExternalLongAliasOne + SafeExternalLongAlias: + alias: SafeExternalLong OptionalAlias: alias: optional safety: safe diff --git a/conjure-java-core/src/test/resources/external-long-test-service.yml b/conjure-java-core/src/test/resources/external-long-test-service.yml new file mode 100644 index 000000000..4f7627e8c --- /dev/null +++ b/conjure-java-core/src/test/resources/external-long-test-service.yml @@ -0,0 +1,20 @@ +types: + imports: + DangerousLong: + external: + java: java.lang.Long + safety: do-not-log + +services: + ExternalLongTestService: + name: External Long Test Service + package: com.palantir.product + default-auth: header + base-path: /external-long + + endpoints: + testExternalLongArg: + http: POST /test + args: + externalLong: + type: DangerousLong From 8e9717cfc1ae54bd151a42993910719767ec9872 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Thu, 2 Feb 2023 15:20:39 -0500 Subject: [PATCH 03/16] Style --- .../com/palantir/conjure/java/types/SafetyEvaluatorTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java index 01b6b683f..3fc0346b2 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java @@ -216,8 +216,6 @@ void testExternalRefType_OnlyAtUsageTime(TypeDefinition typeDefinition, ConjureD assertThat(evaluator.evaluate(typeDefinition)).isEmpty(); } - // TODO test the generation logic?? like an E2E test - @Test void testMapSafetyUnsafeValue() { TypeDefinition object = TypeDefinition.object(ObjectDefinition.builder() From 168ee8781e08c57c883eb45ecec1ad4ed99774d6 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Thu, 2 Feb 2023 15:36:15 -0500 Subject: [PATCH 04/16] squash additional direct usages of getSafety --- .../java/com/palantir/conjure/java/ConjureTags.java | 8 +++----- .../palantir/conjure/java/types/AliasGenerator.java | 2 +- .../java/types/BeanBuilderAuxiliarySettersUtils.java | 4 ++-- .../conjure/java/types/BeanBuilderGenerator.java | 5 +++-- .../com/palantir/conjure/java/types/BeanGenerator.java | 10 +++++----- .../palantir/conjure/java/types/ErrorGenerator.java | 5 +++-- .../palantir/conjure/java/types/SafetyEvaluator.java | 7 ++++--- .../palantir/conjure/java/types/UnionGenerator.java | 10 ++++++---- .../com/palantir/conjure/java/util/SafetyUtils.java | 6 +++--- 9 files changed, 30 insertions(+), 27 deletions(-) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/ConjureTags.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/ConjureTags.java index 6cf0ebc32..cf8bb4e82 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/ConjureTags.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/ConjureTags.java @@ -82,9 +82,7 @@ public static Optional validateArgument(ArgumentDefinition argument, public static Optional safety(ArgumentDefinition argument) { validateTags(argument); - // Always prefer the external import's safety. If EI are supported, this is correct. If not, there won't be an - // annotation anywhere so it doesn't matter. - Optional argumentSafety = SafetyUtils.getSafety(argument); + Optional argumentSafety = SafetyUtils.getMaybeExternalSafety(argument); if (argumentSafety.isPresent()) { return argumentSafety; } @@ -109,12 +107,12 @@ public static void validateTags(ArgumentDefinition argument) { isSafe(tags) ? "safe" : "unsafe", argument.getArgName())); } - Optional argumentSafety = SafetyUtils.getSafety(argument); + Optional argumentSafety = SafetyUtils.getMaybeExternalSafety(argument); if (argumentSafety.isPresent()) { if (markerSafety.isPresent()) { throw new IllegalStateException(String.format( "Unexpected 'safety: %s' value in addition to a '%s' marker on argument '%s'", - argument.getSafety().get().accept(DefFormatSafetyVisitor.INSTANCE), + argumentSafety.get().accept(DefFormatSafetyVisitor.INSTANCE), markerSafety.get().accept(MarkerNameLogSafetyVisitor.INSTANCE), argument.getArgName())); } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/AliasGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/AliasGenerator.java index 87bef7e54..83c4ce14d 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/AliasGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/AliasGenerator.java @@ -66,7 +66,7 @@ public static JavaFile generateAliasType( TypeMapper typeMapper, SafetyEvaluator safetyEvaluator, AliasDefinition typeDef, Options options) { com.palantir.conjure.spec.TypeName prefixedTypeName = Packages.getPrefixedName(typeDef.getTypeName(), options.packagePrefix()); - Optional safety = SafetyUtils.getSafety(typeDef); + Optional safety = SafetyUtils.getMaybeExternalSafety(typeDef); TypeName aliasTypeName = ConjureAnnotations.withSafety(typeMapper.getClassName(typeDef.getAlias()), safety); ClassName thisClass = ClassName.get(prefixedTypeName.getPackage(), prefixedTypeName.getName()); diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java index a4986053b..81fa90683 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java @@ -58,7 +58,7 @@ public static MethodSpec.Builder createCollectionSetterBuilder( .addParameter(Parameters.nonnullParameter( widenParameterIfPossible(field.type, type, typeMapper), field.name, - SafetyUtils.getSafety(enriched.conjureDef()))); + SafetyUtils.getMaybeExternalSafety(enriched.conjureDef()))); } public static MethodSpec.Builder createOptionalSetterBuilder( @@ -69,7 +69,7 @@ public static MethodSpec.Builder createOptionalSetterBuilder( .addParameter(Parameters.nonnullParameter( typeMapper.getClassName(type.getItemType()), field.name, - SafetyUtils.getSafety(enriched.conjureDef()))); + SafetyUtils.getMaybeExternalSafety(enriched.conjureDef()))); } public static MethodSpec.Builder createItemSetterBuilder( 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 5e0ed8ac2..d59f8e4c4 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 @@ -264,7 +264,8 @@ private MethodSpec createFromObject(Collection enrichedFields, bo private EnrichedField createField(FieldName fieldName, FieldDefinition field) { Type type = field.getType(); - TypeName typeName = ConjureAnnotations.withSafety(typeMapper.getClassName(type), SafetyUtils.getSafety(field)); + TypeName typeName = + ConjureAnnotations.withSafety(typeMapper.getClassName(type), SafetyUtils.getMaybeExternalSafety(field)); FieldSpec.Builder spec = FieldSpec.builder(typeName, JavaNameSanitizer.sanitize(fieldName), Modifier.PRIVATE); if (type.accept(TypeVisitor.IS_LIST) || type.accept(TypeVisitor.IS_SET) || type.accept(TypeVisitor.IS_MAP)) { spec.initializer("new $T<>()", type.accept(COLLECTION_CONCRETE_TYPE)); @@ -329,7 +330,7 @@ private MethodSpec createSetter( .addParameter(Parameters.nonnullParameter( BeanBuilderAuxiliarySettersUtils.widenParameterIfPossible(field.type, type, typeMapper), field.name, - SafetyUtils.getSafety(enriched.conjureDef()))) + SafetyUtils.getMaybeExternalSafety(enriched.conjureDef()))) .addCode(verifyNotBuilt()) .addCode(typeAwareAssignment(enriched, type, shouldClearFirst)); diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java index 09dc0846a..6d334f061 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java @@ -465,7 +465,7 @@ private static MethodSpec createGetter( .addAnnotation(AnnotationSpec.builder(JsonProperty.class) .addMember("value", "$S", field.fieldName().get()) .build()) - .addAnnotations(ConjureAnnotations.safety(SafetyUtils.getSafety(field.conjureDef()))) + .addAnnotations(ConjureAnnotations.safety(SafetyUtils.getMaybeExternalSafety(field.conjureDef()))) .returns(field.poetSpec().type); Type conjureDefType = field.conjureDef().getType(); if (featureFlags.excludeEmptyOptionals()) { @@ -542,10 +542,10 @@ private static MethodSpec createStaticFactoryMethod(ImmutableList .addCode("return $L;", SINGLETON_INSTANCE_NAME); } else { builder.addCode("return builder()"); - fields.forEach(field -> builder.addParameter( - ParameterSpec.builder(getTypeNameWithoutOptional(field.poetSpec()), field.poetSpec().name) - .addAnnotations(ConjureAnnotations.safety(SafetyUtils.getSafety(field.conjureDef()))) - .build())); + fields.forEach(field -> builder.addParameter(ParameterSpec.builder( + getTypeNameWithoutOptional(field.poetSpec()), field.poetSpec().name) + .addAnnotations(ConjureAnnotations.safety(SafetyUtils.getMaybeExternalSafety(field.conjureDef()))) + .build())); // Follow order on adding methods on builder to comply with staged builders option if set sortedEnrichedFields(fields).map(EnrichedField::poetSpec).forEach(spec -> { if (isOptional(spec)) { diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ErrorGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ErrorGenerator.java index af145d25b..840dc57d2 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ErrorGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ErrorGenerator.java @@ -27,6 +27,7 @@ import com.palantir.conjure.java.api.errors.ServiceException; import com.palantir.conjure.java.util.Javadoc; import com.palantir.conjure.java.util.Packages; +import com.palantir.conjure.java.util.SafetyUtils; import com.palantir.conjure.java.util.TypeFunctions; import com.palantir.conjure.spec.ConjureDefinition; import com.palantir.conjure.spec.ErrorDefinition; @@ -165,7 +166,7 @@ private JavaFile generateErrorTypesForNamespace( .build())) .map(arg -> { TypeName argumentTypeName = typeMapper.getClassName(arg.getType()); - Optional required = arg.getSafety(); + Optional required = SafetyUtils.getMaybeExternalSafety(arg); Optional typeSafety = safetyEvaluator.evaluate(arg.getType()); if (!SafetyEvaluator.allows(required, typeSafety)) { throw new IllegalStateException(String.format( @@ -182,7 +183,7 @@ private JavaFile generateErrorTypesForNamespace( return ParameterSpec.builder( argumentTypeName, arg.getFieldName().get()) - .addAnnotations(ConjureAnnotations.safety(arg.getSafety())) + .addAnnotations(ConjureAnnotations.safety(required)) .addJavadoc( "$L", StringUtils.appendIfMissing( diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java index 08a025a47..2f237032f 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java @@ -90,7 +90,8 @@ private TypeDefinitionSafetyVisitor(Map definitionMap, @Override public Optional visitAlias(AliasDefinition value) { - return with(value.getTypeName(), () -> getSafety(value.getAlias(), SafetyUtils.getSafety(value))); + return with( + value.getTypeName(), () -> getSafety(value.getAlias(), SafetyUtils.getMaybeExternalSafety(value))); } @Override @@ -103,7 +104,7 @@ public Optional visitObject(ObjectDefinition value) { return with(value.getTypeName(), () -> { Optional safety = Optional.of(LogSafety.SAFE); for (FieldDefinition field : value.getFields()) { - safety = combine(safety, getSafety(field.getType(), SafetyUtils.getSafety(field))); + safety = combine(safety, getSafety(field.getType(), SafetyUtils.getMaybeExternalSafety(field))); } return safety; }); @@ -114,7 +115,7 @@ public Optional visitUnion(UnionDefinition value) { return with(value.getTypeName(), () -> { Optional safety = UNKNOWN_UNION_VARINT_SAFETY; for (FieldDefinition variant : value.getUnion()) { - safety = combine(safety, getSafety(variant.getType(), SafetyUtils.getSafety(variant))); + safety = combine(safety, getSafety(variant.getType(), SafetyUtils.getMaybeExternalSafety(variant))); } return safety; }); diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/UnionGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/UnionGenerator.java index 56304e612..854a647de 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/UnionGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/UnionGenerator.java @@ -105,7 +105,7 @@ public static JavaFile generateUnionType( .collect(StableCollectors.toLinkedMap( Function.identity(), entry -> ConjureAnnotations.withSafety( - typeMapper.getClassName(entry.getType()), SafetyUtils.getSafety(entry)))); + typeMapper.getClassName(entry.getType()), SafetyUtils.getMaybeExternalSafety(entry)))); List fields = ImmutableList.of(FieldSpec.builder(baseClass, VALUE_FIELD_NAME, Modifier.PRIVATE, Modifier.FINAL) .build()); @@ -180,7 +180,8 @@ private static List generateStaticFactories( MethodSpec.Builder builder = MethodSpec.methodBuilder(JavaNameSanitizer.sanitize(memberName)) .addModifiers(Modifier.PUBLIC, Modifier.STATIC) .addParameter(ParameterSpec.builder(memberType, variableName) - .addAnnotations(ConjureAnnotations.safety(SafetyUtils.getSafety(memberTypeDef))) + .addAnnotations(ConjureAnnotations.safety( + SafetyUtils.getMaybeExternalSafety(memberTypeDef))) .build()) .addStatement( "return new $T(new $T($L))", @@ -315,7 +316,8 @@ private static List generateMemberVisitMethods(Map sortedStageNameTypePairs(Map new NameTypeMetadata( sanitizeUnknown(entry.getKey().getFieldName().get()), entry.getValue(), - SafetyUtils.getSafety(entry.getKey()))) + SafetyUtils.getMaybeExternalSafety(entry.getKey()))) .sorted(Comparator.comparing(p -> p.memberName)), Stream.of(NameTypeMetadata.UNKNOWN)); } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java index 8352604ae..1eb12cbb6 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java @@ -27,21 +27,21 @@ public final class SafetyUtils { private SafetyUtils() {} - public static Optional getSafety(AliasDefinition alias) { + public static Optional getMaybeExternalSafety(AliasDefinition alias) { if (alias.getAlias().accept(MoreVisitors.IS_EXTERNAL)) { return alias.getAlias().accept(MoreVisitors.EXTERNAL).getSafety(); } return alias.getSafety(); } - public static Optional getSafety(FieldDefinition field) { + public static Optional getMaybeExternalSafety(FieldDefinition field) { if (field.getType().accept(MoreVisitors.IS_EXTERNAL)) { return field.getType().accept(MoreVisitors.EXTERNAL).getSafety(); } return field.getSafety(); } - public static Optional getSafety(ArgumentDefinition argument) { + public static Optional getMaybeExternalSafety(ArgumentDefinition argument) { if (argument.getType().accept(MoreVisitors.IS_EXTERNAL)) { return argument.getType().accept(MoreVisitors.EXTERNAL).getSafety(); } From 9120f564d798c7347336f61ededd8694e53b2a55 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Thu, 2 Feb 2023 15:44:42 -0500 Subject: [PATCH 05/16] fix generated test code --- .../java/com/palantir/product/SafeExternalAliasExample.java | 3 ++- .../java/com/palantir/product/SafeExternalLongAlias.java | 4 +++- .../java/com/palantir/product/SafeExternalLongExample.java | 1 + .../prefix/com/palantir/product/SafeExternalAliasExample.java | 3 ++- .../prefix/com/palantir/product/SafeExternalLongAlias.java | 4 +++- .../prefix/com/palantir/product/SafeExternalLongExample.java | 1 + 6 files changed, 12 insertions(+), 4 deletions(-) diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java index 95129b69e..6f434d818 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java @@ -5,6 +5,7 @@ import com.palantir.logsafe.Preconditions; import com.palantir.logsafe.Safe; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import javax.annotation.processing.Generated; @Safe @@ -27,7 +28,7 @@ public String toString() { } @Override - public boolean equals(Object other) { + public boolean equals(@Nullable Object other) { return this == other || (other instanceof SafeExternalAliasExample && this.value.equals(((SafeExternalAliasExample) other).value)); diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongAlias.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongAlias.java index 57d90260b..c21a63351 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongAlias.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongAlias.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; import com.palantir.logsafe.Safe; +import javax.annotation.Nullable; import javax.annotation.processing.Generated; @Safe @@ -21,12 +22,13 @@ public long get() { } @Override + @Safe public String toString() { return String.valueOf(value); } @Override - public boolean equals(Object other) { + public boolean equals(@Nullable Object other) { return this == other || (other instanceof SafeExternalLongAlias && this.value == ((SafeExternalLongAlias) other).value); } diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java index fee2b2945..5970d7e89 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java @@ -43,6 +43,7 @@ public int hashCode() { } @Override + @Safe public String toString() { return "SafeExternalLongExample{safeExternalLongValue: " + safeExternalLongValue + '}'; } diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java index 2800b4d49..136c56441 100644 --- a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java @@ -5,6 +5,7 @@ import com.palantir.logsafe.Preconditions; import com.palantir.logsafe.Safe; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import javax.annotation.processing.Generated; @Safe @@ -27,7 +28,7 @@ public String toString() { } @Override - public boolean equals(Object other) { + public boolean equals(@Nullable Object other) { return this == other || (other instanceof SafeExternalAliasExample && this.value.equals(((SafeExternalAliasExample) other).value)); diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongAlias.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongAlias.java index 46d137c45..1fbf0d5db 100644 --- a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongAlias.java +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongAlias.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; import com.palantir.logsafe.Safe; +import javax.annotation.Nullable; import javax.annotation.processing.Generated; @Safe @@ -21,12 +22,13 @@ public long get() { } @Override + @Safe public String toString() { return String.valueOf(value); } @Override - public boolean equals(Object other) { + public boolean equals(@Nullable Object other) { return this == other || (other instanceof SafeExternalLongAlias && this.value == ((SafeExternalLongAlias) other).value); } diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java index 9cca4a14c..75ab7fc43 100644 --- a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java @@ -44,6 +44,7 @@ public int hashCode() { } @Override + @Safe public String toString() { return "SafeExternalLongExample{safeExternalLongValue: " + safeExternalLongValue + '}'; } From abdd7d49ab4de1d0f3a70a1458bad3347f56e944 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Wed, 8 Feb 2023 13:51:45 -0500 Subject: [PATCH 06/16] resurrecting this PR --- .../product/SafeExternalLongExample.java | 88 +++++++++++++++++-- .../product/SafeExternalAliasExample.java | 50 ----------- .../product/SafeExternalLongExample.java | 88 +++++++++++++++++-- .../src/test/resources/example-types.yml | 3 +- 4 files changed, 163 insertions(+), 66 deletions(-) delete mode 100644 conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java index 5970d7e89..841063df7 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java @@ -1,7 +1,9 @@ package com.palantir.product; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.palantir.logsafe.Preconditions; import com.palantir.logsafe.Safe; @@ -9,6 +11,9 @@ import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import java.util.ArrayList; import java.util.List; +import java.util.Optional; +import java.util.function.Function; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.processing.Generated; @@ -18,8 +23,14 @@ public final class SafeExternalLongExample { private final long safeExternalLongValue; - private SafeExternalLongExample(long safeExternalLongValue) { + private final Optional optionalSafeExternalLong; + + private int memoizedHashCode; + + private SafeExternalLongExample(long safeExternalLongValue, Optional optionalSafeExternalLong) { + validateFields(optionalSafeExternalLong); this.safeExternalLongValue = safeExternalLongValue; + this.optionalSafeExternalLong = optionalSafeExternalLong; } @JsonProperty("safeExternalLongValue") @@ -28,28 +39,72 @@ public long getSafeExternalLongValue() { return this.safeExternalLongValue; } + @JsonProperty("optionalSafeExternalLong") + @JsonInclude(JsonInclude.Include.NON_ABSENT) + public Optional getOptionalSafeExternalLong() { + return this.optionalSafeExternalLong; + } + @Override public boolean equals(@Nullable Object other) { return this == other || (other instanceof SafeExternalLongExample && equalTo((SafeExternalLongExample) other)); } private boolean equalTo(SafeExternalLongExample other) { - return this.safeExternalLongValue == other.safeExternalLongValue; + if (this.memoizedHashCode != 0 + && other.memoizedHashCode != 0 + && this.memoizedHashCode != other.memoizedHashCode) { + return false; + } + return this.safeExternalLongValue == other.safeExternalLongValue + && this.optionalSafeExternalLong.equals(other.optionalSafeExternalLong); } @Override public int hashCode() { - return Long.hashCode(this.safeExternalLongValue); + int result = memoizedHashCode; + if (result == 0) { + int hash = 1; + hash = 31 * hash + Long.hashCode(this.safeExternalLongValue); + hash = 31 * hash + this.optionalSafeExternalLong.hashCode(); + result = hash; + memoizedHashCode = result; + } + return result; } @Override @Safe public String toString() { - return "SafeExternalLongExample{safeExternalLongValue: " + safeExternalLongValue + '}'; + return "SafeExternalLongExample{safeExternalLongValue: " + safeExternalLongValue + + ", optionalSafeExternalLong: " + optionalSafeExternalLong + '}'; + } + + public static SafeExternalLongExample of(@Safe long safeExternalLongValue, long optionalSafeExternalLong) { + return builder() + .safeExternalLongValue(safeExternalLongValue) + .optionalSafeExternalLong(Optional.of(optionalSafeExternalLong)) + .build(); + } + + private static void validateFields(Optional optionalSafeExternalLong) { + List missingFields = null; + missingFields = addFieldIfMissing(missingFields, optionalSafeExternalLong, "optionalSafeExternalLong"); + if (missingFields != null) { + throw new SafeIllegalArgumentException( + "Some required fields have not been set", SafeArg.of("missingFields", missingFields)); + } } - public static SafeExternalLongExample of(@Safe long safeExternalLongValue) { - return builder().safeExternalLongValue(safeExternalLongValue).build(); + private static List addFieldIfMissing(List prev, Object fieldValue, String fieldName) { + List missingFields = prev; + if (fieldValue == null) { + if (missingFields == null) { + missingFields = new ArrayList<>(1); + } + missingFields.add(fieldName); + } + return missingFields; } public static Builder builder() { @@ -62,6 +117,8 @@ public static final class Builder { private long safeExternalLongValue; + private Optional optionalSafeExternalLong = Optional.empty(); + private boolean _safeExternalLongValueInitialized = false; private Builder() {} @@ -69,6 +126,7 @@ private Builder() {} public Builder from(SafeExternalLongExample other) { checkNotBuilt(); safeExternalLongValue(other.getSafeExternalLongValue()); + optionalSafeExternalLong(other.getOptionalSafeExternalLong()); return this; } @@ -80,6 +138,22 @@ public Builder safeExternalLongValue(@Safe long safeExternalLongValue) { return this; } + @JsonSetter(value = "optionalSafeExternalLong", nulls = Nulls.SKIP) + public Builder optionalSafeExternalLong(@Nonnull Optional optionalSafeExternalLong) { + checkNotBuilt(); + this.optionalSafeExternalLong = Preconditions.checkNotNull( + optionalSafeExternalLong, "optionalSafeExternalLong cannot be null") + .map(Function.identity()); + return this; + } + + public Builder optionalSafeExternalLong(long optionalSafeExternalLong) { + checkNotBuilt(); + this.optionalSafeExternalLong = Optional.of( + Preconditions.checkNotNull(optionalSafeExternalLong, "optionalSafeExternalLong cannot be null")); + return this; + } + private void validatePrimitiveFieldsHaveBeenInitialized() { List missingFields = null; missingFields = @@ -105,7 +179,7 @@ public SafeExternalLongExample build() { checkNotBuilt(); this._buildInvoked = true; validatePrimitiveFieldsHaveBeenInitialized(); - return new SafeExternalLongExample(safeExternalLongValue); + return new SafeExternalLongExample(safeExternalLongValue, optionalSafeExternalLong); } private void checkNotBuilt() { diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java deleted file mode 100644 index 136c56441..000000000 --- a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalAliasExample.java +++ /dev/null @@ -1,50 +0,0 @@ -package test.prefix.com.palantir.product; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonValue; -import com.palantir.logsafe.Preconditions; -import com.palantir.logsafe.Safe; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import javax.annotation.processing.Generated; - -@Safe -@Generated("com.palantir.conjure.java.types.AliasGenerator") -public final class SafeExternalAliasExample { - private final SafeExternalLongAlias value; - - private SafeExternalAliasExample(@Nonnull SafeExternalLongAlias value) { - this.value = Preconditions.checkNotNull(value, "value cannot be null"); - } - - @JsonValue - public SafeExternalLongAlias get() { - return value; - } - - @Override - public String toString() { - return value.toString(); - } - - @Override - public boolean equals(@Nullable Object other) { - return this == other - || (other instanceof SafeExternalAliasExample - && this.value.equals(((SafeExternalAliasExample) other).value)); - } - - @Override - public int hashCode() { - return value.hashCode(); - } - - public static SafeExternalAliasExample valueOf(String value) { - return of(SafeExternalLongAlias.valueOf(value)); - } - - @JsonCreator(mode = JsonCreator.Mode.DELEGATING) - public static SafeExternalAliasExample of(@Nonnull SafeExternalLongAlias value) { - return new SafeExternalAliasExample(value); - } -} diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java index 75ab7fc43..fadd945e1 100644 --- a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java @@ -1,8 +1,10 @@ package test.prefix.com.palantir.product; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.palantir.logsafe.Preconditions; import com.palantir.logsafe.Safe; @@ -10,6 +12,9 @@ import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import java.util.ArrayList; import java.util.List; +import java.util.Optional; +import java.util.function.Function; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.processing.Generated; @@ -19,8 +24,14 @@ public final class SafeExternalLongExample { private final long safeExternalLongValue; - private SafeExternalLongExample(long safeExternalLongValue) { + private final Optional optionalSafeExternalLong; + + private int memoizedHashCode; + + private SafeExternalLongExample(long safeExternalLongValue, Optional optionalSafeExternalLong) { + validateFields(optionalSafeExternalLong); this.safeExternalLongValue = safeExternalLongValue; + this.optionalSafeExternalLong = optionalSafeExternalLong; } @JsonProperty("safeExternalLongValue") @@ -29,28 +40,72 @@ public long getSafeExternalLongValue() { return this.safeExternalLongValue; } + @JsonProperty("optionalSafeExternalLong") + @JsonInclude(JsonInclude.Include.NON_ABSENT) + public Optional getOptionalSafeExternalLong() { + return this.optionalSafeExternalLong; + } + @Override public boolean equals(@Nullable Object other) { return this == other || (other instanceof SafeExternalLongExample && equalTo((SafeExternalLongExample) other)); } private boolean equalTo(SafeExternalLongExample other) { - return this.safeExternalLongValue == other.safeExternalLongValue; + if (this.memoizedHashCode != 0 + && other.memoizedHashCode != 0 + && this.memoizedHashCode != other.memoizedHashCode) { + return false; + } + return this.safeExternalLongValue == other.safeExternalLongValue + && this.optionalSafeExternalLong.equals(other.optionalSafeExternalLong); } @Override public int hashCode() { - return Long.hashCode(this.safeExternalLongValue); + int result = memoizedHashCode; + if (result == 0) { + int hash = 1; + hash = 31 * hash + Long.hashCode(this.safeExternalLongValue); + hash = 31 * hash + this.optionalSafeExternalLong.hashCode(); + result = hash; + memoizedHashCode = result; + } + return result; } @Override @Safe public String toString() { - return "SafeExternalLongExample{safeExternalLongValue: " + safeExternalLongValue + '}'; + return "SafeExternalLongExample{safeExternalLongValue: " + safeExternalLongValue + + ", optionalSafeExternalLong: " + optionalSafeExternalLong + '}'; + } + + public static SafeExternalLongExample of(@Safe long safeExternalLongValue, long optionalSafeExternalLong) { + return builder() + .safeExternalLongValue(safeExternalLongValue) + .optionalSafeExternalLong(Optional.of(optionalSafeExternalLong)) + .build(); + } + + private static void validateFields(Optional optionalSafeExternalLong) { + List missingFields = null; + missingFields = addFieldIfMissing(missingFields, optionalSafeExternalLong, "optionalSafeExternalLong"); + if (missingFields != null) { + throw new SafeIllegalArgumentException( + "Some required fields have not been set", SafeArg.of("missingFields", missingFields)); + } } - public static SafeExternalLongExample of(@Safe long safeExternalLongValue) { - return builder().safeExternalLongValue(safeExternalLongValue).build(); + private static List addFieldIfMissing(List prev, Object fieldValue, String fieldName) { + List missingFields = prev; + if (fieldValue == null) { + if (missingFields == null) { + missingFields = new ArrayList<>(1); + } + missingFields.add(fieldName); + } + return missingFields; } public static Builder builder() { @@ -64,6 +119,8 @@ public static final class Builder { private long safeExternalLongValue; + private Optional optionalSafeExternalLong = Optional.empty(); + private boolean _safeExternalLongValueInitialized = false; private Builder() {} @@ -71,6 +128,7 @@ private Builder() {} public Builder from(SafeExternalLongExample other) { checkNotBuilt(); safeExternalLongValue(other.getSafeExternalLongValue()); + optionalSafeExternalLong(other.getOptionalSafeExternalLong()); return this; } @@ -82,6 +140,22 @@ public Builder safeExternalLongValue(@Safe long safeExternalLongValue) { return this; } + @JsonSetter(value = "optionalSafeExternalLong", nulls = Nulls.SKIP) + public Builder optionalSafeExternalLong(@Nonnull Optional optionalSafeExternalLong) { + checkNotBuilt(); + this.optionalSafeExternalLong = Preconditions.checkNotNull( + optionalSafeExternalLong, "optionalSafeExternalLong cannot be null") + .map(Function.identity()); + return this; + } + + public Builder optionalSafeExternalLong(long optionalSafeExternalLong) { + checkNotBuilt(); + this.optionalSafeExternalLong = Optional.of( + Preconditions.checkNotNull(optionalSafeExternalLong, "optionalSafeExternalLong cannot be null")); + return this; + } + private void validatePrimitiveFieldsHaveBeenInitialized() { List missingFields = null; missingFields = @@ -107,7 +181,7 @@ public SafeExternalLongExample build() { checkNotBuilt(); this._buildInvoked = true; validatePrimitiveFieldsHaveBeenInitialized(); - return new SafeExternalLongExample(safeExternalLongValue); + return new SafeExternalLongExample(safeExternalLongValue, optionalSafeExternalLong); } private void checkNotBuilt() { diff --git a/conjure-java-core/src/test/resources/example-types.yml b/conjure-java-core/src/test/resources/example-types.yml index 142ddc767..5ab09c621 100644 --- a/conjure-java-core/src/test/resources/example-types.yml +++ b/conjure-java-core/src/test/resources/example-types.yml @@ -33,6 +33,7 @@ types: SafeExternalLongExample: fields: safeExternalLongValue: SafeExternalLong + optionalSafeExternalLong: optional RidExample: fields: ridValue: rid @@ -139,8 +140,6 @@ types: alias: datetime BinaryAliasExample: alias: binary - SafeExternalAliasExample: - alias: SafeExternalLongAlias PrimitiveOptionalsExample: fields: num: optional From 2e72ef53c8987c353ccdac99cd1d61e25277456e Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Wed, 8 Feb 2023 17:27:35 -0500 Subject: [PATCH 07/16] add some tests --- .../conjure/java/ConjureTagsTest.java | 37 -------- .../java/types/ExternalImportSafetyTests.java | 91 +++++++++++++++++++ .../java/types/SafetyEvaluatorTest.java | 88 ------------------ 3 files changed, 91 insertions(+), 125 deletions(-) create mode 100644 conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/ConjureTagsTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/ConjureTagsTest.java index 95ce1b2a4..1bf0fdb0c 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/ConjureTagsTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/ConjureTagsTest.java @@ -223,43 +223,6 @@ void testExternalImport_NoSafety() { assertThat(ConjureTags.validateArgument(argument, safetyEvaluator)).isEmpty(); } - // upstream validation logic should catch this degenerate case, but codegen should still prefer the import time - // annotations - @Test - void testExternalImport_ImportAndUsageTime() { - SafetyEvaluator safetyEvaluator = new SafetyEvaluator(ImmutableMap.of()); - Type external = Type.external(ExternalReference.builder() - .externalReference(TypeName.of("Long", "java.lang")) - .fallback(Type.primitive(PrimitiveType.STRING)) - .safety(LogSafety.DO_NOT_LOG) - .build()); - ArgumentDefinition argument = ArgumentDefinition.builder() - .argName(ArgumentName.of("testArgument")) - .type(external) - .safety(LogSafety.UNSAFE) - .paramType(ParameterType.body(BodyParameterType.of())) - .build(); - assertThat(ConjureTags.validateArgument(argument, safetyEvaluator)).hasValue(LogSafety.DO_NOT_LOG); - } - - // upstream validation logic should catch this degenerate case, but codegen should still prefer the import time - // annotations - @Test - void testExternalImport_UsageTimeOnly() { - SafetyEvaluator safetyEvaluator = new SafetyEvaluator(ImmutableMap.of()); - Type external = Type.external(ExternalReference.builder() - .externalReference(TypeName.of("Long", "java.lang")) - .fallback(Type.primitive(PrimitiveType.STRING)) - .build()); - ArgumentDefinition argument = ArgumentDefinition.builder() - .argName(ArgumentName.of("testArgument")) - .type(external) - .safety(LogSafety.UNSAFE) - .paramType(ParameterType.body(BodyParameterType.of())) - .build(); - assertThat(ConjureTags.validateArgument(argument, safetyEvaluator)).isEmpty(); - } - private static ArgumentDefinition tags(String... tags) { return ArgumentDefinition.builder() .argName(ArgumentName.of("name")) diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java new file mode 100644 index 000000000..bc2a08159 --- /dev/null +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java @@ -0,0 +1,91 @@ +/* + * (c) Copyright 2023 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.conjure.java.types; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.palantir.logsafe.DoNotLog; +import com.palantir.logsafe.Safe; +import com.palantir.product.SafeExternalLongAlias; +import com.palantir.product.SafeExternalLongExample; +import com.palantir.product.UndertowExternalLongTestService; +import java.lang.annotation.Annotation; +import java.lang.reflect.Method; +import java.lang.reflect.Parameter; +import java.util.Arrays; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +public class ExternalImportSafetyTests { + + @Test + public void testAliasAnnotations() { + assertThat(SafeExternalLongAlias.class).hasAnnotation(Safe.class); + + assertMethodHasAnnotation(SafeExternalLongAlias.class, "toString", Safe.class); + assertMethodHasAnnotation(SafeExternalLongAlias.class, "get", Safe.class); + assertMethodParamHasAnnotation(SafeExternalLongAlias.class, "valueOf", "value", Safe.class); + assertMethodParamHasAnnotation(SafeExternalLongAlias.class, "of", "value", Safe.class); + } + + @Test + public void testObjectAnnotations() { + assertThat(SafeExternalLongExample.class).hasAnnotation(Safe.class); + + assertMethodHasAnnotation(SafeExternalLongExample.class, "toString", Safe.class); + assertMethodHasAnnotation(SafeExternalLongExample.class, "getSafeExternalLongValue", Safe.class); + + assertMethodParamHasAnnotation(SafeExternalLongExample.class, "of", "safeExternalLongValue", Safe.class); + + Optional> builder = Arrays.stream(SafeExternalLongExample.class.getClasses()) + .filter(subclass -> subclass.getName().contains("Builder")) + .findAny(); + assertThat(builder).isPresent(); + assertMethodParamHasAnnotation(builder.get(), "safeExternalLongValue", "safeExternalLongValue", Safe.class); + } + + @Test + public void testService() { + assertMethodParamHasAnnotation( + UndertowExternalLongTestService.class, "testExternalLongArg", "externalLong", DoNotLog.class); + } + + private void assertMethodHasAnnotation( + Class parentClass, String methodName, Class annotation) { + Method[] methods = parentClass.getMethods(); + assertThat(methods) + .filteredOn(method -> method.getName().equals(methodName)) + .hasSize(1) + .allMatch(method -> method.isAnnotationPresent(annotation)); + } + + private void assertMethodParamHasAnnotation( + Class parentClass, String methodName, String parameterName, Class annotation) { + Method[] methods = parentClass.getMethods(); + Optional valueOfMethod = Arrays.stream(methods) + .filter(method -> method.getName().equals(methodName)) + .findAny(); + Parameter[] test = valueOfMethod.get().getParameters(); + assertThat(valueOfMethod) + .map(method -> Arrays.stream(test) + .filter(parameter -> parameter.getName().contains(parameterName)) + .findAny()) + .isPresent() + .get() + .satisfies(parameter -> parameter.get().isAnnotationPresent(annotation)); + } +} diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java index 3fc0346b2..a54ec43a0 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java @@ -17,7 +17,6 @@ package com.palantir.conjure.java.types; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.google.common.collect.Iterables; import com.palantir.conjure.defs.SafetyDeclarationRequirements; @@ -177,45 +176,6 @@ void testExternalRefType_NoSafety(TypeDefinition typeDefinition, ConjureDefiniti assertThat(evaluator.evaluate(typeDefinition)).isEmpty(); } - private static Stream providesExternalRefTypes_ImportAndUsageTime() { - Type external = Type.external(ExternalReference.builder() - .externalReference(TypeName.of("Long", "java.lang")) - .fallback(Type.primitive(PrimitiveType.STRING)) - .safety(LogSafety.DO_NOT_LOG) - .build()); - return getTypes_SafetyAtUsageTime(external); - } - - @ParameterizedTest - @MethodSource("providesExternalRefTypes_ImportAndUsageTime") - void testExternalRefType_AtImportAndUsageTime(TypeDefinition typeDefinition, ConjureDefinition conjureDef) { - // upstream validation logic should catch this degenerate case, but codegen should still prefer the import time - // annotations - assertThatThrownBy( - () -> ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED)); - SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); - assertThat(evaluator.evaluate(typeDefinition)).hasValue(LogSafety.DO_NOT_LOG); - } - - private static Stream providesExternalRefTypes_OnlyAtUsageTime() { - Type external = Type.external(ExternalReference.builder() - .externalReference(TypeName.of("Long", "java.lang")) - .fallback(Type.primitive(PrimitiveType.STRING)) - .build()); - return getTypes_SafetyAtUsageTime(external); - } - - @ParameterizedTest - @MethodSource("providesExternalRefTypes_OnlyAtUsageTime") - void testExternalRefType_OnlyAtUsageTime(TypeDefinition typeDefinition, ConjureDefinition conjureDef) { - // upstream validation logic should catch this degenerate case, but codegen should still prefer the import time - // annotations - assertThatThrownBy( - () -> ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED)); - SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); - assertThat(evaluator.evaluate(typeDefinition)).isEmpty(); - } - @Test void testMapSafetyUnsafeValue() { TypeDefinition object = TypeDefinition.object(ObjectDefinition.builder() @@ -449,52 +409,4 @@ private static Stream getTypes_SafetyAtImportTime(Type externalRefere Arguments.of(Named.of("Alias", aliasType), conjureAliasDef), Arguments.of(Named.of("Union", unionType), conjureUnionDef)); } - - private static Stream getTypes_SafetyAtUsageTime(Type externalReference) { - TypeDefinition objectType = TypeDefinition.object(ObjectDefinition.builder() - .typeName(FOO) - .fields(FieldDefinition.builder() - .fieldName(FieldName.of("import")) - .type(externalReference) - .docs(DOCS) - .safety(LogSafety.UNSAFE) - .build()) - .build()); - ConjureDefinition conjureObjectDef = ConjureDefinition.builder() - .version(1) - .types(objectType) - .types(UNSAFE_ALIAS) - .build(); - - TypeDefinition aliasType = TypeDefinition.alias(AliasDefinition.builder() - .typeName(FOO) - .alias(externalReference) - .safety(LogSafety.UNSAFE) - .build()); - ConjureDefinition conjureAliasDef = ConjureDefinition.builder() - .version(1) - .types(aliasType) - .types(UNSAFE_ALIAS) - .build(); - - TypeDefinition unionType = TypeDefinition.union(UnionDefinition.builder() - .union(FieldDefinition.builder() - .fieldName(FieldName.of("importOne")) - .type(externalReference) - .safety(LogSafety.DO_NOT_LOG) - .docs(DOCS) - .build()) - .typeName(FOO) - .build()); - ConjureDefinition conjureUnionDef = ConjureDefinition.builder() - .version(1) - .types(unionType) - .types(UNSAFE_ALIAS) - .build(); - - return Stream.of( - Arguments.of(Named.of("Object", objectType), conjureObjectDef), - Arguments.of(Named.of("Alias", aliasType), conjureAliasDef), - Arguments.of(Named.of("Union", unionType), conjureUnionDef)); - } } From ec54b4c235be374aab859e39d4d9aad6f9b501e3 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Wed, 8 Feb 2023 18:29:32 -0500 Subject: [PATCH 08/16] now i know how reflection works i guess --- .../product/ExternalLongUnionExample.java | 87 ++---------------- .../product/ExternalLongUnionExample.java | 86 +---------------- .../java/types/ExternalImportSafetyTests.java | 92 +++++++++++++++++-- .../src/test/resources/example-types.yml | 3 +- 4 files changed, 98 insertions(+), 170 deletions(-) diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java index 21659a397..33fdc7076 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java @@ -24,7 +24,7 @@ import javax.annotation.processing.Generated; /** - * A union of a safe and unsafe external long. + * A union of a safe long. */ @Generated("com.palantir.conjure.java.types.UnionGenerator") public final class ExternalLongUnionExample { @@ -44,18 +44,11 @@ public static ExternalLongUnionExample safeLong(@Safe long value) { return new ExternalLongUnionExample(new SafeLongWrapper(value)); } - public static ExternalLongUnionExample unsafeLong(long value) { - return new ExternalLongUnionExample(new UnsafeLongWrapper(value)); - } - public static ExternalLongUnionExample unknown(@Safe String type, Object value) { switch (Preconditions.checkNotNull(type, "Type is required")) { case "safeLong": throw new SafeIllegalArgumentException( "Unknown type cannot be created as the provided type is known: safeLong"); - case "unsafeLong": - throw new SafeIllegalArgumentException( - "Unknown type cannot be created as the provided type is known: unsafeLong"); default: return new ExternalLongUnionExample(new UnknownWrapper(type, Collections.singletonMap(type, value))); } @@ -88,8 +81,6 @@ public String toString() { public interface Visitor { T visitSafeLong(@Safe long value); - T visitUnsafeLong(long value); - T visitUnknown(@Safe String unknownType, Object unknownValue); static SafeLongStageVisitorBuilder builder() { @@ -98,30 +89,18 @@ static SafeLongStageVisitorBuilder builder() { } private static final class VisitorBuilder - implements SafeLongStageVisitorBuilder, - UnsafeLongStageVisitorBuilder, - UnknownStageVisitorBuilder, - Completed_StageVisitorBuilder { + implements SafeLongStageVisitorBuilder, UnknownStageVisitorBuilder, Completed_StageVisitorBuilder { private Function<@Safe Long, T> safeLongVisitor; - private Function unsafeLongVisitor; - private BiFunction<@Safe String, Object, T> unknownVisitor; @Override - public UnsafeLongStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor) { + public UnknownStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor) { Preconditions.checkNotNull(safeLongVisitor, "safeLongVisitor cannot be null"); this.safeLongVisitor = safeLongVisitor; return this; } - @Override - public UnknownStageVisitorBuilder unsafeLong(@Nonnull Function unsafeLongVisitor) { - Preconditions.checkNotNull(unsafeLongVisitor, "unsafeLongVisitor cannot be null"); - this.unsafeLongVisitor = unsafeLongVisitor; - return this; - } - @Override public Completed_StageVisitorBuilder unknown(@Nonnull BiFunction<@Safe String, Object, T> unknownVisitor) { Preconditions.checkNotNull(unknownVisitor, "unknownVisitor cannot be null"); @@ -148,8 +127,8 @@ public Completed_StageVisitorBuilder throwOnUnknown() { @Override public Visitor build() { + // TODO: can i skip this??? final Function<@Safe Long, T> safeLongVisitor = this.safeLongVisitor; - final Function unsafeLongVisitor = this.unsafeLongVisitor; final BiFunction<@Safe String, Object, T> unknownVisitor = this.unknownVisitor; return new Visitor() { @Override @@ -157,11 +136,6 @@ public T visitSafeLong(long value) { return safeLongVisitor.apply(value); } - @Override - public T visitUnsafeLong(long value) { - return unsafeLongVisitor.apply(value); - } - @Override public T visitUnknown(String unknownType, Object unknownValue) { return unknownVisitor.apply(unknownType, unknownValue); @@ -171,11 +145,7 @@ public T visitUnknown(String unknownType, Object unknownValue) { } public interface SafeLongStageVisitorBuilder { - UnsafeLongStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor); - } - - public interface UnsafeLongStageVisitorBuilder { - UnknownStageVisitorBuilder unsafeLong(@Nonnull Function unsafeLongVisitor); + UnknownStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor); } public interface UnknownStageVisitorBuilder { @@ -196,7 +166,7 @@ public interface Completed_StageVisitorBuilder { property = "type", visible = true, defaultImpl = UnknownWrapper.class) - @JsonSubTypes({@JsonSubTypes.Type(SafeLongWrapper.class), @JsonSubTypes.Type(UnsafeLongWrapper.class)}) + @JsonSubTypes(@JsonSubTypes.Type(SafeLongWrapper.class)) @JsonIgnoreProperties(ignoreUnknown = true) private interface Base { T accept(Visitor visitor); @@ -247,51 +217,6 @@ public String toString() { } } - @JsonTypeName("unsafeLong") - private static final class UnsafeLongWrapper implements Base { - private final long value; - - @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) - private UnsafeLongWrapper(@JsonSetter("unsafeLong") @Nonnull long value) { - Preconditions.checkNotNull(value, "unsafeLong cannot be null"); - this.value = value; - } - - @JsonProperty(value = "type", index = 0) - private String getType() { - return "unsafeLong"; - } - - @JsonProperty("unsafeLong") - private long getValue() { - return value; - } - - @Override - public T accept(Visitor visitor) { - return visitor.visitUnsafeLong(value); - } - - @Override - public boolean equals(@Nullable Object other) { - return this == other || (other instanceof UnsafeLongWrapper && equalTo((UnsafeLongWrapper) other)); - } - - private boolean equalTo(UnsafeLongWrapper other) { - return this.value == other.value; - } - - @Override - public int hashCode() { - return Long.hashCode(this.value); - } - - @Override - public String toString() { - return "UnsafeLongWrapper{value: " + value + '}'; - } - } - private static final class UnknownWrapper implements Base { private final String type; diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ExternalLongUnionExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ExternalLongUnionExample.java index 624442ef9..498c0b6a9 100644 --- a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ExternalLongUnionExample.java +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/ExternalLongUnionExample.java @@ -23,7 +23,7 @@ import javax.annotation.processing.Generated; /** - * A union of a safe and unsafe external long. + * A union of a safe long. */ @Generated("com.palantir.conjure.java.types.UnionGenerator") public final class ExternalLongUnionExample { @@ -43,18 +43,11 @@ public static ExternalLongUnionExample safeLong(@Safe long value) { return new ExternalLongUnionExample(new SafeLongWrapper(value)); } - public static ExternalLongUnionExample unsafeLong(long value) { - return new ExternalLongUnionExample(new UnsafeLongWrapper(value)); - } - public static ExternalLongUnionExample unknown(@Safe String type, Object value) { switch (Preconditions.checkNotNull(type, "Type is required")) { case "safeLong": throw new SafeIllegalArgumentException( "Unknown type cannot be created as the provided type is known: safeLong"); - case "unsafeLong": - throw new SafeIllegalArgumentException( - "Unknown type cannot be created as the provided type is known: unsafeLong"); default: return new ExternalLongUnionExample(new UnknownWrapper(type, Collections.singletonMap(type, value))); } @@ -87,8 +80,6 @@ public String toString() { public interface Visitor { T visitSafeLong(@Safe long value); - T visitUnsafeLong(long value); - T visitUnknown(@Safe String unknownType); static SafeLongStageVisitorBuilder builder() { @@ -97,30 +88,18 @@ static SafeLongStageVisitorBuilder builder() { } private static final class VisitorBuilder - implements SafeLongStageVisitorBuilder, - UnsafeLongStageVisitorBuilder, - UnknownStageVisitorBuilder, - Completed_StageVisitorBuilder { + implements SafeLongStageVisitorBuilder, UnknownStageVisitorBuilder, Completed_StageVisitorBuilder { private Function<@Safe Long, T> safeLongVisitor; - private Function unsafeLongVisitor; - private Function unknownVisitor; @Override - public UnsafeLongStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor) { + public UnknownStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor) { Preconditions.checkNotNull(safeLongVisitor, "safeLongVisitor cannot be null"); this.safeLongVisitor = safeLongVisitor; return this; } - @Override - public UnknownStageVisitorBuilder unsafeLong(@Nonnull Function unsafeLongVisitor) { - Preconditions.checkNotNull(unsafeLongVisitor, "unsafeLongVisitor cannot be null"); - this.unsafeLongVisitor = unsafeLongVisitor; - return this; - } - @Override public Completed_StageVisitorBuilder unknown(@Nonnull Function unknownVisitor) { Preconditions.checkNotNull(unknownVisitor, "unknownVisitor cannot be null"); @@ -141,7 +120,6 @@ public Completed_StageVisitorBuilder throwOnUnknown() { @Override public Visitor build() { final Function<@Safe Long, T> safeLongVisitor = this.safeLongVisitor; - final Function unsafeLongVisitor = this.unsafeLongVisitor; final Function unknownVisitor = this.unknownVisitor; return new Visitor() { @Override @@ -149,11 +127,6 @@ public T visitSafeLong(long value) { return safeLongVisitor.apply(value); } - @Override - public T visitUnsafeLong(long value) { - return unsafeLongVisitor.apply(value); - } - @Override public T visitUnknown(String value) { return unknownVisitor.apply(value); @@ -163,11 +136,7 @@ public T visitUnknown(String value) { } public interface SafeLongStageVisitorBuilder { - UnsafeLongStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor); - } - - public interface UnsafeLongStageVisitorBuilder { - UnknownStageVisitorBuilder unsafeLong(@Nonnull Function unsafeLongVisitor); + UnknownStageVisitorBuilder safeLong(@Nonnull Function<@Safe Long, T> safeLongVisitor); } public interface UnknownStageVisitorBuilder { @@ -186,7 +155,7 @@ public interface Completed_StageVisitorBuilder { property = "type", visible = true, defaultImpl = UnknownWrapper.class) - @JsonSubTypes({@JsonSubTypes.Type(SafeLongWrapper.class), @JsonSubTypes.Type(UnsafeLongWrapper.class)}) + @JsonSubTypes(@JsonSubTypes.Type(SafeLongWrapper.class)) @JsonIgnoreProperties(ignoreUnknown = true) private interface Base { T accept(Visitor visitor); @@ -237,51 +206,6 @@ public String toString() { } } - @JsonTypeName("unsafeLong") - private static final class UnsafeLongWrapper implements Base { - private final long value; - - @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) - private UnsafeLongWrapper(@JsonSetter("unsafeLong") @Nonnull long value) { - Preconditions.checkNotNull(value, "unsafeLong cannot be null"); - this.value = value; - } - - @JsonProperty(value = "type", index = 0) - private String getType() { - return "unsafeLong"; - } - - @JsonProperty("unsafeLong") - private long getValue() { - return value; - } - - @Override - public T accept(Visitor visitor) { - return visitor.visitUnsafeLong(value); - } - - @Override - public boolean equals(@Nullable Object other) { - return this == other || (other instanceof UnsafeLongWrapper && equalTo((UnsafeLongWrapper) other)); - } - - private boolean equalTo(UnsafeLongWrapper other) { - return this.value == other.value; - } - - @Override - public int hashCode() { - return Long.hashCode(this.value); - } - - @Override - public String toString() { - return "UnsafeLongWrapper{value: " + value + '}'; - } - } - private static final class UnknownWrapper implements Base { private final String type; diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java index bc2a08159..55afc0b62 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java @@ -20,14 +20,19 @@ import com.palantir.logsafe.DoNotLog; import com.palantir.logsafe.Safe; +import com.palantir.product.ExternalLongUnionExample; import com.palantir.product.SafeExternalLongAlias; import com.palantir.product.SafeExternalLongExample; import com.palantir.product.UndertowExternalLongTestService; import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedParameterizedType; +import java.lang.reflect.AnnotatedType; import java.lang.reflect.Method; -import java.lang.reflect.Parameter; import java.util.Arrays; +import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; public class ExternalImportSafetyTests { @@ -59,11 +64,63 @@ public void testObjectAnnotations() { } @Test - public void testService() { + public void testServiceAnnotations() { assertMethodParamHasAnnotation( UndertowExternalLongTestService.class, "testExternalLongArg", "externalLong", DoNotLog.class); } + @Test + public void testUnionAnnotations() { + assertMethodParamHasAnnotation(ExternalLongUnionExample.class, "safeLong", "value", Safe.class); + assertMethodParamHasAnnotation(ExternalLongUnionExample.class, "unknown", "type", Safe.class); + + Optional> visitor = Arrays.stream(ExternalLongUnionExample.class.getClasses()) + .filter(subclass -> subclass.getName().contains("$Visitor")) + .findAny(); + assertThat(visitor).isPresent(); + assertMethodParamHasAnnotation(visitor.get(), "visitSafeLong", "value", Safe.class); + assertMethodParamHasAnnotation(visitor.get(), "visitUnknown", "unknownType", Safe.class); + + Optional> visitorBuilder = Arrays.stream(ExternalLongUnionExample.class.getDeclaredClasses()) + .filter(subclass -> subclass.getName().contains("$VisitorBuilder")) + .findAny(); + assertThat(visitorBuilder).isPresent(); + assertFieldTypeParamHasAnnotation(visitorBuilder.get(), "safeLongVisitor", "Long", Safe.class); + assertFieldTypeParamHasAnnotation(visitorBuilder.get(), "unknownVisitor", "String", Safe.class); + assertMethodParamWithTypeParameterHasAnnotation( + visitorBuilder.get(), "safeLong", "safeLongVisitor", "Long", Safe.class); + assertMethodParamWithTypeParameterHasAnnotation( + visitorBuilder.get(), "unknown", "unknownVisitor", "Long", Safe.class); + + Optional> stageVisitorBuilder = Arrays.stream(ExternalLongUnionExample.class.getDeclaredClasses()) + .filter(subclass -> subclass.getName().contains("SafeLongStageVisitorBuilder")) + .findAny(); + assertThat(stageVisitorBuilder).isPresent(); + assertMethodParamWithTypeParameterHasAnnotation( + stageVisitorBuilder.get(), "safeLong", "safeLongVisitor", "Long", Safe.class); + + Optional> unknownStageVisitorBuilder = Arrays.stream( + ExternalLongUnionExample.class.getDeclaredClasses()) + .filter(subclass -> subclass.getName().contains("UnknownStageVisitorBuilder")) + .findAny(); + assertThat(unknownStageVisitorBuilder).isPresent(); + assertMethodParamWithTypeParameterHasAnnotation( + unknownStageVisitorBuilder.get(), "unknown", "unknownVisitor", "Long", Safe.class); + } + + private void assertFieldTypeParamHasAnnotation( + Class parentClass, String fieldName, String typeName, Class annotation) { + AnnotatedType type = Arrays.stream(parentClass.getDeclaredFields()) + .filter(field -> field.getName().contains(fieldName)) + .findAny() + .get() + .getAnnotatedType(); + assertThat(((AnnotatedParameterizedType) type).getAnnotatedActualTypeArguments()) + .filteredOn(t -> t.getType().getTypeName().contains(typeName)) + .hasSize(1) + .allMatch(t -> t.isAnnotationPresent(annotation)); + } + private void assertMethodHasAnnotation( Class parentClass, String methodName, Class annotation) { Method[] methods = parentClass.getMethods(); @@ -76,16 +133,39 @@ private void assertMethodHasAnnotation( private void assertMethodParamHasAnnotation( Class parentClass, String methodName, String parameterName, Class annotation) { Method[] methods = parentClass.getMethods(); - Optional valueOfMethod = Arrays.stream(methods) + Optional desiredMethod = Arrays.stream(methods) .filter(method -> method.getName().equals(methodName)) .findAny(); - Parameter[] test = valueOfMethod.get().getParameters(); - assertThat(valueOfMethod) - .map(method -> Arrays.stream(test) + assertThat(desiredMethod).isPresent(); + assertThat(desiredMethod) + .map(method -> Arrays.stream(desiredMethod.get().getParameters()) .filter(parameter -> parameter.getName().contains(parameterName)) .findAny()) .isPresent() .get() .satisfies(parameter -> parameter.get().isAnnotationPresent(annotation)); } + + private void assertMethodParamWithTypeParameterHasAnnotation( + Class parentClass, + String methodName, + String parameterName, + String typeParameter, + Class annotation) { + Method[] methods = parentClass.getMethods(); + List desiredMethods = Arrays.stream(methods) + .filter(method -> method.getName().equals(methodName)) + .collect(Collectors.toList()); + assertThat(desiredMethods).isNotEmpty(); + Stream annotatedTypes = desiredMethods.stream() + .map(method -> Arrays.stream(method.getParameters()) + .filter(parameter -> parameter.getName().contains(parameterName)) + .findAny() + .get() + .getAnnotatedType()); + assertThat(annotatedTypes).allMatch(annotatedType -> Arrays.stream( + ((AnnotatedParameterizedType) annotatedType).getAnnotatedActualTypeArguments()) + .filter(t -> t.getType().getTypeName().contains(typeParameter)) + .allMatch(t -> t.isAnnotationPresent(annotation))); + } } diff --git a/conjure-java-core/src/test/resources/example-types.yml b/conjure-java-core/src/test/resources/example-types.yml index 5ab09c621..c80b9263f 100644 --- a/conjure-java-core/src/test/resources/example-types.yml +++ b/conjure-java-core/src/test/resources/example-types.yml @@ -224,10 +224,9 @@ types: EmptyUnionTypeExample: union: {} ExternalLongUnionExample: - docs: A union of a safe and unsafe external long. + docs: A union of a safe long. union: safeLong: SafeExternalLong - unsafeLong: ExternalLong EmptyObjectExample: fields: {} AliasAsMapKeyExample: From a3356d1ea6976141b07b2aed7a0a602fa638f19c Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Thu, 9 Feb 2023 14:13:33 -0500 Subject: [PATCH 09/16] fix broken test --- .../java/com/palantir/product/ExternalLongUnionExample.java | 1 - 1 file changed, 1 deletion(-) diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java index 33fdc7076..dd58e391e 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/ExternalLongUnionExample.java @@ -127,7 +127,6 @@ public Completed_StageVisitorBuilder throwOnUnknown() { @Override public Visitor build() { - // TODO: can i skip this??? final Function<@Safe Long, T> safeLongVisitor = this.safeLongVisitor; final BiFunction<@Safe String, Object, T> unknownVisitor = this.unknownVisitor; return new Visitor() { From e29e2d88826526d0e82d1aec7df1208a217530b2 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Thu, 9 Feb 2023 15:06:38 -0500 Subject: [PATCH 10/16] refactoring test --- .../product/SafeExternalAliasExample.java | 50 ------ .../java/types/ExternalImportSafetyTests.java | 164 ++++++++++-------- .../java/types/SafetyEvaluatorTest.java | 1 - 3 files changed, 88 insertions(+), 127 deletions(-) delete mode 100644 conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java deleted file mode 100644 index 6f434d818..000000000 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalAliasExample.java +++ /dev/null @@ -1,50 +0,0 @@ -package com.palantir.product; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonValue; -import com.palantir.logsafe.Preconditions; -import com.palantir.logsafe.Safe; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import javax.annotation.processing.Generated; - -@Safe -@Generated("com.palantir.conjure.java.types.AliasGenerator") -public final class SafeExternalAliasExample { - private final SafeExternalLongAlias value; - - private SafeExternalAliasExample(@Nonnull SafeExternalLongAlias value) { - this.value = Preconditions.checkNotNull(value, "value cannot be null"); - } - - @JsonValue - public SafeExternalLongAlias get() { - return value; - } - - @Override - public String toString() { - return value.toString(); - } - - @Override - public boolean equals(@Nullable Object other) { - return this == other - || (other instanceof SafeExternalAliasExample - && this.value.equals(((SafeExternalAliasExample) other).value)); - } - - @Override - public int hashCode() { - return value.hashCode(); - } - - public static SafeExternalAliasExample valueOf(String value) { - return of(SafeExternalLongAlias.valueOf(value)); - } - - @JsonCreator(mode = JsonCreator.Mode.DELEGATING) - public static SafeExternalAliasExample of(@Nonnull SafeExternalLongAlias value) { - return new SafeExternalAliasExample(value); - } -} diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java index 55afc0b62..05250f495 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java @@ -27,11 +27,11 @@ import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedParameterizedType; import java.lang.reflect.AnnotatedType; +import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.lang.reflect.Parameter; import java.util.Arrays; -import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -56,11 +56,8 @@ public void testObjectAnnotations() { assertMethodParamHasAnnotation(SafeExternalLongExample.class, "of", "safeExternalLongValue", Safe.class); - Optional> builder = Arrays.stream(SafeExternalLongExample.class.getClasses()) - .filter(subclass -> subclass.getName().contains("Builder")) - .findAny(); - assertThat(builder).isPresent(); - assertMethodParamHasAnnotation(builder.get(), "safeExternalLongValue", "safeExternalLongValue", Safe.class); + Class builder = getMatchingSubclass(SafeExternalLongExample.class, "Builder"); + assertMethodParamHasAnnotation(builder, "safeExternalLongValue", "safeExternalLongValue", Safe.class); } @Test @@ -74,76 +71,48 @@ public void testUnionAnnotations() { assertMethodParamHasAnnotation(ExternalLongUnionExample.class, "safeLong", "value", Safe.class); assertMethodParamHasAnnotation(ExternalLongUnionExample.class, "unknown", "type", Safe.class); - Optional> visitor = Arrays.stream(ExternalLongUnionExample.class.getClasses()) - .filter(subclass -> subclass.getName().contains("$Visitor")) - .findAny(); - assertThat(visitor).isPresent(); - assertMethodParamHasAnnotation(visitor.get(), "visitSafeLong", "value", Safe.class); - assertMethodParamHasAnnotation(visitor.get(), "visitUnknown", "unknownType", Safe.class); + Class visitor = getMatchingSubclass(ExternalLongUnionExample.class, "$Visitor"); + assertMethodParamHasAnnotation(visitor, "visitSafeLong", "value", Safe.class); + assertMethodParamHasAnnotation(visitor, "visitUnknown", "unknownType", Safe.class); - Optional> visitorBuilder = Arrays.stream(ExternalLongUnionExample.class.getDeclaredClasses()) - .filter(subclass -> subclass.getName().contains("$VisitorBuilder")) - .findAny(); - assertThat(visitorBuilder).isPresent(); - assertFieldTypeParamHasAnnotation(visitorBuilder.get(), "safeLongVisitor", "Long", Safe.class); - assertFieldTypeParamHasAnnotation(visitorBuilder.get(), "unknownVisitor", "String", Safe.class); + Class visitorBuilder = getMatchingSubclass(ExternalLongUnionExample.class, "$VisitorBuilder"); + assertFieldTypeParamHasAnnotation(visitorBuilder, "safeLongVisitor", "Long", Safe.class); + assertFieldTypeParamHasAnnotation(visitorBuilder, "unknownVisitor", "String", Safe.class); assertMethodParamWithTypeParameterHasAnnotation( - visitorBuilder.get(), "safeLong", "safeLongVisitor", "Long", Safe.class); + visitorBuilder, "safeLong", "safeLongVisitor", "Long", Safe.class); assertMethodParamWithTypeParameterHasAnnotation( - visitorBuilder.get(), "unknown", "unknownVisitor", "Long", Safe.class); + visitorBuilder, "unknown", "unknownVisitor", "String", Safe.class); - Optional> stageVisitorBuilder = Arrays.stream(ExternalLongUnionExample.class.getDeclaredClasses()) - .filter(subclass -> subclass.getName().contains("SafeLongStageVisitorBuilder")) - .findAny(); - assertThat(stageVisitorBuilder).isPresent(); + Class stageVisitorBuilder = + getMatchingSubclass(ExternalLongUnionExample.class, "SafeLongStageVisitorBuilder"); assertMethodParamWithTypeParameterHasAnnotation( - stageVisitorBuilder.get(), "safeLong", "safeLongVisitor", "Long", Safe.class); + stageVisitorBuilder, "safeLong", "safeLongVisitor", "Long", Safe.class); - Optional> unknownStageVisitorBuilder = Arrays.stream( - ExternalLongUnionExample.class.getDeclaredClasses()) - .filter(subclass -> subclass.getName().contains("UnknownStageVisitorBuilder")) - .findAny(); - assertThat(unknownStageVisitorBuilder).isPresent(); + Class unknownStageVisitorBuilder = + getMatchingSubclass(ExternalLongUnionExample.class, "UnknownStageVisitorBuilder"); assertMethodParamWithTypeParameterHasAnnotation( - unknownStageVisitorBuilder.get(), "unknown", "unknownVisitor", "Long", Safe.class); - } - - private void assertFieldTypeParamHasAnnotation( - Class parentClass, String fieldName, String typeName, Class annotation) { - AnnotatedType type = Arrays.stream(parentClass.getDeclaredFields()) - .filter(field -> field.getName().contains(fieldName)) - .findAny() - .get() - .getAnnotatedType(); - assertThat(((AnnotatedParameterizedType) type).getAnnotatedActualTypeArguments()) - .filteredOn(t -> t.getType().getTypeName().contains(typeName)) - .hasSize(1) - .allMatch(t -> t.isAnnotationPresent(annotation)); + unknownStageVisitorBuilder, "unknown", "unknownVisitor", "String", Safe.class); } private void assertMethodHasAnnotation( Class parentClass, String methodName, Class annotation) { - Method[] methods = parentClass.getMethods(); - assertThat(methods) - .filteredOn(method -> method.getName().equals(methodName)) - .hasSize(1) + Stream desiredMethods = getMatchingMethods(parentClass, methodName); + assertThat(desiredMethods) + .withFailMessage(String.format( + "Expected %s:%s to have annotation %s", + parentClass.getName(), methodName, annotation.getName())) .allMatch(method -> method.isAnnotationPresent(annotation)); } private void assertMethodParamHasAnnotation( Class parentClass, String methodName, String parameterName, Class annotation) { - Method[] methods = parentClass.getMethods(); - Optional desiredMethod = Arrays.stream(methods) - .filter(method -> method.getName().equals(methodName)) - .findAny(); - assertThat(desiredMethod).isPresent(); - assertThat(desiredMethod) - .map(method -> Arrays.stream(desiredMethod.get().getParameters()) - .filter(parameter -> parameter.getName().contains(parameterName)) - .findAny()) - .isPresent() - .get() - .satisfies(parameter -> parameter.get().isAnnotationPresent(annotation)); + Stream desiredMethods = getMatchingMethods(parentClass, methodName); + assertThat(desiredMethods) + .withFailMessage(String.format( + "Expected %s:%s parameter %s to have annotation %s", + parentClass.getName(), methodName, parameterName, annotation.getName())) + .map(method -> getMatchingParameter(method, parameterName)) + .allMatch(parameter -> parameter.isAnnotationPresent(annotation)); } private void assertMethodParamWithTypeParameterHasAnnotation( @@ -152,20 +121,63 @@ private void assertMethodParamWithTypeParameterHasAnnotation( String parameterName, String typeParameter, Class annotation) { - Method[] methods = parentClass.getMethods(); - List desiredMethods = Arrays.stream(methods) - .filter(method -> method.getName().equals(methodName)) - .collect(Collectors.toList()); - assertThat(desiredMethods).isNotEmpty(); - Stream annotatedTypes = desiredMethods.stream() - .map(method -> Arrays.stream(method.getParameters()) - .filter(parameter -> parameter.getName().contains(parameterName)) - .findAny() - .get() - .getAnnotatedType()); - assertThat(annotatedTypes).allMatch(annotatedType -> Arrays.stream( - ((AnnotatedParameterizedType) annotatedType).getAnnotatedActualTypeArguments()) - .filter(t -> t.getType().getTypeName().contains(typeParameter)) - .allMatch(t -> t.isAnnotationPresent(annotation))); + Stream desiredMethods = getMatchingMethods(parentClass, methodName); + Stream annotatedTypes = desiredMethods.map( + method -> getMatchingParameter(method, parameterName).getAnnotatedType()); + assertThat(annotatedTypes) + .withFailMessage(String.format( + "Expected %s:%s parameter %s of type %s to have annotation %s", + parentClass.getName(), methodName, parameterName, typeParameter, annotation.getName())) + .map(annotatedType -> getAnnotatedTypeParameter(annotatedType, typeParameter)) + .allMatch(t -> t.isAnnotationPresent(annotation)); + } + + private void assertFieldTypeParamHasAnnotation( + Class parentClass, String fieldName, String typeName, Class annotation) { + Stream desiredFields = Arrays.stream(parentClass.getDeclaredFields()) + .filter(field -> field.getName().contains(fieldName)); + Stream annotatedTypeParameters = + desiredFields.map(Field::getAnnotatedType).map(type -> getAnnotatedTypeParameter(type, typeName)); + assertThat(annotatedTypeParameters) + .withFailMessage(String.format( + "Expected %s:%s of type %s to have annotation %s", + parentClass.getName(), fieldName, typeName, annotation.getName())) + .allMatch(t -> t.isAnnotationPresent(annotation)); + } + + private Stream getMatchingMethods(Class parentClass, String methodName) { + return Arrays.stream(parentClass.getMethods()) + .filter(method -> method.getName().equals(methodName)); + } + + private Class getMatchingSubclass(Class parentClass, String subclassName) { + Optional> subclassIfExists = Arrays.stream(SafeExternalLongExample.class.getClasses()) + .filter(subclass -> subclass.getName().contains("Builder")) + .findAny(); + assertThat(subclassIfExists) + .withFailMessage(String.format("Expected %s:%s to exist", parentClass, subclassName)) + .isPresent(); + return subclassIfExists.get(); + } + + private Parameter getMatchingParameter(Method method, String parameterName) { + Optional parameterIfExists = Arrays.stream(method.getParameters()) + .filter(parameter -> parameter.getName().contains(parameterName)) + .findAny(); + assertThat(parameterIfExists) + .withFailMessage(String.format("Expected to find parameter %s on method %s", parameterName, method)) + .isPresent(); + return parameterIfExists.get(); + } + + private AnnotatedType getAnnotatedTypeParameter(AnnotatedType parameterizedType, String parameter) { + Optional typeParameterIfExists = Arrays.stream( + ((AnnotatedParameterizedType) parameterizedType).getAnnotatedActualTypeArguments()) + .filter(t -> t.getType().getTypeName().contains(parameter)) + .findAny(); + assertThat(typeParameterIfExists) + .withFailMessage("Expected type parameter %s on type %s", parameter, parameterizedType) + .isPresent(); + return typeParameterIfExists.get(); } } diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java index a54ec43a0..095e38a91 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java @@ -137,7 +137,6 @@ void testMapSafeKey() { .types(object) .types(SAFE_ALIAS) .build(); - // there's an ABI break between 4.28.0 -> 4.36.0 ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); SafetyEvaluator evaluator = new SafetyEvaluator(conjureDef); assertThat(evaluator.evaluate(object)).isEmpty(); From f05558efedcae6f62bc576d6504d893b9177ce54 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Fri, 10 Feb 2023 14:46:10 -0500 Subject: [PATCH 11/16] playing around with wrappers --- .../product/SafeExternalLongExample.java | 113 +++++++++++++++--- .../product/SafeExternalLongExample.java | 113 +++++++++++++++--- .../BeanBuilderAuxiliarySettersUtils.java | 17 ++- .../java/types/BeanBuilderGenerator.java | 6 +- .../conjure/java/util/SafetyUtils.java | 110 +++++++++++++++-- .../java/types/ExternalImportSafetyTests.java | 6 + .../java/types/ObjectGeneratorTests.java | 15 +++ .../src/test/resources/example-types.yml | 13 ++ 8 files changed, 347 insertions(+), 46 deletions(-) diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java index 841063df7..3288f5ec7 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java @@ -5,13 +5,17 @@ import com.fasterxml.jackson.annotation.JsonSetter; import com.fasterxml.jackson.annotation.Nulls; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.palantir.conjure.java.lib.internal.ConjureCollections; import com.palantir.logsafe.Preconditions; import com.palantir.logsafe.Safe; import com.palantir.logsafe.SafeArg; import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.function.Function; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -25,12 +29,22 @@ public final class SafeExternalLongExample { private final Optional optionalSafeExternalLong; + private final List safeExternalLongList; + + private final Set safeExternalLongSet; + private int memoizedHashCode; - private SafeExternalLongExample(long safeExternalLongValue, Optional optionalSafeExternalLong) { - validateFields(optionalSafeExternalLong); + private SafeExternalLongExample( + long safeExternalLongValue, + Optional optionalSafeExternalLong, + List safeExternalLongList, + Set safeExternalLongSet) { + validateFields(optionalSafeExternalLong, safeExternalLongList, safeExternalLongSet); this.safeExternalLongValue = safeExternalLongValue; this.optionalSafeExternalLong = optionalSafeExternalLong; + this.safeExternalLongList = Collections.unmodifiableList(safeExternalLongList); + this.safeExternalLongSet = Collections.unmodifiableSet(safeExternalLongSet); } @JsonProperty("safeExternalLongValue") @@ -40,11 +54,24 @@ public long getSafeExternalLongValue() { } @JsonProperty("optionalSafeExternalLong") + @Safe @JsonInclude(JsonInclude.Include.NON_ABSENT) public Optional getOptionalSafeExternalLong() { return this.optionalSafeExternalLong; } + @JsonProperty("safeExternalLongList") + @Safe + public List getSafeExternalLongList() { + return this.safeExternalLongList; + } + + @JsonProperty("safeExternalLongSet") + @Safe + public Set getSafeExternalLongSet() { + return this.safeExternalLongSet; + } + @Override public boolean equals(@Nullable Object other) { return this == other || (other instanceof SafeExternalLongExample && equalTo((SafeExternalLongExample) other)); @@ -57,7 +84,9 @@ private boolean equalTo(SafeExternalLongExample other) { return false; } return this.safeExternalLongValue == other.safeExternalLongValue - && this.optionalSafeExternalLong.equals(other.optionalSafeExternalLong); + && this.optionalSafeExternalLong.equals(other.optionalSafeExternalLong) + && this.safeExternalLongList.equals(other.safeExternalLongList) + && this.safeExternalLongSet.equals(other.safeExternalLongSet); } @Override @@ -67,6 +96,8 @@ public int hashCode() { int hash = 1; hash = 31 * hash + Long.hashCode(this.safeExternalLongValue); hash = 31 * hash + this.optionalSafeExternalLong.hashCode(); + hash = 31 * hash + this.safeExternalLongList.hashCode(); + hash = 31 * hash + this.safeExternalLongSet.hashCode(); result = hash; memoizedHashCode = result; } @@ -77,19 +108,16 @@ public int hashCode() { @Safe public String toString() { return "SafeExternalLongExample{safeExternalLongValue: " + safeExternalLongValue - + ", optionalSafeExternalLong: " + optionalSafeExternalLong + '}'; + + ", optionalSafeExternalLong: " + optionalSafeExternalLong + ", safeExternalLongList: " + + safeExternalLongList + ", safeExternalLongSet: " + safeExternalLongSet + '}'; } - public static SafeExternalLongExample of(@Safe long safeExternalLongValue, long optionalSafeExternalLong) { - return builder() - .safeExternalLongValue(safeExternalLongValue) - .optionalSafeExternalLong(Optional.of(optionalSafeExternalLong)) - .build(); - } - - private static void validateFields(Optional optionalSafeExternalLong) { + private static void validateFields( + Optional optionalSafeExternalLong, List safeExternalLongList, Set safeExternalLongSet) { List missingFields = null; missingFields = addFieldIfMissing(missingFields, optionalSafeExternalLong, "optionalSafeExternalLong"); + missingFields = addFieldIfMissing(missingFields, safeExternalLongList, "safeExternalLongList"); + missingFields = addFieldIfMissing(missingFields, safeExternalLongSet, "safeExternalLongSet"); if (missingFields != null) { throw new SafeIllegalArgumentException( "Some required fields have not been set", SafeArg.of("missingFields", missingFields)); @@ -100,7 +128,7 @@ private static List addFieldIfMissing(List prev, Object fieldVal List missingFields = prev; if (fieldValue == null) { if (missingFields == null) { - missingFields = new ArrayList<>(1); + missingFields = new ArrayList<>(3); } missingFields.add(fieldName); } @@ -117,7 +145,11 @@ public static final class Builder { private long safeExternalLongValue; - private Optional optionalSafeExternalLong = Optional.empty(); + private Optional<@Safe Long> optionalSafeExternalLong = Optional.empty(); + + private List<@Safe Long> safeExternalLongList = new ArrayList<>(); + + private Set<@Safe Long> safeExternalLongSet = new LinkedHashSet<>(); private boolean _safeExternalLongValueInitialized = false; @@ -127,6 +159,8 @@ public Builder from(SafeExternalLongExample other) { checkNotBuilt(); safeExternalLongValue(other.getSafeExternalLongValue()); optionalSafeExternalLong(other.getOptionalSafeExternalLong()); + safeExternalLongList(other.getSafeExternalLongList()); + safeExternalLongSet(other.getSafeExternalLongSet()); return this; } @@ -139,7 +173,7 @@ public Builder safeExternalLongValue(@Safe long safeExternalLongValue) { } @JsonSetter(value = "optionalSafeExternalLong", nulls = Nulls.SKIP) - public Builder optionalSafeExternalLong(@Nonnull Optional optionalSafeExternalLong) { + public Builder optionalSafeExternalLong(@Safe @Nonnull Optional optionalSafeExternalLong) { checkNotBuilt(); this.optionalSafeExternalLong = Preconditions.checkNotNull( optionalSafeExternalLong, "optionalSafeExternalLong cannot be null") @@ -147,13 +181,57 @@ public Builder optionalSafeExternalLong(@Nonnull Optional option return this; } - public Builder optionalSafeExternalLong(long optionalSafeExternalLong) { + public Builder optionalSafeExternalLong(@Safe long optionalSafeExternalLong) { checkNotBuilt(); this.optionalSafeExternalLong = Optional.of( Preconditions.checkNotNull(optionalSafeExternalLong, "optionalSafeExternalLong cannot be null")); return this; } + @JsonSetter(value = "safeExternalLongList", nulls = Nulls.SKIP, contentNulls = Nulls.FAIL) + public Builder safeExternalLongList(@Safe @Nonnull Iterable safeExternalLongList) { + checkNotBuilt(); + this.safeExternalLongList = ConjureCollections.newArrayList( + Preconditions.checkNotNull(safeExternalLongList, "safeExternalLongList cannot be null")); + return this; + } + + public Builder addAllSafeExternalLongList(@Safe @Nonnull Iterable safeExternalLongList) { + checkNotBuilt(); + ConjureCollections.addAll( + this.safeExternalLongList, + Preconditions.checkNotNull(safeExternalLongList, "safeExternalLongList cannot be null")); + return this; + } + + public Builder safeExternalLongList(long safeExternalLongList) { + checkNotBuilt(); + this.safeExternalLongList.add(safeExternalLongList); + return this; + } + + @JsonSetter(value = "safeExternalLongSet", nulls = Nulls.SKIP, contentNulls = Nulls.FAIL) + public Builder safeExternalLongSet(@Safe @Nonnull Iterable safeExternalLongSet) { + checkNotBuilt(); + this.safeExternalLongSet = ConjureCollections.newLinkedHashSet( + Preconditions.checkNotNull(safeExternalLongSet, "safeExternalLongSet cannot be null")); + return this; + } + + public Builder addAllSafeExternalLongSet(@Safe @Nonnull Iterable safeExternalLongSet) { + checkNotBuilt(); + ConjureCollections.addAll( + this.safeExternalLongSet, + Preconditions.checkNotNull(safeExternalLongSet, "safeExternalLongSet cannot be null")); + return this; + } + + public Builder safeExternalLongSet(long safeExternalLongSet) { + checkNotBuilt(); + this.safeExternalLongSet.add(safeExternalLongSet); + return this; + } + private void validatePrimitiveFieldsHaveBeenInitialized() { List missingFields = null; missingFields = @@ -179,7 +257,8 @@ public SafeExternalLongExample build() { checkNotBuilt(); this._buildInvoked = true; validatePrimitiveFieldsHaveBeenInitialized(); - return new SafeExternalLongExample(safeExternalLongValue, optionalSafeExternalLong); + return new SafeExternalLongExample( + safeExternalLongValue, optionalSafeExternalLong, safeExternalLongList, safeExternalLongSet); } private void checkNotBuilt() { diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java index fadd945e1..c12453c4f 100644 --- a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java @@ -6,13 +6,17 @@ import com.fasterxml.jackson.annotation.JsonSetter; import com.fasterxml.jackson.annotation.Nulls; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.palantir.conjure.java.lib.internal.ConjureCollections; import com.palantir.logsafe.Preconditions; import com.palantir.logsafe.Safe; import com.palantir.logsafe.SafeArg; import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.function.Function; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -26,12 +30,22 @@ public final class SafeExternalLongExample { private final Optional optionalSafeExternalLong; + private final List safeExternalLongList; + + private final Set safeExternalLongSet; + private int memoizedHashCode; - private SafeExternalLongExample(long safeExternalLongValue, Optional optionalSafeExternalLong) { - validateFields(optionalSafeExternalLong); + private SafeExternalLongExample( + long safeExternalLongValue, + Optional optionalSafeExternalLong, + List safeExternalLongList, + Set safeExternalLongSet) { + validateFields(optionalSafeExternalLong, safeExternalLongList, safeExternalLongSet); this.safeExternalLongValue = safeExternalLongValue; this.optionalSafeExternalLong = optionalSafeExternalLong; + this.safeExternalLongList = Collections.unmodifiableList(safeExternalLongList); + this.safeExternalLongSet = Collections.unmodifiableSet(safeExternalLongSet); } @JsonProperty("safeExternalLongValue") @@ -41,11 +55,24 @@ public long getSafeExternalLongValue() { } @JsonProperty("optionalSafeExternalLong") + @Safe @JsonInclude(JsonInclude.Include.NON_ABSENT) public Optional getOptionalSafeExternalLong() { return this.optionalSafeExternalLong; } + @JsonProperty("safeExternalLongList") + @Safe + public List getSafeExternalLongList() { + return this.safeExternalLongList; + } + + @JsonProperty("safeExternalLongSet") + @Safe + public Set getSafeExternalLongSet() { + return this.safeExternalLongSet; + } + @Override public boolean equals(@Nullable Object other) { return this == other || (other instanceof SafeExternalLongExample && equalTo((SafeExternalLongExample) other)); @@ -58,7 +85,9 @@ private boolean equalTo(SafeExternalLongExample other) { return false; } return this.safeExternalLongValue == other.safeExternalLongValue - && this.optionalSafeExternalLong.equals(other.optionalSafeExternalLong); + && this.optionalSafeExternalLong.equals(other.optionalSafeExternalLong) + && this.safeExternalLongList.equals(other.safeExternalLongList) + && this.safeExternalLongSet.equals(other.safeExternalLongSet); } @Override @@ -68,6 +97,8 @@ public int hashCode() { int hash = 1; hash = 31 * hash + Long.hashCode(this.safeExternalLongValue); hash = 31 * hash + this.optionalSafeExternalLong.hashCode(); + hash = 31 * hash + this.safeExternalLongList.hashCode(); + hash = 31 * hash + this.safeExternalLongSet.hashCode(); result = hash; memoizedHashCode = result; } @@ -78,19 +109,16 @@ public int hashCode() { @Safe public String toString() { return "SafeExternalLongExample{safeExternalLongValue: " + safeExternalLongValue - + ", optionalSafeExternalLong: " + optionalSafeExternalLong + '}'; + + ", optionalSafeExternalLong: " + optionalSafeExternalLong + ", safeExternalLongList: " + + safeExternalLongList + ", safeExternalLongSet: " + safeExternalLongSet + '}'; } - public static SafeExternalLongExample of(@Safe long safeExternalLongValue, long optionalSafeExternalLong) { - return builder() - .safeExternalLongValue(safeExternalLongValue) - .optionalSafeExternalLong(Optional.of(optionalSafeExternalLong)) - .build(); - } - - private static void validateFields(Optional optionalSafeExternalLong) { + private static void validateFields( + Optional optionalSafeExternalLong, List safeExternalLongList, Set safeExternalLongSet) { List missingFields = null; missingFields = addFieldIfMissing(missingFields, optionalSafeExternalLong, "optionalSafeExternalLong"); + missingFields = addFieldIfMissing(missingFields, safeExternalLongList, "safeExternalLongList"); + missingFields = addFieldIfMissing(missingFields, safeExternalLongSet, "safeExternalLongSet"); if (missingFields != null) { throw new SafeIllegalArgumentException( "Some required fields have not been set", SafeArg.of("missingFields", missingFields)); @@ -101,7 +129,7 @@ private static List addFieldIfMissing(List prev, Object fieldVal List missingFields = prev; if (fieldValue == null) { if (missingFields == null) { - missingFields = new ArrayList<>(1); + missingFields = new ArrayList<>(3); } missingFields.add(fieldName); } @@ -119,7 +147,11 @@ public static final class Builder { private long safeExternalLongValue; - private Optional optionalSafeExternalLong = Optional.empty(); + private Optional<@Safe Long> optionalSafeExternalLong = Optional.empty(); + + private List<@Safe Long> safeExternalLongList = new ArrayList<>(); + + private Set<@Safe Long> safeExternalLongSet = new LinkedHashSet<>(); private boolean _safeExternalLongValueInitialized = false; @@ -129,6 +161,8 @@ public Builder from(SafeExternalLongExample other) { checkNotBuilt(); safeExternalLongValue(other.getSafeExternalLongValue()); optionalSafeExternalLong(other.getOptionalSafeExternalLong()); + safeExternalLongList(other.getSafeExternalLongList()); + safeExternalLongSet(other.getSafeExternalLongSet()); return this; } @@ -141,7 +175,7 @@ public Builder safeExternalLongValue(@Safe long safeExternalLongValue) { } @JsonSetter(value = "optionalSafeExternalLong", nulls = Nulls.SKIP) - public Builder optionalSafeExternalLong(@Nonnull Optional optionalSafeExternalLong) { + public Builder optionalSafeExternalLong(@Safe @Nonnull Optional optionalSafeExternalLong) { checkNotBuilt(); this.optionalSafeExternalLong = Preconditions.checkNotNull( optionalSafeExternalLong, "optionalSafeExternalLong cannot be null") @@ -149,13 +183,57 @@ public Builder optionalSafeExternalLong(@Nonnull Optional option return this; } - public Builder optionalSafeExternalLong(long optionalSafeExternalLong) { + public Builder optionalSafeExternalLong(@Safe long optionalSafeExternalLong) { checkNotBuilt(); this.optionalSafeExternalLong = Optional.of( Preconditions.checkNotNull(optionalSafeExternalLong, "optionalSafeExternalLong cannot be null")); return this; } + @JsonSetter(value = "safeExternalLongList", nulls = Nulls.SKIP) + public Builder safeExternalLongList(@Safe @Nonnull Iterable safeExternalLongList) { + checkNotBuilt(); + this.safeExternalLongList = ConjureCollections.newArrayList( + Preconditions.checkNotNull(safeExternalLongList, "safeExternalLongList cannot be null")); + return this; + } + + public Builder addAllSafeExternalLongList(@Safe @Nonnull Iterable safeExternalLongList) { + checkNotBuilt(); + ConjureCollections.addAll( + this.safeExternalLongList, + Preconditions.checkNotNull(safeExternalLongList, "safeExternalLongList cannot be null")); + return this; + } + + public Builder safeExternalLongList(long safeExternalLongList) { + checkNotBuilt(); + this.safeExternalLongList.add(safeExternalLongList); + return this; + } + + @JsonSetter(value = "safeExternalLongSet", nulls = Nulls.SKIP) + public Builder safeExternalLongSet(@Safe @Nonnull Iterable safeExternalLongSet) { + checkNotBuilt(); + this.safeExternalLongSet = ConjureCollections.newLinkedHashSet( + Preconditions.checkNotNull(safeExternalLongSet, "safeExternalLongSet cannot be null")); + return this; + } + + public Builder addAllSafeExternalLongSet(@Safe @Nonnull Iterable safeExternalLongSet) { + checkNotBuilt(); + ConjureCollections.addAll( + this.safeExternalLongSet, + Preconditions.checkNotNull(safeExternalLongSet, "safeExternalLongSet cannot be null")); + return this; + } + + public Builder safeExternalLongSet(long safeExternalLongSet) { + checkNotBuilt(); + this.safeExternalLongSet.add(safeExternalLongSet); + return this; + } + private void validatePrimitiveFieldsHaveBeenInitialized() { List missingFields = null; missingFields = @@ -181,7 +259,8 @@ public SafeExternalLongExample build() { checkNotBuilt(); this._buildInvoked = true; validatePrimitiveFieldsHaveBeenInitialized(); - return new SafeExternalLongExample(safeExternalLongValue, optionalSafeExternalLong); + return new SafeExternalLongExample( + safeExternalLongValue, optionalSafeExternalLong, safeExternalLongList, safeExternalLongSet); } private void checkNotBuilt() { diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java index 81fa90683..10b7cbbbd 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java @@ -22,6 +22,7 @@ import com.palantir.conjure.java.util.SafetyUtils; import com.palantir.conjure.java.visitor.DefaultTypeVisitor; import com.palantir.conjure.spec.FieldDefinition; +import com.palantir.conjure.spec.LogSafety; import com.palantir.conjure.spec.MapType; import com.palantir.conjure.spec.OptionalType; import com.palantir.conjure.spec.PrimitiveType; @@ -97,12 +98,14 @@ public static MethodSpec.Builder publicSetter(EnrichedField enriched, ClassName .returns(returnClass); } - public static TypeName widenParameterIfPossible(TypeName current, Type type, TypeMapper typeMapper) { + public static TypeName widenParameterIfPossible( + TypeName current, Type type, TypeMapper typeMapper, Optional safety) { + // if inner type name is external if (type.accept(TypeVisitor.IS_LIST)) { Type innerType = type.accept(TypeVisitor.LIST).getItemType(); TypeName innerTypeName = typeMapper.getClassName(innerType).box(); if (isWidenableContainedType(innerType)) { - innerTypeName = WildcardTypeName.subtypeOf(innerTypeName); + innerTypeName = WildcardTypeName.subtypeOf(innerTypeName).annotated(ConjureAnnotations.safety(safety)); } return ParameterizedTypeName.get(ClassName.get(Iterable.class), innerTypeName); } @@ -111,7 +114,7 @@ public static TypeName widenParameterIfPossible(TypeName current, Type type, Typ Type innerType = type.accept(TypeVisitor.SET).getItemType(); TypeName innerTypeName = typeMapper.getClassName(innerType).box(); if (isWidenableContainedType(innerType)) { - innerTypeName = WildcardTypeName.subtypeOf(innerTypeName); + innerTypeName = WildcardTypeName.subtypeOf(innerTypeName).annotated(ConjureAnnotations.safety(safety)); } return ParameterizedTypeName.get(ClassName.get(Iterable.class), innerTypeName); @@ -123,12 +126,18 @@ public static TypeName widenParameterIfPossible(TypeName current, Type type, Typ return current; } TypeName innerTypeName = typeMapper.getClassName(innerType).box(); - return ParameterizedTypeName.get(ClassName.get(Optional.class), WildcardTypeName.subtypeOf(innerTypeName)); + WildcardTypeName wildcard = + WildcardTypeName.subtypeOf(innerTypeName).annotated(ConjureAnnotations.safety(safety)); + return ParameterizedTypeName.get(ClassName.get(Optional.class), wildcard); } return current; } + public static TypeName widenParameterIfPossible(TypeName current, Type type, TypeMapper typeMapper) { + return widenParameterIfPossible(current, type, typeMapper, Optional.empty()); + } + // we want to widen containers of anything that's not a primitive, a conjure reference or an optional // since we know all of those are final. public static boolean isWidenableContainedType(Type containedType) { 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 d59f8e4c4..e187f3893 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 @@ -328,7 +328,11 @@ private MethodSpec createSetter( boolean shouldClearFirst = true; MethodSpec.Builder setterBuilder = BeanBuilderAuxiliarySettersUtils.publicSetter(enriched, builderClass) .addParameter(Parameters.nonnullParameter( - BeanBuilderAuxiliarySettersUtils.widenParameterIfPossible(field.type, type, typeMapper), + BeanBuilderAuxiliarySettersUtils.widenParameterIfPossible( + field.type, + type, + typeMapper, + SafetyUtils.getMaybeExternalSafety(enriched.conjureDef())), field.name, SafetyUtils.getMaybeExternalSafety(enriched.conjureDef()))) .addCode(verifyNotBuilt()) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java index 1eb12cbb6..e50d9c8d6 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java @@ -16,11 +16,19 @@ package com.palantir.conjure.java.util; -import com.palantir.conjure.java.visitor.MoreVisitors; import com.palantir.conjure.spec.AliasDefinition; import com.palantir.conjure.spec.ArgumentDefinition; +import com.palantir.conjure.spec.ExternalReference; import com.palantir.conjure.spec.FieldDefinition; +import com.palantir.conjure.spec.ListType; import com.palantir.conjure.spec.LogSafety; +import com.palantir.conjure.spec.MapType; +import com.palantir.conjure.spec.OptionalType; +import com.palantir.conjure.spec.PrimitiveType; +import com.palantir.conjure.spec.SetType; +import com.palantir.conjure.spec.Type; +import com.palantir.conjure.spec.TypeName; +import com.palantir.logsafe.Safe; import java.util.Optional; public final class SafetyUtils { @@ -28,23 +36,111 @@ public final class SafetyUtils { private SafetyUtils() {} public static Optional getMaybeExternalSafety(AliasDefinition alias) { - if (alias.getAlias().accept(MoreVisitors.IS_EXTERNAL)) { - return alias.getAlias().accept(MoreVisitors.EXTERNAL).getSafety(); + if (alias.getAlias().accept(WrapsExternalType.INSTANCE)) { + return alias.getAlias().accept(GetMaybeExternalSafety.INSTANCE); } return alias.getSafety(); } public static Optional getMaybeExternalSafety(FieldDefinition field) { - if (field.getType().accept(MoreVisitors.IS_EXTERNAL)) { - return field.getType().accept(MoreVisitors.EXTERNAL).getSafety(); + if (field.getType().accept(WrapsExternalType.INSTANCE)) { + return field.getType().accept(GetMaybeExternalSafety.INSTANCE); } return field.getSafety(); } public static Optional getMaybeExternalSafety(ArgumentDefinition argument) { - if (argument.getType().accept(MoreVisitors.IS_EXTERNAL)) { - return argument.getType().accept(MoreVisitors.EXTERNAL).getSafety(); + if (argument.getType().accept(WrapsExternalType.INSTANCE)) { + return argument.getType().accept(GetMaybeExternalSafety.INSTANCE); } return argument.getSafety(); } + + private enum WrapsExternalType implements Type.Visitor { + INSTANCE; + + @Override + public java.lang.Boolean visitPrimitive(PrimitiveType value) { + return false; + } + + @Override + public Boolean visitOptional(OptionalType value) { + return value.getItemType().accept(INSTANCE); + } + + @Override + public Boolean visitList(ListType value) { + return value.getItemType().accept(INSTANCE); + } + + @Override + public Boolean visitSet(SetType value) { + return value.getItemType().accept(INSTANCE); + } + + @Override + public Boolean visitMap(MapType _value) { + return false; + } + + @Override + public Boolean visitReference(TypeName _value) { + return false; + } + + @Override + public Boolean visitExternal(ExternalReference _value) { + return true; + } + + @Override + public Boolean visitUnknown(String _unknownType) { + return false; + } + } + + private enum GetMaybeExternalSafety implements Type.Visitor> { + INSTANCE; + + @Override + public Optional visitPrimitive(PrimitiveType _value) { + return Optional.empty(); + } + + @Override + public Optional visitOptional(OptionalType value) { + return value.getItemType().accept(INSTANCE); + } + + @Override + public Optional visitList(ListType value) { + return value.getItemType().accept(INSTANCE); + } + + @Override + public Optional visitSet(SetType value) { + return value.getItemType().accept(INSTANCE); + } + + @Override + public Optional visitMap(MapType _value) { + return Optional.empty(); + } + + @Override + public Optional visitReference(TypeName _value) { + return Optional.empty(); + } + + @Override + public Optional visitExternal(ExternalReference value) { + return value.getSafety(); + } + + @Override + public Optional visitUnknown(@Safe String unknownType) { + return Optional.empty(); + } + } } diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java index 05250f495..0cec087ac 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java @@ -94,6 +94,12 @@ public void testUnionAnnotations() { unknownStageVisitorBuilder, "unknown", "unknownVisitor", "String", Safe.class); } + @Test + public void testOptionalAnnotations() { + assertFieldTypeParamHasAnnotation( + SafeExternalLongExample.class, "optionalSafeExternalLong", "Long", Safe.class); + } + private void assertMethodHasAnnotation( Class parentClass, String methodName, Class annotation) { Stream desiredMethods = getMatchingMethods(parentClass, methodName); 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 9e11c78f8..7a5a08466 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 @@ -118,6 +118,21 @@ public void testObjectGenerator_stagedBuilder() throws IOException { assertThatFilesAreTheSame(files, REFERENCE_FILES_FOLDER); } + @Test + public void testLavanya() throws IOException { + ConjureDefinition def = Conjure.parse(ImmutableList.of(new File("src/test/resources/example.yml"))); + List files = new GenerationCoordinator( + MoreExecutors.directExecutor(), + ImmutableSet.of(new ObjectGenerator(Options.builder() + .useStagedBuilders(true) + .excludeEmptyOptionals(true) + .jetbrainsContractAnnotations(true) + .build()))) + .emit(def, tempDir); + + System.out.println(compiledFileContent(tempDir, "com/palantir/product/NestedTypesExample.java")); + } + @Test public void testObjectGenerator_excludeEmptyCollections() throws IOException { ConjureDefinition def = diff --git a/conjure-java-core/src/test/resources/example-types.yml b/conjure-java-core/src/test/resources/example-types.yml index c80b9263f..6a2b9a037 100644 --- a/conjure-java-core/src/test/resources/example-types.yml +++ b/conjure-java-core/src/test/resources/example-types.yml @@ -34,6 +34,19 @@ types: fields: safeExternalLongValue: SafeExternalLong optionalSafeExternalLong: optional + safeExternalLongList: list + safeExternalLongSet: set + NestedTypesExample: + fields: + optionalString: + type: optional + safety: safe + stringList: + type: list + safety: safe + stringSet: + type: set + safety: safe RidExample: fields: ridValue: rid From ba1f619870e638c43a16fd8a388d0ef0620be43e Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Fri, 10 Feb 2023 15:48:42 -0500 Subject: [PATCH 12/16] poet :( --- .../palantir/product/NestedTypesExample.java | 220 +++++++++++++++++ .../palantir/product/NestedTypesExample.java | 222 ++++++++++++++++++ .../BeanBuilderAuxiliarySettersUtils.java | 3 +- .../java/types/ObjectGeneratorTests.java | 25 ++ .../src/test/resources/example.yml | 17 ++ 5 files changed, 486 insertions(+), 1 deletion(-) create mode 100644 conjure-java-core/src/integrationInput/java/com/palantir/product/NestedTypesExample.java create mode 100644 conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/NestedTypesExample.java create mode 100644 conjure-java-core/src/test/resources/example.yml diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/NestedTypesExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/NestedTypesExample.java new file mode 100644 index 000000000..d30e80824 --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/NestedTypesExample.java @@ -0,0 +1,220 @@ +package com.palantir.product; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.palantir.conjure.java.lib.internal.ConjureCollections; +import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import javax.annotation.processing.Generated; + +@Safe +@JsonDeserialize(builder = NestedTypesExample.Builder.class) +@Generated("com.palantir.conjure.java.types.BeanGenerator") +public final class NestedTypesExample { + private final Optional optionalString; + + private final List stringList; + + private final Set stringSet; + + private int memoizedHashCode; + + private NestedTypesExample(Optional optionalString, List stringList, Set stringSet) { + validateFields(optionalString, stringList, stringSet); + this.optionalString = optionalString; + this.stringList = Collections.unmodifiableList(stringList); + this.stringSet = Collections.unmodifiableSet(stringSet); + } + + @JsonProperty("optionalString") + @Safe + @JsonInclude(JsonInclude.Include.NON_ABSENT) + public Optional getOptionalString() { + return this.optionalString; + } + + @JsonProperty("stringList") + @Safe + public List getStringList() { + return this.stringList; + } + + @JsonProperty("stringSet") + @Safe + public Set getStringSet() { + return this.stringSet; + } + + @Override + public boolean equals(@Nullable Object other) { + return this == other || (other instanceof NestedTypesExample && equalTo((NestedTypesExample) other)); + } + + private boolean equalTo(NestedTypesExample other) { + if (this.memoizedHashCode != 0 + && other.memoizedHashCode != 0 + && this.memoizedHashCode != other.memoizedHashCode) { + return false; + } + return this.optionalString.equals(other.optionalString) + && this.stringList.equals(other.stringList) + && this.stringSet.equals(other.stringSet); + } + + @Override + public int hashCode() { + int result = memoizedHashCode; + if (result == 0) { + int hash = 1; + hash = 31 * hash + this.optionalString.hashCode(); + hash = 31 * hash + this.stringList.hashCode(); + hash = 31 * hash + this.stringSet.hashCode(); + result = hash; + memoizedHashCode = result; + } + return result; + } + + @Override + @Safe + public String toString() { + return "NestedTypesExample{optionalString: " + optionalString + ", stringList: " + stringList + ", stringSet: " + + stringSet + '}'; + } + + public static NestedTypesExample of( + @Safe String optionalString, @Safe List stringList, @Safe Set stringSet) { + return builder() + .optionalString(Optional.of(optionalString)) + .stringList(stringList) + .stringSet(stringSet) + .build(); + } + + private static void validateFields( + Optional optionalString, List stringList, Set stringSet) { + List missingFields = null; + missingFields = addFieldIfMissing(missingFields, optionalString, "optionalString"); + missingFields = addFieldIfMissing(missingFields, stringList, "stringList"); + missingFields = addFieldIfMissing(missingFields, stringSet, "stringSet"); + if (missingFields != null) { + throw new SafeIllegalArgumentException( + "Some required fields have not been set", SafeArg.of("missingFields", missingFields)); + } + } + + private static List addFieldIfMissing(List prev, Object fieldValue, String fieldName) { + List missingFields = prev; + if (fieldValue == null) { + if (missingFields == null) { + missingFields = new ArrayList<>(3); + } + missingFields.add(fieldName); + } + return missingFields; + } + + public static Builder builder() { + return new Builder(); + } + + @Generated("com.palantir.conjure.java.types.BeanBuilderGenerator") + public static final class Builder { + boolean _buildInvoked; + + private Optional<@Safe String> optionalString = Optional.empty(); + + private List<@Safe String> stringList = new ArrayList<>(); + + private Set<@Safe String> stringSet = new LinkedHashSet<>(); + + private Builder() {} + + public Builder from(NestedTypesExample other) { + checkNotBuilt(); + optionalString(other.getOptionalString()); + stringList(other.getStringList()); + stringSet(other.getStringSet()); + return this; + } + + @JsonSetter(value = "optionalString", nulls = Nulls.SKIP) + public Builder optionalString(@Safe @Nonnull Optional<@Safe String> optionalString) { + checkNotBuilt(); + this.optionalString = Preconditions.checkNotNull(optionalString, "optionalString cannot be null"); + return this; + } + + public Builder optionalString(@Safe @Nonnull String optionalString) { + checkNotBuilt(); + this.optionalString = + Optional.of(Preconditions.checkNotNull(optionalString, "optionalString cannot be null")); + return this; + } + + @JsonSetter(value = "stringList", nulls = Nulls.SKIP, contentNulls = Nulls.FAIL) + public Builder stringList(@Safe @Nonnull Iterable stringList) { + checkNotBuilt(); + this.stringList = ConjureCollections.newArrayList( + Preconditions.checkNotNull(stringList, "stringList cannot be null")); + return this; + } + + public Builder addAllStringList(@Safe @Nonnull Iterable stringList) { + checkNotBuilt(); + ConjureCollections.addAll( + this.stringList, Preconditions.checkNotNull(stringList, "stringList cannot be null")); + return this; + } + + public Builder stringList(String stringList) { + checkNotBuilt(); + this.stringList.add(stringList); + return this; + } + + @JsonSetter(value = "stringSet", nulls = Nulls.SKIP, contentNulls = Nulls.FAIL) + public Builder stringSet(@Safe @Nonnull Iterable stringSet) { + checkNotBuilt(); + this.stringSet = ConjureCollections.newLinkedHashSet( + Preconditions.checkNotNull(stringSet, "stringSet cannot be null")); + return this; + } + + public Builder addAllStringSet(@Safe @Nonnull Iterable stringSet) { + checkNotBuilt(); + ConjureCollections.addAll( + this.stringSet, Preconditions.checkNotNull(stringSet, "stringSet cannot be null")); + return this; + } + + public Builder stringSet(String stringSet) { + checkNotBuilt(); + this.stringSet.add(stringSet); + return this; + } + + public NestedTypesExample build() { + checkNotBuilt(); + this._buildInvoked = true; + return new NestedTypesExample(optionalString, stringList, stringSet); + } + + private void checkNotBuilt() { + Preconditions.checkState(!_buildInvoked, "Build has already been called"); + } + } +} diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/NestedTypesExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/NestedTypesExample.java new file mode 100644 index 000000000..26a2dec68 --- /dev/null +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/NestedTypesExample.java @@ -0,0 +1,222 @@ +package test.prefix.com.palantir.product; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.palantir.conjure.java.lib.internal.ConjureCollections; +import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import javax.annotation.processing.Generated; + +@Safe +@JsonDeserialize(builder = NestedTypesExample.Builder.class) +@Generated("com.palantir.conjure.java.types.BeanGenerator") +public final class NestedTypesExample { + private final Optional optionalString; + + private final List stringList; + + private final Set stringSet; + + private int memoizedHashCode; + + private NestedTypesExample(Optional optionalString, List stringList, Set stringSet) { + validateFields(optionalString, stringList, stringSet); + this.optionalString = optionalString; + this.stringList = Collections.unmodifiableList(stringList); + this.stringSet = Collections.unmodifiableSet(stringSet); + } + + @JsonProperty("optionalString") + @Safe + @JsonInclude(JsonInclude.Include.NON_ABSENT) + public Optional getOptionalString() { + return this.optionalString; + } + + @JsonProperty("stringList") + @Safe + public List getStringList() { + return this.stringList; + } + + @JsonProperty("stringSet") + @Safe + public Set getStringSet() { + return this.stringSet; + } + + @Override + public boolean equals(@Nullable Object other) { + return this == other || (other instanceof NestedTypesExample && equalTo((NestedTypesExample) other)); + } + + private boolean equalTo(NestedTypesExample other) { + if (this.memoizedHashCode != 0 + && other.memoizedHashCode != 0 + && this.memoizedHashCode != other.memoizedHashCode) { + return false; + } + return this.optionalString.equals(other.optionalString) + && this.stringList.equals(other.stringList) + && this.stringSet.equals(other.stringSet); + } + + @Override + public int hashCode() { + int result = memoizedHashCode; + if (result == 0) { + int hash = 1; + hash = 31 * hash + this.optionalString.hashCode(); + hash = 31 * hash + this.stringList.hashCode(); + hash = 31 * hash + this.stringSet.hashCode(); + result = hash; + memoizedHashCode = result; + } + return result; + } + + @Override + @Safe + public String toString() { + return "NestedTypesExample{optionalString: " + optionalString + ", stringList: " + stringList + ", stringSet: " + + stringSet + '}'; + } + + public static NestedTypesExample of( + @Safe String optionalString, @Safe List stringList, @Safe Set stringSet) { + return builder() + .optionalString(Optional.of(optionalString)) + .stringList(stringList) + .stringSet(stringSet) + .build(); + } + + private static void validateFields( + Optional optionalString, List stringList, Set stringSet) { + List missingFields = null; + missingFields = addFieldIfMissing(missingFields, optionalString, "optionalString"); + missingFields = addFieldIfMissing(missingFields, stringList, "stringList"); + missingFields = addFieldIfMissing(missingFields, stringSet, "stringSet"); + if (missingFields != null) { + throw new SafeIllegalArgumentException( + "Some required fields have not been set", SafeArg.of("missingFields", missingFields)); + } + } + + private static List addFieldIfMissing(List prev, Object fieldValue, String fieldName) { + List missingFields = prev; + if (fieldValue == null) { + if (missingFields == null) { + missingFields = new ArrayList<>(3); + } + missingFields.add(fieldName); + } + return missingFields; + } + + public static Builder builder() { + return new Builder(); + } + + @Generated("com.palantir.conjure.java.types.BeanBuilderGenerator") + @JsonIgnoreProperties(ignoreUnknown = true) + public static final class Builder { + boolean _buildInvoked; + + private Optional<@Safe String> optionalString = Optional.empty(); + + private List<@Safe String> stringList = new ArrayList<>(); + + private Set<@Safe String> stringSet = new LinkedHashSet<>(); + + private Builder() {} + + public Builder from(NestedTypesExample other) { + checkNotBuilt(); + optionalString(other.getOptionalString()); + stringList(other.getStringList()); + stringSet(other.getStringSet()); + return this; + } + + @JsonSetter(value = "optionalString", nulls = Nulls.SKIP) + public Builder optionalString(@Safe @Nonnull Optional<@Safe String> optionalString) { + checkNotBuilt(); + this.optionalString = Preconditions.checkNotNull(optionalString, "optionalString cannot be null"); + return this; + } + + public Builder optionalString(@Safe @Nonnull String optionalString) { + checkNotBuilt(); + this.optionalString = + Optional.of(Preconditions.checkNotNull(optionalString, "optionalString cannot be null")); + return this; + } + + @JsonSetter(value = "stringList", nulls = Nulls.SKIP) + public Builder stringList(@Safe @Nonnull Iterable stringList) { + checkNotBuilt(); + this.stringList = ConjureCollections.newArrayList( + Preconditions.checkNotNull(stringList, "stringList cannot be null")); + return this; + } + + public Builder addAllStringList(@Safe @Nonnull Iterable stringList) { + checkNotBuilt(); + ConjureCollections.addAll( + this.stringList, Preconditions.checkNotNull(stringList, "stringList cannot be null")); + return this; + } + + public Builder stringList(String stringList) { + checkNotBuilt(); + this.stringList.add(stringList); + return this; + } + + @JsonSetter(value = "stringSet", nulls = Nulls.SKIP) + public Builder stringSet(@Safe @Nonnull Iterable stringSet) { + checkNotBuilt(); + this.stringSet = ConjureCollections.newLinkedHashSet( + Preconditions.checkNotNull(stringSet, "stringSet cannot be null")); + return this; + } + + public Builder addAllStringSet(@Safe @Nonnull Iterable stringSet) { + checkNotBuilt(); + ConjureCollections.addAll( + this.stringSet, Preconditions.checkNotNull(stringSet, "stringSet cannot be null")); + return this; + } + + public Builder stringSet(String stringSet) { + checkNotBuilt(); + this.stringSet.add(stringSet); + return this; + } + + public NestedTypesExample build() { + checkNotBuilt(); + this._buildInvoked = true; + return new NestedTypesExample(optionalString, stringList, stringSet); + } + + private void checkNotBuilt() { + Preconditions.checkState(!_buildInvoked, "Build has already been called"); + } + } +} diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java index 10b7cbbbd..9e40f1644 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java @@ -128,7 +128,8 @@ public static TypeName widenParameterIfPossible( TypeName innerTypeName = typeMapper.getClassName(innerType).box(); WildcardTypeName wildcard = WildcardTypeName.subtypeOf(innerTypeName).annotated(ConjureAnnotations.safety(safety)); - return ParameterizedTypeName.get(ClassName.get(Optional.class), wildcard); + ParameterizedTypeName p = ParameterizedTypeName.get(ClassName.get(Optional.class), wildcard); + return p; } return current; 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 7a5a08466..e5a1bdeec 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 @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.MoreExecutors; import com.palantir.conjure.defs.Conjure; +import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.GenerationCoordinator; import com.palantir.conjure.java.Options; import com.palantir.conjure.spec.AliasDefinition; @@ -37,6 +38,11 @@ import com.palantir.conjure.spec.Type; import com.palantir.conjure.spec.TypeDefinition; import com.palantir.conjure.spec.TypeName; +import com.squareup.javapoet.ClassName; +import com.squareup.javapoet.JavaFile; +import com.squareup.javapoet.ParameterizedTypeName; +import com.squareup.javapoet.TypeSpec; +import com.squareup.javapoet.WildcardTypeName; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -44,6 +50,8 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; +import java.util.Optional; +import javax.lang.model.element.Modifier; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -191,6 +199,23 @@ public void testConjureErrors() throws IOException { assertThatFilesAreTheSame(files, REFERENCE_FILES_FOLDER); } + @Test + public void testPoet() throws IOException { + WildcardTypeName wildcard = WildcardTypeName.subtypeOf(com.squareup.javapoet.TypeName.LONG.box()) + .annotated(ConjureAnnotations.safety(Optional.of(LogSafety.SAFE))); + ParameterizedTypeName paramTypeName = ParameterizedTypeName.get(ClassName.get(Optional.class), wildcard); + TypeSpec types = TypeSpec.classBuilder("PoetTestClass") + .addField(paramTypeName, "testWildcardField", Modifier.PRIVATE) + .build(); + JavaFile file = JavaFile.builder("com.palantir.test", types) + .skipJavaLangImports(true) + .indent(" ") + .build(); + file.writeTo(tempDir.toPath(), StandardCharsets.UTF_8); + assertThat(compiledFileContent(tempDir, "com/palantir/test/PoetTestClass.java")) + .doesNotContain("@Safe ? extends"); + } + @Test public void testErrorSafetyDisagreement() { ErrorGenerator errorGenerator = new ErrorGenerator(Options.builder() diff --git a/conjure-java-core/src/test/resources/example.yml b/conjure-java-core/src/test/resources/example.yml new file mode 100644 index 000000000..706a0917e --- /dev/null +++ b/conjure-java-core/src/test/resources/example.yml @@ -0,0 +1,17 @@ +types: + imports: + SafeExternalLong: + base-type: string + external: + java: java.lang.Long + safety: safe + definitions: + default-package: com.palantir.product + objects: + NestedTypesExample: + fields: + optionalString: + type: optional + safety: safe + optionalLong: + type: optional \ No newline at end of file From 3c5b062a6d6b0c8c0fa1821a786a3f185db424ea Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Fri, 10 Feb 2023 16:27:28 -0500 Subject: [PATCH 13/16] collections and optionals work now --- .../palantir/product/NestedTypesExample.java | 220 ----------------- .../product/SafeExternalLongExample.java | 4 +- .../palantir/product/NestedTypesExample.java | 222 ------------------ .../product/SafeExternalLongExample.java | 4 +- .../BeanBuilderAuxiliarySettersUtils.java | 27 +-- .../java/types/BeanBuilderGenerator.java | 18 +- .../conjure/java/types/BeanGenerator.java | 6 +- .../java/types/ExternalImportSafetyTests.java | 29 ++- .../java/types/ObjectGeneratorTests.java | 40 ---- .../src/test/resources/example-types.yml | 11 - .../src/test/resources/example.yml | 17 -- 11 files changed, 56 insertions(+), 542 deletions(-) delete mode 100644 conjure-java-core/src/integrationInput/java/com/palantir/product/NestedTypesExample.java delete mode 100644 conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/NestedTypesExample.java delete mode 100644 conjure-java-core/src/test/resources/example.yml diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/NestedTypesExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/NestedTypesExample.java deleted file mode 100644 index d30e80824..000000000 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/NestedTypesExample.java +++ /dev/null @@ -1,220 +0,0 @@ -package com.palantir.product; - -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.annotation.JsonSetter; -import com.fasterxml.jackson.annotation.Nulls; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.palantir.conjure.java.lib.internal.ConjureCollections; -import com.palantir.logsafe.Preconditions; -import com.palantir.logsafe.Safe; -import com.palantir.logsafe.SafeArg; -import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import javax.annotation.processing.Generated; - -@Safe -@JsonDeserialize(builder = NestedTypesExample.Builder.class) -@Generated("com.palantir.conjure.java.types.BeanGenerator") -public final class NestedTypesExample { - private final Optional optionalString; - - private final List stringList; - - private final Set stringSet; - - private int memoizedHashCode; - - private NestedTypesExample(Optional optionalString, List stringList, Set stringSet) { - validateFields(optionalString, stringList, stringSet); - this.optionalString = optionalString; - this.stringList = Collections.unmodifiableList(stringList); - this.stringSet = Collections.unmodifiableSet(stringSet); - } - - @JsonProperty("optionalString") - @Safe - @JsonInclude(JsonInclude.Include.NON_ABSENT) - public Optional getOptionalString() { - return this.optionalString; - } - - @JsonProperty("stringList") - @Safe - public List getStringList() { - return this.stringList; - } - - @JsonProperty("stringSet") - @Safe - public Set getStringSet() { - return this.stringSet; - } - - @Override - public boolean equals(@Nullable Object other) { - return this == other || (other instanceof NestedTypesExample && equalTo((NestedTypesExample) other)); - } - - private boolean equalTo(NestedTypesExample other) { - if (this.memoizedHashCode != 0 - && other.memoizedHashCode != 0 - && this.memoizedHashCode != other.memoizedHashCode) { - return false; - } - return this.optionalString.equals(other.optionalString) - && this.stringList.equals(other.stringList) - && this.stringSet.equals(other.stringSet); - } - - @Override - public int hashCode() { - int result = memoizedHashCode; - if (result == 0) { - int hash = 1; - hash = 31 * hash + this.optionalString.hashCode(); - hash = 31 * hash + this.stringList.hashCode(); - hash = 31 * hash + this.stringSet.hashCode(); - result = hash; - memoizedHashCode = result; - } - return result; - } - - @Override - @Safe - public String toString() { - return "NestedTypesExample{optionalString: " + optionalString + ", stringList: " + stringList + ", stringSet: " - + stringSet + '}'; - } - - public static NestedTypesExample of( - @Safe String optionalString, @Safe List stringList, @Safe Set stringSet) { - return builder() - .optionalString(Optional.of(optionalString)) - .stringList(stringList) - .stringSet(stringSet) - .build(); - } - - private static void validateFields( - Optional optionalString, List stringList, Set stringSet) { - List missingFields = null; - missingFields = addFieldIfMissing(missingFields, optionalString, "optionalString"); - missingFields = addFieldIfMissing(missingFields, stringList, "stringList"); - missingFields = addFieldIfMissing(missingFields, stringSet, "stringSet"); - if (missingFields != null) { - throw new SafeIllegalArgumentException( - "Some required fields have not been set", SafeArg.of("missingFields", missingFields)); - } - } - - private static List addFieldIfMissing(List prev, Object fieldValue, String fieldName) { - List missingFields = prev; - if (fieldValue == null) { - if (missingFields == null) { - missingFields = new ArrayList<>(3); - } - missingFields.add(fieldName); - } - return missingFields; - } - - public static Builder builder() { - return new Builder(); - } - - @Generated("com.palantir.conjure.java.types.BeanBuilderGenerator") - public static final class Builder { - boolean _buildInvoked; - - private Optional<@Safe String> optionalString = Optional.empty(); - - private List<@Safe String> stringList = new ArrayList<>(); - - private Set<@Safe String> stringSet = new LinkedHashSet<>(); - - private Builder() {} - - public Builder from(NestedTypesExample other) { - checkNotBuilt(); - optionalString(other.getOptionalString()); - stringList(other.getStringList()); - stringSet(other.getStringSet()); - return this; - } - - @JsonSetter(value = "optionalString", nulls = Nulls.SKIP) - public Builder optionalString(@Safe @Nonnull Optional<@Safe String> optionalString) { - checkNotBuilt(); - this.optionalString = Preconditions.checkNotNull(optionalString, "optionalString cannot be null"); - return this; - } - - public Builder optionalString(@Safe @Nonnull String optionalString) { - checkNotBuilt(); - this.optionalString = - Optional.of(Preconditions.checkNotNull(optionalString, "optionalString cannot be null")); - return this; - } - - @JsonSetter(value = "stringList", nulls = Nulls.SKIP, contentNulls = Nulls.FAIL) - public Builder stringList(@Safe @Nonnull Iterable stringList) { - checkNotBuilt(); - this.stringList = ConjureCollections.newArrayList( - Preconditions.checkNotNull(stringList, "stringList cannot be null")); - return this; - } - - public Builder addAllStringList(@Safe @Nonnull Iterable stringList) { - checkNotBuilt(); - ConjureCollections.addAll( - this.stringList, Preconditions.checkNotNull(stringList, "stringList cannot be null")); - return this; - } - - public Builder stringList(String stringList) { - checkNotBuilt(); - this.stringList.add(stringList); - return this; - } - - @JsonSetter(value = "stringSet", nulls = Nulls.SKIP, contentNulls = Nulls.FAIL) - public Builder stringSet(@Safe @Nonnull Iterable stringSet) { - checkNotBuilt(); - this.stringSet = ConjureCollections.newLinkedHashSet( - Preconditions.checkNotNull(stringSet, "stringSet cannot be null")); - return this; - } - - public Builder addAllStringSet(@Safe @Nonnull Iterable stringSet) { - checkNotBuilt(); - ConjureCollections.addAll( - this.stringSet, Preconditions.checkNotNull(stringSet, "stringSet cannot be null")); - return this; - } - - public Builder stringSet(String stringSet) { - checkNotBuilt(); - this.stringSet.add(stringSet); - return this; - } - - public NestedTypesExample build() { - checkNotBuilt(); - this._buildInvoked = true; - return new NestedTypesExample(optionalString, stringList, stringSet); - } - - private void checkNotBuilt() { - Preconditions.checkState(!_buildInvoked, "Build has already been called"); - } - } -} diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java index 3288f5ec7..dc77dfbf4 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeExternalLongExample.java @@ -204,7 +204,7 @@ public Builder addAllSafeExternalLongList(@Safe @Nonnull Iterable return this; } - public Builder safeExternalLongSet(long safeExternalLongSet) { + public Builder safeExternalLongSet(@Safe long safeExternalLongSet) { checkNotBuilt(); this.safeExternalLongSet.add(safeExternalLongSet); return this; diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/NestedTypesExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/NestedTypesExample.java deleted file mode 100644 index 26a2dec68..000000000 --- a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/NestedTypesExample.java +++ /dev/null @@ -1,222 +0,0 @@ -package test.prefix.com.palantir.product; - -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.annotation.JsonSetter; -import com.fasterxml.jackson.annotation.Nulls; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; -import com.palantir.conjure.java.lib.internal.ConjureCollections; -import com.palantir.logsafe.Preconditions; -import com.palantir.logsafe.Safe; -import com.palantir.logsafe.SafeArg; -import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import javax.annotation.processing.Generated; - -@Safe -@JsonDeserialize(builder = NestedTypesExample.Builder.class) -@Generated("com.palantir.conjure.java.types.BeanGenerator") -public final class NestedTypesExample { - private final Optional optionalString; - - private final List stringList; - - private final Set stringSet; - - private int memoizedHashCode; - - private NestedTypesExample(Optional optionalString, List stringList, Set stringSet) { - validateFields(optionalString, stringList, stringSet); - this.optionalString = optionalString; - this.stringList = Collections.unmodifiableList(stringList); - this.stringSet = Collections.unmodifiableSet(stringSet); - } - - @JsonProperty("optionalString") - @Safe - @JsonInclude(JsonInclude.Include.NON_ABSENT) - public Optional getOptionalString() { - return this.optionalString; - } - - @JsonProperty("stringList") - @Safe - public List getStringList() { - return this.stringList; - } - - @JsonProperty("stringSet") - @Safe - public Set getStringSet() { - return this.stringSet; - } - - @Override - public boolean equals(@Nullable Object other) { - return this == other || (other instanceof NestedTypesExample && equalTo((NestedTypesExample) other)); - } - - private boolean equalTo(NestedTypesExample other) { - if (this.memoizedHashCode != 0 - && other.memoizedHashCode != 0 - && this.memoizedHashCode != other.memoizedHashCode) { - return false; - } - return this.optionalString.equals(other.optionalString) - && this.stringList.equals(other.stringList) - && this.stringSet.equals(other.stringSet); - } - - @Override - public int hashCode() { - int result = memoizedHashCode; - if (result == 0) { - int hash = 1; - hash = 31 * hash + this.optionalString.hashCode(); - hash = 31 * hash + this.stringList.hashCode(); - hash = 31 * hash + this.stringSet.hashCode(); - result = hash; - memoizedHashCode = result; - } - return result; - } - - @Override - @Safe - public String toString() { - return "NestedTypesExample{optionalString: " + optionalString + ", stringList: " + stringList + ", stringSet: " - + stringSet + '}'; - } - - public static NestedTypesExample of( - @Safe String optionalString, @Safe List stringList, @Safe Set stringSet) { - return builder() - .optionalString(Optional.of(optionalString)) - .stringList(stringList) - .stringSet(stringSet) - .build(); - } - - private static void validateFields( - Optional optionalString, List stringList, Set stringSet) { - List missingFields = null; - missingFields = addFieldIfMissing(missingFields, optionalString, "optionalString"); - missingFields = addFieldIfMissing(missingFields, stringList, "stringList"); - missingFields = addFieldIfMissing(missingFields, stringSet, "stringSet"); - if (missingFields != null) { - throw new SafeIllegalArgumentException( - "Some required fields have not been set", SafeArg.of("missingFields", missingFields)); - } - } - - private static List addFieldIfMissing(List prev, Object fieldValue, String fieldName) { - List missingFields = prev; - if (fieldValue == null) { - if (missingFields == null) { - missingFields = new ArrayList<>(3); - } - missingFields.add(fieldName); - } - return missingFields; - } - - public static Builder builder() { - return new Builder(); - } - - @Generated("com.palantir.conjure.java.types.BeanBuilderGenerator") - @JsonIgnoreProperties(ignoreUnknown = true) - public static final class Builder { - boolean _buildInvoked; - - private Optional<@Safe String> optionalString = Optional.empty(); - - private List<@Safe String> stringList = new ArrayList<>(); - - private Set<@Safe String> stringSet = new LinkedHashSet<>(); - - private Builder() {} - - public Builder from(NestedTypesExample other) { - checkNotBuilt(); - optionalString(other.getOptionalString()); - stringList(other.getStringList()); - stringSet(other.getStringSet()); - return this; - } - - @JsonSetter(value = "optionalString", nulls = Nulls.SKIP) - public Builder optionalString(@Safe @Nonnull Optional<@Safe String> optionalString) { - checkNotBuilt(); - this.optionalString = Preconditions.checkNotNull(optionalString, "optionalString cannot be null"); - return this; - } - - public Builder optionalString(@Safe @Nonnull String optionalString) { - checkNotBuilt(); - this.optionalString = - Optional.of(Preconditions.checkNotNull(optionalString, "optionalString cannot be null")); - return this; - } - - @JsonSetter(value = "stringList", nulls = Nulls.SKIP) - public Builder stringList(@Safe @Nonnull Iterable stringList) { - checkNotBuilt(); - this.stringList = ConjureCollections.newArrayList( - Preconditions.checkNotNull(stringList, "stringList cannot be null")); - return this; - } - - public Builder addAllStringList(@Safe @Nonnull Iterable stringList) { - checkNotBuilt(); - ConjureCollections.addAll( - this.stringList, Preconditions.checkNotNull(stringList, "stringList cannot be null")); - return this; - } - - public Builder stringList(String stringList) { - checkNotBuilt(); - this.stringList.add(stringList); - return this; - } - - @JsonSetter(value = "stringSet", nulls = Nulls.SKIP) - public Builder stringSet(@Safe @Nonnull Iterable stringSet) { - checkNotBuilt(); - this.stringSet = ConjureCollections.newLinkedHashSet( - Preconditions.checkNotNull(stringSet, "stringSet cannot be null")); - return this; - } - - public Builder addAllStringSet(@Safe @Nonnull Iterable stringSet) { - checkNotBuilt(); - ConjureCollections.addAll( - this.stringSet, Preconditions.checkNotNull(stringSet, "stringSet cannot be null")); - return this; - } - - public Builder stringSet(String stringSet) { - checkNotBuilt(); - this.stringSet.add(stringSet); - return this; - } - - public NestedTypesExample build() { - checkNotBuilt(); - this._buildInvoked = true; - return new NestedTypesExample(optionalString, stringList, stringSet); - } - - private void checkNotBuilt() { - Preconditions.checkState(!_buildInvoked, "Build has already been called"); - } - } -} diff --git a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java index c12453c4f..6b1bbdefc 100644 --- a/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java +++ b/conjure-java-core/src/integrationInput/java/test/prefix/com/palantir/product/SafeExternalLongExample.java @@ -206,7 +206,7 @@ public Builder addAllSafeExternalLongList(@Safe @Nonnull Iterable return this; } - public Builder safeExternalLongSet(long safeExternalLongSet) { + public Builder safeExternalLongSet(@Safe long safeExternalLongSet) { checkNotBuilt(); this.safeExternalLongSet.add(safeExternalLongSet); return this; diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java index 9e40f1644..d95bdcf40 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java @@ -74,9 +74,15 @@ public static MethodSpec.Builder createOptionalSetterBuilder( } public static MethodSpec.Builder createItemSetterBuilder( - EnrichedField enriched, Type itemType, TypeMapper typeMapper, ClassName returnClass) { + EnrichedField enriched, + Type itemType, + TypeMapper typeMapper, + ClassName returnClass, + Optional safety) { FieldSpec field = enriched.poetSpec(); - return publicSetter(enriched, returnClass).addParameter(typeMapper.getClassName(itemType), field.name); + return publicSetter(enriched, returnClass) + .addParameter( + typeMapper.getClassName(itemType).annotated(ConjureAnnotations.safety(safety)), field.name); } public static MethodSpec.Builder createMapSetterBuilder( @@ -98,14 +104,12 @@ public static MethodSpec.Builder publicSetter(EnrichedField enriched, ClassName .returns(returnClass); } - public static TypeName widenParameterIfPossible( - TypeName current, Type type, TypeMapper typeMapper, Optional safety) { - // if inner type name is external + public static TypeName widenParameterIfPossible(TypeName current, Type type, TypeMapper typeMapper) { if (type.accept(TypeVisitor.IS_LIST)) { Type innerType = type.accept(TypeVisitor.LIST).getItemType(); TypeName innerTypeName = typeMapper.getClassName(innerType).box(); if (isWidenableContainedType(innerType)) { - innerTypeName = WildcardTypeName.subtypeOf(innerTypeName).annotated(ConjureAnnotations.safety(safety)); + innerTypeName = WildcardTypeName.subtypeOf(innerTypeName); } return ParameterizedTypeName.get(ClassName.get(Iterable.class), innerTypeName); } @@ -114,7 +118,7 @@ public static TypeName widenParameterIfPossible( Type innerType = type.accept(TypeVisitor.SET).getItemType(); TypeName innerTypeName = typeMapper.getClassName(innerType).box(); if (isWidenableContainedType(innerType)) { - innerTypeName = WildcardTypeName.subtypeOf(innerTypeName).annotated(ConjureAnnotations.safety(safety)); + innerTypeName = WildcardTypeName.subtypeOf(innerTypeName); } return ParameterizedTypeName.get(ClassName.get(Iterable.class), innerTypeName); @@ -126,19 +130,12 @@ public static TypeName widenParameterIfPossible( return current; } TypeName innerTypeName = typeMapper.getClassName(innerType).box(); - WildcardTypeName wildcard = - WildcardTypeName.subtypeOf(innerTypeName).annotated(ConjureAnnotations.safety(safety)); - ParameterizedTypeName p = ParameterizedTypeName.get(ClassName.get(Optional.class), wildcard); - return p; + return ParameterizedTypeName.get(ClassName.get(Optional.class), WildcardTypeName.subtypeOf(innerTypeName)); } return current; } - public static TypeName widenParameterIfPossible(TypeName current, Type type, TypeMapper typeMapper) { - return widenParameterIfPossible(current, type, typeMapper, Optional.empty()); - } - // we want to widen containers of anything that's not a primitive, a conjure reference or an optional // since we know all of those are final. public static boolean isWidenableContainedType(Type containedType) { 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 e187f3893..94ababa62 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 @@ -36,6 +36,7 @@ import com.palantir.conjure.spec.FieldDefinition; import com.palantir.conjure.spec.FieldName; import com.palantir.conjure.spec.ListType; +import com.palantir.conjure.spec.LogSafety; import com.palantir.conjure.spec.MapType; import com.palantir.conjure.spec.ObjectDefinition; import com.palantir.conjure.spec.OptionalType; @@ -328,11 +329,7 @@ private MethodSpec createSetter( boolean shouldClearFirst = true; MethodSpec.Builder setterBuilder = BeanBuilderAuxiliarySettersUtils.publicSetter(enriched, builderClass) .addParameter(Parameters.nonnullParameter( - BeanBuilderAuxiliarySettersUtils.widenParameterIfPossible( - field.type, - type, - typeMapper, - SafetyUtils.getMaybeExternalSafety(enriched.conjureDef())), + BeanBuilderAuxiliarySettersUtils.widenParameterIfPossible(field.type, type, typeMapper), field.name, SafetyUtils.getMaybeExternalSafety(enriched.conjureDef()))) .addCode(verifyNotBuilt()) @@ -432,17 +429,18 @@ private boolean isByteBuffer(Type type) { private List createAuxiliarySetters(EnrichedField enriched, boolean override) { Type type = enriched.conjureDef().getType(); + Optional safety = SafetyUtils.getMaybeExternalSafety(enriched.conjureDef()); if (type.accept(TypeVisitor.IS_LIST)) { return ImmutableList.of( createCollectionSetter("addAll", enriched, override), - createItemSetter(enriched, type.accept(TypeVisitor.LIST).getItemType(), override)); + createItemSetter(enriched, type.accept(TypeVisitor.LIST).getItemType(), override, safety)); } if (type.accept(TypeVisitor.IS_SET)) { return ImmutableList.of( createCollectionSetter("addAll", enriched, override), - createItemSetter(enriched, type.accept(TypeVisitor.SET).getItemType(), override)); + createItemSetter(enriched, type.accept(TypeVisitor.SET).getItemType(), override, safety)); } if (type.accept(TypeVisitor.IS_MAP)) { @@ -494,9 +492,11 @@ private boolean isPrimitiveOptional(OptionalType optionalType) { optionalType.getItemType().accept(TypeVisitor.PRIMITIVE).get()); } - private MethodSpec createItemSetter(EnrichedField enriched, Type itemType, boolean override) { + private MethodSpec createItemSetter( + EnrichedField enriched, Type itemType, boolean override, Optional safety) { FieldSpec field = enriched.poetSpec(); - return BeanBuilderAuxiliarySettersUtils.createItemSetterBuilder(enriched, itemType, typeMapper, builderClass) + return BeanBuilderAuxiliarySettersUtils.createItemSetterBuilder( + enriched, itemType, typeMapper, builderClass, safety) .addAnnotations(ConjureAnnotations.override(override)) .addCode(verifyNotBuilt()) .addStatement("this.$1N.add($1N)", field.name) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java index 6d334f061..4d7570fe2 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java @@ -39,6 +39,7 @@ import com.palantir.conjure.spec.FieldDefinition; import com.palantir.conjure.spec.FieldName; import com.palantir.conjure.spec.ListType; +import com.palantir.conjure.spec.LogSafety; import com.palantir.conjure.spec.MapType; import com.palantir.conjure.spec.ObjectDefinition; import com.palantir.conjure.spec.OptionalType; @@ -326,6 +327,7 @@ private static List generateMethodsForFinalStageField( List methodSpecs = new ArrayList<>(); Type type = enriched.conjureDef().getType(); FieldDefinition definition = enriched.conjureDef(); + Optional safety = SafetyUtils.getMaybeExternalSafety(definition); methodSpecs.add(MethodSpec.methodBuilder(JavaNameSanitizer.sanitize(enriched.fieldName())) .addParameter(ParameterSpec.builder( @@ -348,7 +350,7 @@ private static List generateMethodsForFinalStageField( .addModifiers(Modifier.ABSTRACT) .build()); methodSpecs.add(BeanBuilderAuxiliarySettersUtils.createItemSetterBuilder( - enriched, type.accept(TypeVisitor.LIST).getItemType(), typeMapper, returnClass) + enriched, type.accept(TypeVisitor.LIST).getItemType(), typeMapper, returnClass, safety) .addModifiers(Modifier.ABSTRACT) .build()); } @@ -359,7 +361,7 @@ private static List generateMethodsForFinalStageField( .addModifiers(Modifier.ABSTRACT) .build()); methodSpecs.add(BeanBuilderAuxiliarySettersUtils.createItemSetterBuilder( - enriched, type.accept(TypeVisitor.SET).getItemType(), typeMapper, returnClass) + enriched, type.accept(TypeVisitor.SET).getItemType(), typeMapper, returnClass, safety) .addModifiers(Modifier.ABSTRACT) .build()); } diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java index 0cec087ac..2df0f09e6 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java @@ -96,8 +96,33 @@ public void testUnionAnnotations() { @Test public void testOptionalAnnotations() { - assertFieldTypeParamHasAnnotation( - SafeExternalLongExample.class, "optionalSafeExternalLong", "Long", Safe.class); + assertMethodHasAnnotation(SafeExternalLongExample.class, "getOptionalSafeExternalLong", Safe.class); + + Class builderSubclass = getMatchingSubclass(SafeExternalLongExample.class, "$Builder"); + assertFieldTypeParamHasAnnotation(builderSubclass, "optionalSafeExternalLong", "Long", Safe.class); + assertMethodParamHasAnnotation( + builderSubclass, "optionalSafeExternalLong", "optionalSafeExternalLong", Safe.class); + } + + @Test + public void testListAnnotations() { + assertMethodHasAnnotation(SafeExternalLongExample.class, "getSafeExternalLongList", Safe.class); + + Class builderSubclass = getMatchingSubclass(SafeExternalLongExample.class, "$Builder"); + assertFieldTypeParamHasAnnotation(builderSubclass, "safeExternalLongList", "Long", Safe.class); + assertMethodParamHasAnnotation(builderSubclass, "safeExternalLongList", "safeExternalLongList", Safe.class); + assertMethodParamHasAnnotation( + builderSubclass, "addAllSafeExternalLongList", "safeExternalLongList", Safe.class); + } + + @Test + public void testSetAnnotations() { + assertMethodHasAnnotation(SafeExternalLongExample.class, "getSafeExternalLongSet", Safe.class); + + Class builderSubclass = getMatchingSubclass(SafeExternalLongExample.class, "$Builder"); + assertFieldTypeParamHasAnnotation(builderSubclass, "safeExternalLongSet", "Long", Safe.class); + assertMethodParamHasAnnotation(builderSubclass, "safeExternalLongSet", "safeExternalLongSet", Safe.class); + assertMethodParamHasAnnotation(builderSubclass, "addAllSafeExternalLongSet", "safeExternalLongSet", Safe.class); } private void assertMethodHasAnnotation( 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 e5a1bdeec..9e11c78f8 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 @@ -23,7 +23,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.MoreExecutors; import com.palantir.conjure.defs.Conjure; -import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.GenerationCoordinator; import com.palantir.conjure.java.Options; import com.palantir.conjure.spec.AliasDefinition; @@ -38,11 +37,6 @@ import com.palantir.conjure.spec.Type; import com.palantir.conjure.spec.TypeDefinition; import com.palantir.conjure.spec.TypeName; -import com.squareup.javapoet.ClassName; -import com.squareup.javapoet.JavaFile; -import com.squareup.javapoet.ParameterizedTypeName; -import com.squareup.javapoet.TypeSpec; -import com.squareup.javapoet.WildcardTypeName; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -50,8 +44,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; -import java.util.Optional; -import javax.lang.model.element.Modifier; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -126,21 +118,6 @@ public void testObjectGenerator_stagedBuilder() throws IOException { assertThatFilesAreTheSame(files, REFERENCE_FILES_FOLDER); } - @Test - public void testLavanya() throws IOException { - ConjureDefinition def = Conjure.parse(ImmutableList.of(new File("src/test/resources/example.yml"))); - List files = new GenerationCoordinator( - MoreExecutors.directExecutor(), - ImmutableSet.of(new ObjectGenerator(Options.builder() - .useStagedBuilders(true) - .excludeEmptyOptionals(true) - .jetbrainsContractAnnotations(true) - .build()))) - .emit(def, tempDir); - - System.out.println(compiledFileContent(tempDir, "com/palantir/product/NestedTypesExample.java")); - } - @Test public void testObjectGenerator_excludeEmptyCollections() throws IOException { ConjureDefinition def = @@ -199,23 +176,6 @@ public void testConjureErrors() throws IOException { assertThatFilesAreTheSame(files, REFERENCE_FILES_FOLDER); } - @Test - public void testPoet() throws IOException { - WildcardTypeName wildcard = WildcardTypeName.subtypeOf(com.squareup.javapoet.TypeName.LONG.box()) - .annotated(ConjureAnnotations.safety(Optional.of(LogSafety.SAFE))); - ParameterizedTypeName paramTypeName = ParameterizedTypeName.get(ClassName.get(Optional.class), wildcard); - TypeSpec types = TypeSpec.classBuilder("PoetTestClass") - .addField(paramTypeName, "testWildcardField", Modifier.PRIVATE) - .build(); - JavaFile file = JavaFile.builder("com.palantir.test", types) - .skipJavaLangImports(true) - .indent(" ") - .build(); - file.writeTo(tempDir.toPath(), StandardCharsets.UTF_8); - assertThat(compiledFileContent(tempDir, "com/palantir/test/PoetTestClass.java")) - .doesNotContain("@Safe ? extends"); - } - @Test public void testErrorSafetyDisagreement() { ErrorGenerator errorGenerator = new ErrorGenerator(Options.builder() diff --git a/conjure-java-core/src/test/resources/example-types.yml b/conjure-java-core/src/test/resources/example-types.yml index 6a2b9a037..d1d5ef1b0 100644 --- a/conjure-java-core/src/test/resources/example-types.yml +++ b/conjure-java-core/src/test/resources/example-types.yml @@ -36,17 +36,6 @@ types: optionalSafeExternalLong: optional safeExternalLongList: list safeExternalLongSet: set - NestedTypesExample: - fields: - optionalString: - type: optional - safety: safe - stringList: - type: list - safety: safe - stringSet: - type: set - safety: safe RidExample: fields: ridValue: rid diff --git a/conjure-java-core/src/test/resources/example.yml b/conjure-java-core/src/test/resources/example.yml deleted file mode 100644 index 706a0917e..000000000 --- a/conjure-java-core/src/test/resources/example.yml +++ /dev/null @@ -1,17 +0,0 @@ -types: - imports: - SafeExternalLong: - base-type: string - external: - java: java.lang.Long - safety: safe - definitions: - default-package: com.palantir.product - objects: - NestedTypesExample: - fields: - optionalString: - type: optional - safety: safe - optionalLong: - type: optional \ No newline at end of file From 2b44bfe5a25527b63c53952a08505d20e4cc0558 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Sat, 11 Feb 2023 10:14:12 -0500 Subject: [PATCH 14/16] add tests for safetyutils --- .../conjure/java/util/SafetyUtilsTests.java | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 conjure-java-core/src/test/java/com/palantir/conjure/java/util/SafetyUtilsTests.java diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/util/SafetyUtilsTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/util/SafetyUtilsTests.java new file mode 100644 index 000000000..0313fe68e --- /dev/null +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/util/SafetyUtilsTests.java @@ -0,0 +1,77 @@ +/* + * (c) Copyright 2023 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.conjure.java.util; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.palantir.conjure.spec.ExternalReference; +import com.palantir.conjure.spec.FieldDefinition; +import com.palantir.conjure.spec.FieldName; +import com.palantir.conjure.spec.ListType; +import com.palantir.conjure.spec.LogSafety; +import com.palantir.conjure.spec.PrimitiveType; +import com.palantir.conjure.spec.Type; +import com.palantir.conjure.spec.TypeName; +import org.junit.jupiter.api.Test; + +public class SafetyUtilsTests { + + private static final Type SAFE_EXTERNAL = Type.external(ExternalReference.builder() + .externalReference(TypeName.of("Long", "java.lang")) + .fallback(Type.primitive(PrimitiveType.STRING)) + .safety(LogSafety.SAFE) + .build()); + + @Test + public void externalField() { + FieldDefinition field = FieldDefinition.builder() + .fieldName(FieldName.of("testField")) + .type(SAFE_EXTERNAL) + .build(); + assertThat(SafetyUtils.getMaybeExternalSafety(field)).isPresent().get().isEqualTo(LogSafety.SAFE); + } + + @Test + public void nonExternalField() { + FieldDefinition field = FieldDefinition.builder() + .fieldName(FieldName.of("testField")) + .type(Type.primitive(PrimitiveType.STRING)) + .build(); + assertThat(SafetyUtils.getMaybeExternalSafety(field)).isEmpty(); + } + + @Test + public void externalWrapper() { + FieldDefinition field = FieldDefinition.builder() + .fieldName(FieldName.of("testField")) + .type(Type.list(ListType.builder().itemType(SAFE_EXTERNAL).build())) + .build(); + assertThat(SafetyUtils.getMaybeExternalSafety(field)).isPresent().get().isEqualTo(LogSafety.SAFE); + } + + @Test + public void nonExternalWrapper() { + FieldDefinition field = FieldDefinition.builder() + .fieldName(FieldName.of("testField")) + .type(Type.list(ListType.builder() + .itemType(Type.primitive(PrimitiveType.STRING)) + .build())) + .safety(LogSafety.SAFE) + .build(); + assertThat(SafetyUtils.getMaybeExternalSafety(field)).isPresent().get().isEqualTo(LogSafety.SAFE); + } +} From da8b7822b14feade536d2dc9e601fb16337b5a50 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Mon, 13 Feb 2023 13:12:11 -0600 Subject: [PATCH 15/16] add changelog --- changelog/@unreleased/pr-1944.v2.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/@unreleased/pr-1944.v2.yml diff --git a/changelog/@unreleased/pr-1944.v2.yml b/changelog/@unreleased/pr-1944.v2.yml new file mode 100644 index 000000000..944f77093 --- /dev/null +++ b/changelog/@unreleased/pr-1944.v2.yml @@ -0,0 +1,4 @@ +type: feature +feature: Adds support for log safety annotations to external import types. For example, an alias of an external reference type will now have safety annotations on the class, getters, and setters. +links: + - https://github.com/palantir/conjure-java/pull/1944 From ad0b5ac84281db74ac62520481fcdbcf0f0de824 Mon Sep 17 00:00:00 2001 From: Lavanya Singh Date: Mon, 13 Feb 2023 21:23:48 -0600 Subject: [PATCH 16/16] cleaning safety utils --- .../conjure/java/util/SafetyUtils.java | 5 +-- .../java/types/ExternalImportSafetyTests.java | 39 +++++++++++++------ .../java/types/SafetyEvaluatorTest.java | 6 +-- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java index e50d9c8d6..0a094e5cb 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/util/SafetyUtils.java @@ -28,7 +28,6 @@ import com.palantir.conjure.spec.SetType; import com.palantir.conjure.spec.Type; import com.palantir.conjure.spec.TypeName; -import com.palantir.logsafe.Safe; import java.util.Optional; public final class SafetyUtils { @@ -60,7 +59,7 @@ private enum WrapsExternalType implements Type.Visitor { INSTANCE; @Override - public java.lang.Boolean visitPrimitive(PrimitiveType value) { + public java.lang.Boolean visitPrimitive(PrimitiveType _value) { return false; } @@ -139,7 +138,7 @@ public Optional visitExternal(ExternalReference value) { } @Override - public Optional visitUnknown(@Safe String unknownType) { + public Optional visitUnknown(String _unknownType) { return Optional.empty(); } } diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java index 2df0f09e6..3dc11e81c 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ExternalImportSafetyTests.java @@ -31,7 +31,9 @@ import java.lang.reflect.Method; import java.lang.reflect.Parameter; import java.util.Arrays; +import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -54,8 +56,6 @@ public void testObjectAnnotations() { assertMethodHasAnnotation(SafeExternalLongExample.class, "toString", Safe.class); assertMethodHasAnnotation(SafeExternalLongExample.class, "getSafeExternalLongValue", Safe.class); - assertMethodParamHasAnnotation(SafeExternalLongExample.class, "of", "safeExternalLongValue", Safe.class); - Class builder = getMatchingSubclass(SafeExternalLongExample.class, "Builder"); assertMethodParamHasAnnotation(builder, "safeExternalLongValue", "safeExternalLongValue", Safe.class); } @@ -71,17 +71,17 @@ public void testUnionAnnotations() { assertMethodParamHasAnnotation(ExternalLongUnionExample.class, "safeLong", "value", Safe.class); assertMethodParamHasAnnotation(ExternalLongUnionExample.class, "unknown", "type", Safe.class); - Class visitor = getMatchingSubclass(ExternalLongUnionExample.class, "$Visitor"); + Class visitor = getExactlyMatchingSubclass( + ExternalLongUnionExample.class, "com.palantir.product.ExternalLongUnionExample$Visitor"); assertMethodParamHasAnnotation(visitor, "visitSafeLong", "value", Safe.class); assertMethodParamHasAnnotation(visitor, "visitUnknown", "unknownType", Safe.class); - Class visitorBuilder = getMatchingSubclass(ExternalLongUnionExample.class, "$VisitorBuilder"); + Class visitorBuilder = getExactlyMatchingSubclass( + ExternalLongUnionExample.class, "com.palantir.product.ExternalLongUnionExample$VisitorBuilder"); assertFieldTypeParamHasAnnotation(visitorBuilder, "safeLongVisitor", "Long", Safe.class); assertFieldTypeParamHasAnnotation(visitorBuilder, "unknownVisitor", "String", Safe.class); - assertMethodParamWithTypeParameterHasAnnotation( - visitorBuilder, "safeLong", "safeLongVisitor", "Long", Safe.class); - assertMethodParamWithTypeParameterHasAnnotation( - visitorBuilder, "unknown", "unknownVisitor", "String", Safe.class); + assertMethodParamWithTypeParameterHasAnnotation(visitorBuilder, "safeLong", "safeLong", "Long", Safe.class); + assertMethodParamWithTypeParameterHasAnnotation(visitorBuilder, "unknown", "unknown", "String", Safe.class); Class stageVisitorBuilder = getMatchingSubclass(ExternalLongUnionExample.class, "SafeLongStageVisitorBuilder"); @@ -177,13 +177,28 @@ private void assertFieldTypeParamHasAnnotation( } private Stream getMatchingMethods(Class parentClass, String methodName) { - return Arrays.stream(parentClass.getMethods()) - .filter(method -> method.getName().equals(methodName)); + List methods = Arrays.stream(parentClass.getMethods()) + .filter(method -> method.getName().equals(methodName)) + .collect(Collectors.toList()); + assertThat(methods) + .withFailMessage(String.format("Expected method %s on class %s", methodName, parentClass.getName())) + .isNotEmpty(); + return methods.stream(); } private Class getMatchingSubclass(Class parentClass, String subclassName) { - Optional> subclassIfExists = Arrays.stream(SafeExternalLongExample.class.getClasses()) - .filter(subclass -> subclass.getName().contains("Builder")) + Optional> subclassIfExists = Arrays.stream(parentClass.getDeclaredClasses()) + .filter(subclass -> subclass.getName().contains(subclassName)) + .findAny(); + assertThat(subclassIfExists) + .withFailMessage(String.format("Expected %s:%s to exist", parentClass, subclassName)) + .isPresent(); + return subclassIfExists.get(); + } + + private Class getExactlyMatchingSubclass(Class parentClass, String subclassName) { + Optional> subclassIfExists = Arrays.stream(parentClass.getDeclaredClasses()) + .filter(subclass -> subclass.getName().equals(subclassName)) .findAny(); assertThat(subclassIfExists) .withFailMessage(String.format("Expected %s:%s to exist", parentClass, subclassName)) diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java index 095e38a91..922ec9459 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java @@ -148,7 +148,7 @@ private static Stream providesExternalRefTypes_ImportTime() { .fallback(Type.primitive(PrimitiveType.STRING)) .safety(LogSafety.DO_NOT_LOG) .build()); - return getTypes_SafetyAtImportTime(external); + return getTypes(external); } @ParameterizedTest @@ -164,7 +164,7 @@ private static Stream providesExternalRefTypes_NoSafety() { .externalReference(TypeName.of("Long", "java.lang")) .fallback(Type.primitive(PrimitiveType.STRING)) .build()); - return getTypes_SafetyAtImportTime(external); + return getTypes(external); } @ParameterizedTest @@ -366,7 +366,7 @@ void testEmptyEnum() { .hasValue(LogSafety.SAFE); } - private static Stream getTypes_SafetyAtImportTime(Type externalReference) { + private static Stream getTypes(Type externalReference) { TypeDefinition objectType = TypeDefinition.object(ObjectDefinition.builder() .typeName(FOO) .fields(FieldDefinition.builder()