jcrygier / graphql-jpa

JPA Implementation of GraphQL (builds on graphql-java)
MIT License
165 stars 46 forks source link

Upgrade to graphql-java v3.0.0 #37

Closed jcrygier closed 7 years ago

jcrygier commented 7 years ago

Attempts to upgrade to v3.0.0 lead to the following (from @timtebeek):

java.lang.IllegalStateException: Failed to load ApplicationContext
    at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:124)
...
Caused by: graphql.AssertException: All types within a GraphQL schema must have unique names. No two provided types may have the same name.
No provided type may have a name which conflicts with any built in types (including Scalar and Introspection types).
You have redefined the type 'Episode' from being a 'GraphQLEnumType' to a 'GraphQLEnumType'
    at graphql.schema.SchemaUtil.assertTypeUniqueness(SchemaUtil.java:86)
    at graphql.schema.SchemaUtil.collectTypes(SchemaUtil.java:50)
    at graphql.schema.SchemaUtil.collectTypes(SchemaUtil.java:48)
    at graphql.schema.SchemaUtil.collectTypesForObjects(SchemaUtil.java:130)
    at graphql.schema.SchemaUtil.collectTypes(SchemaUtil.java:56)
    at graphql.schema.SchemaUtil.allTypes(SchemaUtil.java:153)
    at graphql.schema.GraphQLSchema.<init>(GraphQLSchema.java:42)
    at graphql.schema.GraphQLSchema$Builder.build(GraphQLSchema.java:130)
    at graphql.schema.GraphQLSchema$Builder.build(GraphQLSchema.java:125)
    at org.crygier.graphql.GraphQLSchemaBuilder.getGraphQLSchema(GraphQLSchemaBuilder.java:37)
    at org.crygier.graphql.GraphQLExecutor.createGraphQL(GraphQLExecutor.java:27)
...

Caused by repeated invocations of this function for org.crygier.graphql.model.starwars.Episode: https://github.com/jcrygier/graphql-jpa/blob/master/src/main/java/org/crygier/graphql/GraphQLSchemaBuilder.java#L218

jcrygier commented 7 years ago

Pushed simple caching solution, so each subsequent call would reference the same object. Not the most elegant solution, but it works.

Bumped version.

timtebeek commented 7 years ago

Great to see the issue resolved so quickly; it's funny, I'd thought of doing the same but figured that'd never get past code review.. ;)

jcrygier commented 7 years ago

Ha, yeah....I really want to pass a 'context' object thru so it's a bit cleaner...but that would have required a bunch of refactoring of some of the Java8 Function References.

The references will stick around in memory a bit longer this way, but i'm assuming it will be okay, as they're JPA references and Class references, that I'm guessing are permanently in memory anyway.