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

WIP Custom Scalar Proof of Concept #1988

Merged
merged 6 commits into from
Dec 22, 2023
Merged

Conversation

shirskor5
Copy link

What: This PR allows users to define custom scalars based on GraphQL's String, Int, and Float literal primitives. This design might also be extended to support custom scalars based on JSON literals in the future.

Why: Current functionality supports mapping object fields to the scalars that are supported by smallrye-grapqhl, however a user cannot define their own scalars in the resulting GraphQL schema.

How: This PR is suggesting a new annotation of @CustomScalar("ScalarName") and a limited set of interfaces representing each GraphQL literal primitive type.

The following is an example of an alternative BigDecimal scalar that serializes to a GraphQL String instead of a Float.

@CustomScalar("BigDecimalString")
public class BigDecimalString implements CustomStringScalar {
    public BigDecimalString(String stringValue) {
        ...
    }
    @Override
    public String stringValue() {
        ...
    }
}

Disclaimer: This is just a suggestion for how this might be done and we are very open to design feedback. Would also like to note that the exception handling was purposefully left undone as we did not want to infer how the exception handling should be done. As we move towards a final design we would appreciate any input on exception handling best practices for this project as well.

@jmartisk
Copy link
Member

jmartisk commented Dec 18, 2023

Thanks, I'm gonna be playing around with this and throwing some notes at you.. :)
First off,
I'm getting this exception whenever I try to run it with Quarkus:

java.lang.NullPointerException: Cannot invoke "io.smallrye.graphql.schema.model.CustomScalarType$CustomScalarPrimitiveType.ordinal()" because "primitiveType" is null
	at io.smallrye.graphql.bootstrap.Bootstrap.getCoercing(Bootstrap.java:263)
	at io.smallrye.graphql.bootstrap.Bootstrap.createGraphQLCustomScalarType(Bootstrap.java:245)
	at io.smallrye.graphql.bootstrap.Bootstrap.createGraphQLCustomScalarTypes(Bootstrap.java:237)
	at io.smallrye.graphql.bootstrap.Bootstrap.generateGraphQLSchema(Bootstrap.java:170)
	at io.smallrye.graphql.bootstrap.Bootstrap.bootstrap(Bootstrap.java:126)
	at io.smallrye.graphql.cdi.producer.GraphQLProducer.initialize(GraphQLProducer.java:52)
	at io.smallrye.graphql.cdi.producer.GraphQLProducer.initialize(GraphQLProducer.java:42)
	at io.smallrye.graphql.cdi.producer.GraphQLProducer.initialize(GraphQLProducer.java:32)
	at io.smallrye.graphql.cdi.producer.GraphQLProducer.initialize(GraphQLProducer.java:27)
	at io.smallrye.graphql.cdi.producer.GraphQLProducer_ClientProxy.initialize(Unknown Source)
	at io.quarkus.smallrye.graphql.runtime.SmallRyeGraphQLRecorder.createExecutionService(SmallRyeGraphQLRecorder.java:36)
	at io.quarkus.deployment.steps.SmallRyeGraphQLProcessor$buildExecutionService1691419614.deploy_1(Unknown Source)
	at io.quarkus.deployment.steps.SmallRyeGraphQLProcessor$buildExecutionService1691419614.deploy(Unknown Source)
	at io.quarkus.runner.ApplicationImpl.<clinit>(Unknown Source)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:70)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:44)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:124)
	at io.quarkus.runner.GeneratedMain.main(Unknown Source)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.runner.bootstrap.StartupActionImpl$1.run(StartupActionImpl.java:113)
	at java.base/java.lang.Thread.run(Thread.java:833)

That is because of the CustomScalarType that is not compatible with Quarkus' requirements for bytecode recording. All fields need to have a properly named getter and setter, otherwise the fields will be null at runtime.
So you need to add a setCustomScalarPrimitiveType method and rename the getter (customScalarPrimitiveType) to getCustomScalarPrimitiveType.

@jmartisk
Copy link
Member

Also, could you add a short story about how to use it in a new file in the docs directory? Plus some Javadocs for CustomStringScalar, CustomIntScalar, CustomFloatScalar and @CustomScalar

@shirskor5
Copy link
Author

Thank you for the tip on Quarkus' requirements for bytecode recording. We have also written a test app for this and it should be working as expected now. I have also added a feature doc along with some java docs where requested. Happy to add more where it is appropriate to do so.

@jmartisk jmartisk marked this pull request as ready for review December 20, 2023 16:03
@jmartisk jmartisk merged commit 2ff693e into smallrye:main Dec 22, 2023
5 checks passed
@jmartisk
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants