Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue where the generated field of my output type is not the same as my POKO entity? #462

Open
wojcik-marcin opened this issue Jun 6, 2023 Discussed in #461 · 5 comments

Comments

@wojcik-marcin
Copy link

Discussed in #461

Originally posted by wojcik-marcin June 5, 2023
I would like to request some guidance with an issue I am experiencing.

I am making use of the SPQR Spring Boot Starter v0.0.7 within my Kotlin Spring Boot application.

I have a Person data class defined as follows:

data class Person(
   val name: String,
   val isMarried: Boolean
)

I have a @GraphQLMutation endpoint to update the Person

@GraphQLMutation
fun updatePerson(@GraphQLArgument(name = "person") person: Person): Person {
   person.isMarried = true
   return Person
}

The GraphQL schema types generated are as follows:

type Person {
   name: String!
   married: Boolean!
}

type PersonInput {
   name: String!
   isMarried: Boolean!
}

As you can see, the input type generates the property name for isMarried correctly, however, I expected the same for the Person type that is being returned as the output type. Instead, it is returning as married.

I am using Jackson as my JSON serializer, and adding @JsonProperty("isMarried") to the field in the data class doesn't seem to work.

How can I enforce that the output type's isMarried property generates correctly to be the same as that of what is defined in the Person data class?

Any assistance in this matter will be greatly appreciated.

@kaqqao
Copy link
Member

kaqqao commented Jun 8, 2023

I'm not sure what names Kotlin ends up generating for the getter and setter, but it seems like it does not generate JavaBeans compliant names (which in this case would be isIsMarried or getIsMarried, and setIsMarried). Side note: this is why it's normally preferred in Java to simply name boolean fields x and not isX, as the related getter will be named isX().
I guess Kotlin generated isMarried() and setIsMarried, which would explain what SPQR ends up seeing.

Anyway, the simplest would be to append @GraphQLQuery(name = "isMarried") on the field, or the getter (maybe via @get:GraphQLQuery?). I'm unfortunately not familiar with Kotlin, so can't say exactly. Please report back what happened once you figure it out, as I'd like to learn what Kotlin does here.

The bottom line is that SPQR by default expects the JavaBeans spec to be respected (field, getField or isField, and setField), or explicitly overridden via annotations (@GraphQLQuery for output types, and @GraphQLInputField for input types, with a fallback to @GraphQLQuery if @GraphQLInputField isn't specified). So you can use these annotations to explicitly set the names.

Advanced usage

You almost certainly don't need to do this, unless you're doing some deep customization, like supporting a new set of annotations, or a JVM language with strange naming conventions. But, you can always provide a custom OperationInfoGenerator that implements any naming logic you desire, or maybe even use one of the existing ones, and set it to BeanResolverBuilder.

E.g. to replace the naming logic the default BeanResolverBuilder uses with the one that simply leave the method name untouched if no annotation is present:

generator.withNestedResolverBuilders((config, defaultBuilders) -> defaultBuilders
       // Customize the built-in BeanResolverBuilder
      .replace(BeanResolverBuilder.class, def -> def.withOperationInfoGenerator(new MemberOperationInfoGenerator())))

Or to provide a fully custom naming logic:

private class CustomOperationInfoGenerator extends AnnotatedOperationInfoGenerator {

        @Override
        public String name(OperationInfoGeneratorParams params) {
            // `elements` contains the field, the getter and the setter, is they exist
            List<AnnotatedElement> elements = params.getElement().getElements();
            // Find the field, if it exists
            Optional<String> field = Utils.extractInstances(elements, Field.class).findFirst().map(Field::getName);
            // Return the explicit name from the annotation, if it exists. If not, the name of field.
            // If that is also missing, return null and let the default logic take over
            return Utils.coalesce(super.name(params), field.orElse(null));
        }
}

// Delegates to your custom impl, and falls back to the defaults if yours returns `null`
OperationInfoGenerator customNaming = new DefaultOperationInfoGenerator()
        .withDelegate(new CustomOperationInfoGenerator());

generator.withNestedResolverBuilders((config, defaultBuilders) -> defaultBuilders
      // Customize the built-in BeanResolverBuilder
      .replace(BeanResolverBuilder.class, def -> def.withOperationInfoGenerator(customNaming)))

@wojcik-marcin
Copy link
Author

Thanks for your response.

I added the @GraphQLQuery annotation to the isMarried property as follows:

@GraphQLType(name = "Person")
data class Person(
    val name: String,
    @GraphQLQuery(name = "isMarried")
    var isMarried: Boolean? = null
)

That did not seem to work. I am still seeing the generated GraphQL type returning the property as married.

I will try with a CustomOperationInfoGenerator and give feedback.

@wojcik-marcin
Copy link
Author

wojcik-marcin commented Jun 9, 2023

When I changed the name of the isMarried field in to married, the @GraphQLQuery(name = "isMarried") annotation successfully renamed the field in the GraphQLType to isMarried.

@GraphQLType(name = "Person")
data class Person(
    val name: String,
    @GraphQLQuery(name = "isMarried")
    var married: Boolean? = null
)

However, if I leave the name of the field in my data class as isMarried, then it seems as though the annotation is being ignored

Could it possibly because SPQR does not detect it as a ElementType.FIELD when the name of the field starts with get, set or is?

@IceBlizz6
Copy link

I think i may have figure out something.
The codebase is using jackson to resolve and name fields.
Try to add jackson-module-kotlin dependency to your project.

implementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.9.8")
Let me know if it works for you.
I just tried it and it looks very promising.

@IceBlizz6
Copy link

Ignore that last post (although you may still want jackson-module-kotlin to solve other issues)
I believe the solution here is to just write a SchemaTransformer.
I made an attempt at this earlier as i had the same problem.

This has not been properly tested, but it may be a starting point for you:

class PreserveBooleanFieldName : SchemaTransformer {
	override fun transformField(
		field: GraphQLFieldDefinition,
		operation: Operation,
		operationMapper: OperationMapper,
		buildContext: BuildContext
	): GraphQLFieldDefinition {
		return if (isBoolean(field.type)) {
			GraphQLFieldDefinition
				.newFieldDefinition(field)
				.name(getOriginalName(operation.typedElement))
				.build()
		} else {
			field
		}
	}

	override fun transformInputField(
		field: GraphQLInputObjectField,
		inputField: InputField,
		operationMapper: OperationMapper,
		buildContext: BuildContext
	): GraphQLInputObjectField {
		return if (isBoolean(field.type)) {
			GraphQLInputObjectField
				.newInputObjectField(field)
				.name(getOriginalName(inputField.typedElement))
				.build()
		} else {
			field
		}
	}

	private fun isBoolean(type: GraphQLType): Boolean {
		val typeName = type.toString()
		return typeName == "Boolean" || typeName == "Boolean!"
	}

	private fun getOriginalName(typedElement: TypedElement): String {
		val elements = typedElement.elements
		val method = elements.filterIsInstance<Method>().singleOrNull()
		val field = elements.filterIsInstance<Field>().singleOrNull()
		if (method != null) {
			return method.name
		} else if (field != null) {
			return field.name
		} else {
			error("No elements found")
		}
	}
}

@kaqqao Do you have a better suggestion for the isBoolean implementation?
that string equals check doesn't look great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants