graphql-java-generator / graphql-maven-plugin-project

graphql-maven-plugin is a Maven Plugin for GraphQL, based on graphql-java. It accelerates the development for both the client and the server, by generating the Java code. It allows a quicker development when in contract-first approach, by avoiding to code the boilerplate code.
https://graphql-maven-plugin-project.graphql-java-generator.com
MIT License
115 stars 47 forks source link

Cannot use custom Controllers without implementing default delegates #215

Open Vojtech-Sassmann opened 1 week ago

Vojtech-Sassmann commented 1 week ago

Since version 2.5, there is a DataFetchersDelegateRegistry class which requires all generated delegates. This prevents defining a custom controller with a custom implementation.

etienne-sf commented 1 week ago

Can you provide here yous custom implementation ? Like stated in the wiki, your custom controller must override the default one.

Please note that this may be related to the #214 issue, on which I'm working.

Etienne

Vojtech-Sassmann commented 1 week ago

Can you provide here yous custom implementation ? Like stated in the wiki, your custom controller must override the default one.

Please note that this may be related to the #214 issue, on which I'm working.

Etienne

Sure thing!

For example, with this schema:

type Foo {
    id: ID!
    name: String
}

extend type Query {
    foo(id: ID!): Foo
}

The library generates the following classes:

However, we have a use case when the methods in the DataFetchersDelegateFoo are not sufficient. (Eg. to fetch some related entities, we want to use DataLoader. But the library doesn't know this, because we are not using the ID in this one situation like we do for all other entities.)

Therefore, we wanted to implement our own custom FooController. We did it like so:

@Controller
@SchemaMapping(typeName = "Foo")
public class FooController
{
   // custom methods
}

And this worked until version 2.5 where a new class DataFetchersDelegateRegistry is generated. This class also requires the DataFetchersDelegateFoo. Because of that, we must provide a dummy implementation of this interface which does nothing.

Like stated ... your custom controller must override the default one.

Does this mean that this use case is not supported? As you have mentioned, the custom controller MUST override the default one?

etienne-sf commented 6 days ago

Hello,

I've read your message several time, but I'm not quite sure of the issue you encounter.

What I understand is that it should work with this dummy implementation of the DataFetchersDelegateFoo and the overrided controller that extends the FooController class.

Can you precise the error you get with this configuration ? (dummy implementation of the DataFetchersDelegateFoo and the overrided controller that extends the FooController class)

Etienne

Vojtech-Sassmann commented 5 days ago

Hello,

I've read your message several time, but I'm not quite sure of the issue you encounter.

What I understand is that it should work with this dummy implementation of the DataFetchersDelegateFoo and the overrided controller that extends the FooController class.

Can you precise the error you get with this configuration ? (dummy implementation of the DataFetchersDelegateFoo and the overrided controller that extends the FooController class)

Etienne

I'm sorry for such confusing description of the problem.

The problem is that I don't like the dummy implementation of the DataFetchersDelegateFoo. I would prefer not having to write it.

Shouldn't be the DataFetchersDelegateFoo not required in the DataFetchersDelegateRegistry ?

etienne-sf commented 5 days ago

The problem is that I don't like the dummy implementation of the DataFetchersDelegateFoo. I would prefer not having to write it.

Yes, crystal clear. I agree with you

Shouldn't be the DataFetchersDelegateFoo not required in the DataFetchersDelegateRegistry ?

The question here is: how to mark this DataFetchersDelegate to be optional?

Here are my thinking on this:

  1. I could have a default implementation for each DataFetchersDelegate in the spring auto-configuration. But I choose not to do that, as I found safer to have an error at startup when a controller is missing, than discover it at runtime (for instance when recompiling on a new version of the GraphQL schema that needs new controllers). This should remain the default behaviour
  2. There could be a parameter which, when set, generate this default implementation for all controllers. But again, the risk to have a missing DataFetchersDelegate seems a much higher issue to me, than having to write a dummy DataFetchersDelegate
  3. Marking just one or several data fetchers as optional seems at first, complex to manager, as a plugin parameters. And actually it's a bad answer to the real needs, which has not link with DataFetchersDelegate. The need is to override the mapping for one (or more) controller entry
  4. Marking some mapping that should not be generated

I guess a parameter that contains a list of TypeName.fieldName for which no controller should be generated seems not that hard, I guess. And it's quite simple to document.

What's your opinion there? Any other idea?