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

Detect and report federated entities not covered by any @EntityMapping method #1088

Open
bystam opened this issue Nov 15, 2024 · 2 comments
Open
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@bystam
Copy link

bystam commented Nov 15, 2024

Hello!

We just rewrote our entire platform from DGS to Spring GraphQL and generally we are very happy with what you have built here! We just stumbled upon one bug we had caused by misconfiguration that got kind of buried in the runtime errors where I think a louder crash upon launch maybe should be preferable.

The "problem"

In short: We had a copy paste error in one of our @EntityMapping declarations for a federated entity, and the result was that some of our federated queries failed without any loud exception simply with the error message No entity fetcher. We noticed eventually because some of our outer use cases failed, of course, but I wonder if the library could detect this problem earlier and complain super-loudly. I think DGS does this, but I am not 100% sure.

In detail, here's what happens (if I read the code correctly):

The suggestion

What if FederationSchemaFactory could (either in afterPropertiesSet or createSchemaTransformer or similar), double check that the list of provided @EntityMapping covers all the declared, resolvable, federated entities (types with @key(..., resolvable: true)), and if not crash with a user friendly exception?

Unless I am missing something, this should be the behaviour one would want, because if you didn't want an @EntityMapping, then you likely should mark the entity in the schema as resolvable: false. And my experience in general with Spring is that it is very good at complaining loudly with a clear explanation when there is a suspected misconfiguration.


More specifically in our case - the copy-paste error we had meant that we have DUPLICATE @EntityMapping declarations for the same time - which also didn't produce any kind of warning or error - which itself maybe should've been an error.

Let me know what you think, and whether you are open to me making an attempt at building this myself and sending a PR.

@bystam bystam changed the title Complain loudly when there are missing @EntityMapping Complain loudly when there are missing @EntityMappings Nov 15, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 15, 2024
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Nov 15, 2024

Thanks for the suggestion, and glad your experience has been good.

We will consider if we can integrate this into the SchemaMappingInspector. It should be possible but federation is a higher level package and there can't be a direct dependence.

Alternatively, it would be straightforward to build it into the FederationSchemaFactory which knows about entity mappings, abd can discover federated entity types similar to the way SchemaTransformer does.

@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 15, 2024
@rstoyanchev rstoyanchev added this to the 1.4.x milestone Nov 15, 2024
@rstoyanchev rstoyanchev changed the title Complain loudly when there are missing @EntityMappings Detect and report federated entities not covered by any @EntityMapping method Nov 15, 2024
@bystam
Copy link
Author

bystam commented Nov 15, 2024

Sounds nice! Thanks for the quick reply! Let me know if I can be of help in any way.

@rstoyanchev rstoyanchev modified the milestones: 1.4.x, 1.4.0-M1 Dec 5, 2024
@rstoyanchev rstoyanchev self-assigned this Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants