From c7d62d7db47147b9097f5f0d8b71bc8c49886244 Mon Sep 17 00:00:00 2001 From: Dan Adajian Date: Fri, 10 May 2024 08:31:01 -0500 Subject: [PATCH 1/4] unit test --- .../expected.kt | 16 ++++++++++------ .../expected.kt | 5 +++-- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/test/unit/should_generate_classes_for_types_with_field_args/expected.kt b/test/unit/should_generate_classes_for_types_with_field_args/expected.kt index 745447d..52f9728 100644 --- a/test/unit/should_generate_classes_for_types_with_field_args/expected.kt +++ b/test/unit/should_generate_classes_for_types_with_field_args/expected.kt @@ -11,10 +11,12 @@ open class TypeWithOnlyFieldArgs { @GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT]) open class HybridType( val nullableField: String? = null, - val nonNullableField: String + val nonNullableField: String, + private val nullableResolver: String? = null, + private val nonNullableResolver: String ) { - open fun nullableResolver(arg: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String? = throw NotImplementedError("HybridType.nullableResolver must be implemented.") - open fun nonNullableResolver(arg: InputTypeForResolver, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("HybridType.nonNullableResolver must be implemented.") + open fun nullableResolver(arg: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String? = nullableResolver + open fun nonNullableResolver(arg: InputTypeForResolver, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = nonNullableResolver } @GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.INPUT_OBJECT]) @@ -36,8 +38,10 @@ open class TypeImplementingInterface( val booleanField1: Boolean? = null, val booleanField2: Boolean = false, val integerField1: Int? = null, - val integerField2: Int + val integerField2: Int, + private val nullableListResolver: List? = null, + private val nonNullableListResolver: List ) : HybridInterface { - override fun nullableListResolver(arg1: Int?, arg2: Int, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): List? = throw NotImplementedError("TypeImplementingInterface.nullableListResolver must be implemented.") - override fun nonNullableListResolver(arg1: Int, arg2: Int?, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): List = throw NotImplementedError("TypeImplementingInterface.nonNullableListResolver must be implemented.") + override fun nullableListResolver(arg1: Int?, arg2: Int, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): List? = nullableListResolver + override fun nonNullableListResolver(arg1: Int, arg2: Int?, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): List = nonNullableListResolver } diff --git a/test/unit/should_replace_federation_directives/expected.kt b/test/unit/should_replace_federation_directives/expected.kt index 0a6a4e1..b7c30b0 100644 --- a/test/unit/should_replace_federation_directives/expected.kt +++ b/test/unit/should_replace_federation_directives/expected.kt @@ -15,7 +15,8 @@ data class FederatedType( @GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT]) open class FederatedTypeResolver( @com.expediagroup.graphql.generator.federation.directives.ExternalDirective - val field2: String? = null + val field2: String? = null, + val field: String ) { - open fun field(arg: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("FederatedTypeResolver.field must be implemented.") + open fun field(arg: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = field } From 89f73dafb2a88a6b972e7dde7e452005fe1eff73 Mon Sep 17 00:00:00 2001 From: Dan Adajian Date: Mon, 20 May 2024 20:00:25 -0500 Subject: [PATCH 2/4] pass tests --- src/definitions/object.ts | 10 +++--- src/helpers/build-field-definition.ts | 35 ++++++++++++++++--- .../expected.kt | 2 +- .../expected.kt | 4 +-- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/definitions/object.ts b/src/definitions/object.ts index 138b8ed..627a0b9 100644 --- a/src/definitions/object.ts +++ b/src/definitions/object.ts @@ -69,13 +69,13 @@ export function buildObjectTypeDefinition( node.fields?.some((fieldNode) => fieldNode.arguments?.length), ); if (shouldGenerateFunctions) { - const fieldsWithNoArguments = node.fields?.filter( + const atLeastOneFieldHasNoArguments = node.fields?.some( (fieldNode) => !fieldNode.arguments?.length, ); const constructor = - !typeInResolverInterfacesConfig && fieldsWithNoArguments?.length - ? `(\n${fieldsWithNoArguments - .map((fieldNode) => { + !typeInResolverInterfacesConfig && atLeastOneFieldHasNoArguments + ? `(\n${node.fields + ?.map((fieldNode) => { const typeMetadata = buildTypeMetadata( fieldNode.type, schema, @@ -87,6 +87,8 @@ export function buildObjectTypeDefinition( schema, config, typeMetadata, + false, + true, ); }) .join(",\n")}\n)` diff --git a/src/helpers/build-field-definition.ts b/src/helpers/build-field-definition.ts index e7bb9b3..f14a697 100644 --- a/src/helpers/build-field-definition.ts +++ b/src/helpers/build-field-definition.ts @@ -32,9 +32,18 @@ export function buildFieldDefinition( config: CodegenConfigWithDefaults, typeMetadata: TypeMetadata, shouldGenerateFunctions?: boolean, + isConstructor?: boolean, ) { - const modifier = buildFieldModifier(node, fieldNode, schema, config); - const fieldArguments = buildFieldArguments(node, fieldNode, schema, config); + const modifier = buildFieldModifier( + node, + fieldNode, + schema, + config, + isConstructor, + ); + const fieldArguments = isConstructor + ? "" + : buildFieldArguments(node, fieldNode, schema, config); const fieldDefinition = `${modifier} ${fieldNode.name.value}${fieldArguments}`; const annotations = buildAnnotations({ config, @@ -49,8 +58,15 @@ export function buildFieldDefinition( ); } - const notImplementedError = ` = throw NotImplementedError("${node.name.value}.${fieldNode.name.value} must be implemented.")`; - const defaultFunctionValue = `${typeMetadata.isNullable ? "?" : ""}${notImplementedError}`; + const notImplementedError = `throw NotImplementedError("${node.name.value}.${fieldNode.name.value} must be implemented.")`; + const atLeastOneFieldHasNoArguments = node.fields?.some( + (fieldNode) => !fieldNode.arguments?.length, + ); + const defaultImplementation = + atLeastOneFieldHasNoArguments && !config.resolverInterfaces + ? fieldNode.name.value + : notImplementedError; + const defaultFunctionValue = `${typeMetadata.isNullable ? "?" : ""} = ${defaultImplementation}`; const defaultValue = shouldGenerateFunctions ? defaultFunctionValue : typeMetadata.defaultValue; @@ -61,7 +77,7 @@ export function buildFieldDefinition( ); const isCompletableFuture = typeInResolverInterfacesConfig?.classMethods === "COMPLETABLE_FUTURE"; - const completableFutureDefinition = `java.util.concurrent.CompletableFuture<${typeMetadata.typeName}${typeMetadata.isNullable ? "?" : ""}>${notImplementedError}`; + const completableFutureDefinition = `java.util.concurrent.CompletableFuture<${typeMetadata.typeName}${typeMetadata.isNullable ? "?" : ""}> = ${defaultImplementation}`; const field = indent( `${fieldDefinition}: ${isCompletableFuture ? completableFutureDefinition : defaultDefinition}`, 2, @@ -74,6 +90,7 @@ function buildFieldModifier( fieldNode: FieldDefinitionNode, schema: GraphQLSchema, config: CodegenConfigWithDefaults, + isConstructor?: boolean, ) { const typeInResolverInterfacesConfig = findTypeInResolverInterfacesConfig( node, @@ -84,6 +101,14 @@ function buildFieldModifier( fieldNode, schema, ); + + if ( + !typeInResolverInterfacesConfig && + isConstructor && + fieldNode.arguments?.length + ) { + return "private val"; + } if (!typeInResolverInterfacesConfig && !fieldNode.arguments?.length) { return shouldOverrideField ? "override val" : "val"; } diff --git a/test/unit/should_generate_classes_for_types_with_field_args/expected.kt b/test/unit/should_generate_classes_for_types_with_field_args/expected.kt index 52f9728..8c2d70b 100644 --- a/test/unit/should_generate_classes_for_types_with_field_args/expected.kt +++ b/test/unit/should_generate_classes_for_types_with_field_args/expected.kt @@ -40,7 +40,7 @@ open class TypeImplementingInterface( val integerField1: Int? = null, val integerField2: Int, private val nullableListResolver: List? = null, - private val nonNullableListResolver: List + private val nonNullableListResolver: List = emptyList() ) : HybridInterface { override fun nullableListResolver(arg1: Int?, arg2: Int, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): List? = nullableListResolver override fun nonNullableListResolver(arg1: Int, arg2: Int?, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): List = nonNullableListResolver diff --git a/test/unit/should_replace_federation_directives/expected.kt b/test/unit/should_replace_federation_directives/expected.kt index b7c30b0..29b782a 100644 --- a/test/unit/should_replace_federation_directives/expected.kt +++ b/test/unit/should_replace_federation_directives/expected.kt @@ -14,9 +14,9 @@ data class FederatedType( @com.expediagroup.graphql.generator.federation.directives.KeyDirective(com.expediagroup.graphql.generator.federation.directives.FieldSet("some other field")) @GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT]) open class FederatedTypeResolver( + private val field: String, @com.expediagroup.graphql.generator.federation.directives.ExternalDirective - val field2: String? = null, - val field: String + val field2: String? = null ) { open fun field(arg: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = field } From 0919994226b857e4fe11247cbeb3b74ecf255d71 Mon Sep 17 00:00:00 2001 From: Dan Adajian Date: Tue, 21 May 2024 07:52:36 -0500 Subject: [PATCH 3/4] refactor --- src/definitions/object.ts | 2 +- src/helpers/build-field-definition.ts | 21 +++++++++------------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/definitions/object.ts b/src/definitions/object.ts index 627a0b9..763e3ef 100644 --- a/src/definitions/object.ts +++ b/src/definitions/object.ts @@ -87,7 +87,7 @@ export function buildObjectTypeDefinition( schema, config, typeMetadata, - false, + shouldGenerateFunctions, true, ); }) diff --git a/src/helpers/build-field-definition.ts b/src/helpers/build-field-definition.ts index f14a697..c4dcce8 100644 --- a/src/helpers/build-field-definition.ts +++ b/src/helpers/build-field-definition.ts @@ -32,16 +32,16 @@ export function buildFieldDefinition( config: CodegenConfigWithDefaults, typeMetadata: TypeMetadata, shouldGenerateFunctions?: boolean, - isConstructor?: boolean, + isConstructorField?: boolean, ) { const modifier = buildFieldModifier( node, fieldNode, schema, config, - isConstructor, + isConstructorField, ); - const fieldArguments = isConstructor + const fieldArguments = isConstructorField ? "" : buildFieldArguments(node, fieldNode, schema, config); const fieldDefinition = `${modifier} ${fieldNode.name.value}${fieldArguments}`; @@ -67,9 +67,10 @@ export function buildFieldDefinition( ? fieldNode.name.value : notImplementedError; const defaultFunctionValue = `${typeMetadata.isNullable ? "?" : ""} = ${defaultImplementation}`; - const defaultValue = shouldGenerateFunctions - ? defaultFunctionValue - : typeMetadata.defaultValue; + const defaultValue = + shouldGenerateFunctions && !isConstructorField + ? defaultFunctionValue + : typeMetadata.defaultValue; const defaultDefinition = `${typeMetadata.typeName}${defaultValue}`; const typeInResolverInterfacesConfig = findTypeInResolverInterfacesConfig( node, @@ -90,7 +91,7 @@ function buildFieldModifier( fieldNode: FieldDefinitionNode, schema: GraphQLSchema, config: CodegenConfigWithDefaults, - isConstructor?: boolean, + isConstructorField?: boolean, ) { const typeInResolverInterfacesConfig = findTypeInResolverInterfacesConfig( node, @@ -102,11 +103,7 @@ function buildFieldModifier( schema, ); - if ( - !typeInResolverInterfacesConfig && - isConstructor && - fieldNode.arguments?.length - ) { + if (isConstructorField && fieldNode.arguments?.length) { return "private val"; } if (!typeInResolverInterfacesConfig && !fieldNode.arguments?.length) { From 5449d1ecf40ec3f4df87855fcbf9755a3922d47f Mon Sep 17 00:00:00 2001 From: Dan Adajian Date: Tue, 21 May 2024 08:15:21 -0500 Subject: [PATCH 4/4] fix bug --- src/helpers/build-field-definition.ts | 11 ++++++----- test/integration/Query.kt | 7 ++++++- test/integration/expected.graphql | 8 +++++++- .../expected.kt | 10 +++++++++- .../schema.graphql | 7 ++++++- 5 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/helpers/build-field-definition.ts b/src/helpers/build-field-definition.ts index c4dcce8..610eda8 100644 --- a/src/helpers/build-field-definition.ts +++ b/src/helpers/build-field-definition.ts @@ -62,8 +62,12 @@ export function buildFieldDefinition( const atLeastOneFieldHasNoArguments = node.fields?.some( (fieldNode) => !fieldNode.arguments?.length, ); + const typeInResolverInterfacesConfig = findTypeInResolverInterfacesConfig( + node, + config, + ); const defaultImplementation = - atLeastOneFieldHasNoArguments && !config.resolverInterfaces + !typeInResolverInterfacesConfig && atLeastOneFieldHasNoArguments ? fieldNode.name.value : notImplementedError; const defaultFunctionValue = `${typeMetadata.isNullable ? "?" : ""} = ${defaultImplementation}`; @@ -72,10 +76,7 @@ export function buildFieldDefinition( ? defaultFunctionValue : typeMetadata.defaultValue; const defaultDefinition = `${typeMetadata.typeName}${defaultValue}`; - const typeInResolverInterfacesConfig = findTypeInResolverInterfacesConfig( - node, - config, - ); + const isCompletableFuture = typeInResolverInterfacesConfig?.classMethods === "COMPLETABLE_FUTURE"; const completableFutureDefinition = `java.util.concurrent.CompletableFuture<${typeMetadata.typeName}${typeMetadata.isNullable ? "?" : ""}> = ${defaultImplementation}`; diff --git a/test/integration/Query.kt b/test/integration/Query.kt index 9786c01..4ba6613 100644 --- a/test/integration/Query.kt +++ b/test/integration/Query.kt @@ -5,5 +5,10 @@ import graphql.schema.DataFetchingEnvironment import test.integration.Query as QueryInterface class IntegrationTestQuery() : Query, QueryInterface() { - override fun testQuery(dataFetchingEnvironment: DataFetchingEnvironment): SomeType = SomeType() + override fun testQuery1(dataFetchingEnvironment: DataFetchingEnvironment): SomeType = SomeType() + override fun testQuery2(dataFetchingEnvironment: DataFetchingEnvironment): SomeHybridType = + SomeHybridType( + someField = "test", + someField2 = "test" + ) } diff --git a/test/integration/expected.graphql b/test/integration/expected.graphql index d395b79..9b0072d 100644 --- a/test/integration/expected.graphql +++ b/test/integration/expected.graphql @@ -30,7 +30,13 @@ directive @specifiedBy( ) on SCALAR type Query { - testQuery: SomeType! + testQuery1: SomeType! + testQuery2: SomeHybridType! +} + +type SomeHybridType { + someField: String! + someField2(input: String!): String } type SomeType { diff --git a/test/unit/should_honor_resolverInterfaces_config/expected.kt b/test/unit/should_honor_resolverInterfaces_config/expected.kt index 4f0e457..9598e0b 100644 --- a/test/unit/should_honor_resolverInterfaces_config/expected.kt +++ b/test/unit/should_honor_resolverInterfaces_config/expected.kt @@ -4,7 +4,7 @@ import com.expediagroup.graphql.generator.annotations.* @GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT]) open class MyIncludedResolverType { - open fun nullableField(dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String? = throw NotImplementedError("MyIncludedResolverType.nullableField must be implemented.") + open fun nullableField(dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): MyChildType? = throw NotImplementedError("MyIncludedResolverType.nullableField must be implemented.") open fun nonNullableField(dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("MyIncludedResolverType.nonNullableField must be implemented.") open fun nullableResolver(arg: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String? = throw NotImplementedError("MyIncludedResolverType.nullableResolver must be implemented.") open fun nonNullableResolver(arg: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String = throw NotImplementedError("MyIncludedResolverType.nonNullableResolver must be implemented.") @@ -12,6 +12,14 @@ open class MyIncludedResolverType { open fun nonNullableListResolver(arg1: Int, arg2: Int? = null, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): List = throw NotImplementedError("MyIncludedResolverType.nonNullableListResolver must be implemented.") } +@GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT]) +open class MyChildType( + val field: String? = null, + private val field2: String? = null +) { + open fun field2(arg: String, dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String? = field2 +} + @GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT]) open class MyIncludedResolverTypeWithNoFieldArgs { open fun nullableField(dataFetchingEnvironment: graphql.schema.DataFetchingEnvironment): String? = throw NotImplementedError("MyIncludedResolverTypeWithNoFieldArgs.nullableField must be implemented.") diff --git a/test/unit/should_honor_resolverInterfaces_config/schema.graphql b/test/unit/should_honor_resolverInterfaces_config/schema.graphql index 03c3279..fcce4fd 100644 --- a/test/unit/should_honor_resolverInterfaces_config/schema.graphql +++ b/test/unit/should_honor_resolverInterfaces_config/schema.graphql @@ -1,5 +1,5 @@ type MyIncludedResolverType { - nullableField: String + nullableField: MyChildType nonNullableField: String! nullableResolver(arg: String!): String nonNullableResolver(arg: String!): String! @@ -7,6 +7,11 @@ type MyIncludedResolverType { nonNullableListResolver(arg1: Int!, arg2: Int): [String!]! } +type MyChildType { + field: String + field2(arg: String!): String +} + type MyIncludedResolverTypeWithNoFieldArgs { nullableField: String nonNullableField: String!