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

Allow edition of the schema with @Observe GraphQLSchema.Builder #1869

Open
Pilpin opened this issue Jul 21, 2023 · 2 comments
Open

Allow edition of the schema with @Observe GraphQLSchema.Builder #1869

Pilpin opened this issue Jul 21, 2023 · 2 comments

Comments

@Pilpin
Copy link

Pilpin commented Jul 21, 2023

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 quarkusio/quarkus#34733

Thanks.

@jmartisk
Copy link
Member

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
Copy link
Member

jmartisk commented Jul 24, 2023

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?

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

2 participants