smallrye-graphql icon indicating copy to clipboard operation
smallrye-graphql copied to clipboard

Allow edition of the schema with @Observe GraphQLSchema.Builder

Open Pilpin opened this issue 2 years ago • 2 comments

Hello,

I'm using quarkus and would like to edit the schema so I tried using a function like public GraphQLSchema.Builder editSchema(@Observes GraphQLSchema.Builder builder) but as far as I understand I can only add things to the builder, I can not edit the schema.

I tried something like this

public GraphQLSchema.Builder edit(@Observes GraphQLSchema.Builder builder) {
    GraphQLSchema _schema = SchemaTransformer.transformSchema(builder.build(), new GraphQLTypeVisitorStub() {
        @Override
        public TraversalControl visitGraphQLObjectType(
                GraphQLObjectType node,
                TraverserContext<GraphQLSchemaElement> context
        ) {
            if (condition) {
                System.out.println(node);
                return changeNode(context, node.transform(b -> {
                    b.doSomething();
                }));
            }

            return super.visitGraphQLObjectType(node, context);
        }
    });

    return GraphQLSchema.newSchema(_schema);
}

The System.out.println(node); gets executed but the schema at url/q/dev-ui/io.quarkus.quarkus-smallrye-graphql/graphql-schema doesn't change.

Am I missing something ? Is this even possible ?

Here's the reproducer https://github.com/Pilpin/quarkus-graphql-schema-bug In it I'm trying to add a field to the country object, and the logs seem to show that it works, up until the observing function returns.

First asked about this on quakus' discussions https://github.com/quarkusio/quarkus/discussions/34733

Thanks.

Pilpin avatar Jul 21 '23 07:07 Pilpin

This is because we are using CDI events for calling the observing method, and CDI events are "fire-and-forget", thus we basically ignore the returned value from the observer... See https://github.com/smallrye/smallrye-graphql/blob/2.2.1/server/implementation-cdi/src/main/java/io/smallrye/graphql/cdi/event/EventsService.java#L29 - here you see that we call the observer with a Builder object, but if the observer returns a different Builder, then we ignore that. Changes to the original Builder are applied, but if the observer returns a different instance, it is discarded.

So the implementation is indeed wrong and I think we will have to step away from using CDI events, and rework it to do something else... Unless CDI events allow you to somehow access the objects returned from observers, which I think they don't.

jmartisk avatar Jul 24 '23 13:07 jmartisk

Thinking what would be the least intrusive way to fix this... We could change the signature of the beforeSchemaBuild method to void beforeSchemaBuild(SchemaBuilderHolder holder) or add this as a new variant to maintain backward compatibility

You would then change it via

void updateSchema(@Observes SchemaBuilderHolder holder) {
   GraphQLSchema.Builder newBuilder = holder.getSchemaBuilder() .... // and some transformations here
   holder.setSchemaBuilder(newBuilder);
}

The SchemaBuilderHolder would be a wrapper that only contains a reference to GraphQLSchema.Builder, so when the event observer changes that reference, the container would be able to receive the "new" builder instance from there. It's ugly but it could work. Any better ideas?

jmartisk avatar Jul 24 '23 14:07 jmartisk