From 1c4bbe2b62fd406c8002672203e964bcec3ca1e7 Mon Sep 17 00:00:00 2001 From: Bahurski Ivan <i89819878196@gmail.com> Date: Sun, 10 Jul 2022 22:39:30 +0300 Subject: [PATCH] bugfix: where query with embedded entities produce an exception when type specific opetator is used --- .../schema/impl/GraphQLJpaSchemaBuilder.java | 136 ++++++++++-------- .../embeddedid/EntityWithEmbeddedIdTest.java | 8 +- .../query/schema/GraphQLExecutorTests.java | 14 ++ .../query/schema/model/embedded/Engine.java | 1 + .../src/test/resources/data.sql | 8 +- 5 files changed, 99 insertions(+), 68 deletions(-) diff --git a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java index 4937b1cc4..28d6a8cc3 100644 --- a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java +++ b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java @@ -20,36 +20,6 @@ import static graphql.schema.GraphQLInputObjectField.newInputObjectField; import static graphql.schema.GraphQLInputObjectType.newInputObject; -import java.beans.Introspector; -import java.lang.reflect.AnnotatedElement; -import java.lang.reflect.Field; -import java.lang.reflect.Member; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Supplier; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import javax.persistence.Convert; -import javax.persistence.EntityManager; -import javax.persistence.metamodel.Attribute; -import javax.persistence.metamodel.EmbeddableType; -import javax.persistence.metamodel.EntityType; -import javax.persistence.metamodel.ManagedType; -import javax.persistence.metamodel.PluralAttribute; -import javax.persistence.metamodel.SingularAttribute; -import javax.persistence.metamodel.Type; - -import org.dataloader.MappedBatchLoaderWithContext; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import com.introproventures.graphql.jpa.query.annotation.GraphQLIgnore; import com.introproventures.graphql.jpa.query.annotation.GraphQLIgnoreFilter; import com.introproventures.graphql.jpa.query.annotation.GraphQLIgnoreOrder; @@ -60,7 +30,6 @@ import com.introproventures.graphql.jpa.query.schema.impl.EntityIntrospector.EntityIntrospectionResult.AttributePropertyDescriptor; import com.introproventures.graphql.jpa.query.schema.impl.PredicateFilter.Criteria; import com.introproventures.graphql.jpa.query.schema.relay.GraphQLJpaRelayDataFetcher; - import graphql.Assert; import graphql.Directives; import graphql.Scalars; @@ -81,6 +50,33 @@ import graphql.schema.GraphQLType; import graphql.schema.GraphQLTypeReference; import graphql.schema.PropertyDataFetcher; +import java.beans.Introspector; +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Field; +import java.lang.reflect.Member; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import javax.persistence.Convert; +import javax.persistence.EntityManager; +import javax.persistence.metamodel.Attribute; +import javax.persistence.metamodel.EmbeddableType; +import javax.persistence.metamodel.EntityType; +import javax.persistence.metamodel.ManagedType; +import javax.persistence.metamodel.PluralAttribute; +import javax.persistence.metamodel.SingularAttribute; +import javax.persistence.metamodel.Type; +import org.dataloader.MappedBatchLoaderWithContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * JPA specific schema builder implementation of {code #GraphQLSchemaBuilder} interface @@ -133,7 +129,7 @@ public class GraphQLJpaSchemaBuilder implements GraphQLSchemaBuilder { private int defaultFetchSize = 100; private int defaultPageLimitSize = 100; private boolean enableDefaultMaxResults = true; - + private RestrictedKeysProvider restrictedKeysProvider = (entityDescriptor) -> Optional.of(Collections.emptyList()); private final Relay relay = new Relay(); @@ -506,7 +502,7 @@ private String resolveTypeName(ManagedType<?> managedType) { String typeName=""; if (managedType instanceof EmbeddableType){ - typeName = managedType.getJavaType().getSimpleName()+"EmbeddableType"; + typeName = managedType.getJavaType().getSimpleName(); } else if (managedType instanceof EntityType) { typeName = ((EntityType<?>)managedType).getName(); } @@ -515,7 +511,14 @@ private String resolveTypeName(ManagedType<?> managedType) { } private GraphQLInputObjectType getWhereInputType(ManagedType<?> managedType) { - return inputObjectCache.computeIfAbsent(managedType, this::computeWhereInputType); + GraphQLInputObjectType type = inputObjectCache.get(managedType); + if (type == null) { + type = computeWhereInputType(managedType); + inputObjectCache.put(managedType, type); + return type; + } + return type; + } private String resolveWhereInputTypeName(ManagedType<?> managedType) { @@ -611,6 +614,11 @@ private GraphQLInputType getWhereAttributeType(Attribute<?,?> attribute) { if(whereAttributesMap.containsKey(type)) return whereAttributesMap.get(type); + if (isEmbeddable(attribute)) { + EmbeddableType<?> embeddableType = (EmbeddableType<?>) ((SingularAttribute<?, ?>) attribute).getType(); + return getWhereInputType(embeddableType); + } + GraphQLInputObjectType.Builder builder = GraphQLInputObjectType.newInputObject() .name(type) .description("Criteria expression specification of "+namingStrategy.singularize(attribute.getName())+" attribute in entity " + attribute.getDeclaringType().getJavaType()) @@ -787,7 +795,7 @@ else if (attribute.getJavaMember().getClass().isAssignableFrom(Field.class) } private GraphQLArgument getArgument(Attribute<?,?> attribute) { - GraphQLInputType type = getAttributeInputType(attribute); + GraphQLInputType type = getAttributeInputTypeForSearchByIdArg(attribute); String description = getSchemaDescription(attribute); return GraphQLArgument.newArgument() @@ -797,25 +805,33 @@ private GraphQLArgument getArgument(Attribute<?,?> attribute) { .build(); } - private GraphQLType getEmbeddableType(EmbeddableType<?> embeddableType, boolean input) { - if (input && embeddableInputCache.containsKey(embeddableType.getJavaType())) - return embeddableInputCache.get(embeddableType.getJavaType()); - - if (!input && embeddableOutputCache.containsKey(embeddableType.getJavaType())) - return embeddableOutputCache.get(embeddableType.getJavaType()); - String embeddableTypeName = namingStrategy.singularize(embeddableType.getJavaType().getSimpleName())+ (input ? "Input" : "") +"EmbeddableType"; - GraphQLType graphQLType=null; + private GraphQLType getEmbeddableType(EmbeddableType<?> embeddableType, boolean input, boolean searchByIdArg) { + GraphQLType graphQLType; if (input) { - graphQLType = GraphQLInputObjectType.newInputObject() - .name(embeddableTypeName) + + if (searchByIdArg) { + if (embeddableInputCache.containsKey(embeddableType.getJavaType())) { + return embeddableInputCache.get(embeddableType.getJavaType()); + } + graphQLType = GraphQLInputObjectType.newInputObject() + .name(namingStrategy.singularize(embeddableType.getJavaType().getSimpleName())+ "InputEmbeddableIdType") .description(getSchemaDescription(embeddableType)) .fields(embeddableType.getAttributes().stream() - .filter(this::isNotIgnored) - .map(this::getInputObjectField) - .collect(Collectors.toList()) + .filter(this::isNotIgnored) + .map(this::getInputObjectField) + .collect(Collectors.toList()) ) .build(); + embeddableInputCache.put(embeddableType.getJavaType(), (GraphQLInputObjectType) graphQLType); + return graphQLType; + } + + graphQLType = getWhereInputType(embeddableType); } else { + if (embeddableOutputCache.containsKey(embeddableType.getJavaType())) { + return embeddableOutputCache.get(embeddableType.getJavaType()); + } + String embeddableTypeName = namingStrategy.singularize(embeddableType.getJavaType().getSimpleName()) + "EmbeddableType"; graphQLType = GraphQLObjectType.newObject() .name(embeddableTypeName) .description(getSchemaDescription(embeddableType)) @@ -825,13 +841,8 @@ private GraphQLType getEmbeddableType(EmbeddableType<?> embeddableType, boolean .collect(Collectors.toList()) ) .build(); - } - if (input) { - embeddableInputCache.putIfAbsent(embeddableType.getJavaType(), (GraphQLInputObjectType) graphQLType); - } else{ embeddableOutputCache.putIfAbsent(embeddableType.getJavaType(), (GraphQLObjectType) graphQLType); } - return graphQLType; } @@ -1021,9 +1032,17 @@ private Stream<Attribute<?,?>> findBasicAttributes(Collection<Attribute<?,?>> at } private GraphQLInputType getAttributeInputType(Attribute<?,?> attribute) { + return getAttributeInputType(attribute, false); + } + + private GraphQLInputType getAttributeInputTypeForSearchByIdArg(Attribute<?,?> attribute) { + return getAttributeInputType(attribute, true); + } + + private GraphQLInputType getAttributeInputType(Attribute<?,?> attribute, boolean searchByIdArgType) { try { - return (GraphQLInputType) getAttributeType(attribute, true); + return (GraphQLInputType) getAttributeType(attribute, true, searchByIdArgType); } catch (ClassCastException e){ throw new IllegalArgumentException("Attribute " + attribute + " cannot be mapped as an Input Argument"); } @@ -1031,21 +1050,21 @@ private GraphQLInputType getAttributeInputType(Attribute<?,?> attribute) { private GraphQLOutputType getAttributeOutputType(Attribute<?,?> attribute) { try { - return (GraphQLOutputType) getAttributeType(attribute, false); + return (GraphQLOutputType) getAttributeType(attribute, false, false); } catch (ClassCastException e) { throw new IllegalArgumentException("Attribute " + attribute + " cannot be mapped as an Output Argument"); } } @SuppressWarnings( "rawtypes" ) - protected GraphQLType getAttributeType(Attribute<?,?> attribute, boolean input) { + protected GraphQLType getAttributeType(Attribute<?,?> attribute, boolean input, boolean searchByIdArgType) { if (isBasic(attribute)) { return getGraphQLTypeFromJavaType(attribute.getJavaType()); } else if (isEmbeddable(attribute)) { EmbeddableType embeddableType = (EmbeddableType) ((SingularAttribute) attribute).getType(); - return getEmbeddableType(embeddableType, input); + return getEmbeddableType(embeddableType, input, searchByIdArgType); } else if (isToMany(attribute)) { EntityType foreignType = (EntityType) ((PluralAttribute) attribute).getElementType(); @@ -1067,8 +1086,7 @@ else if (isElementCollection(attribute)) { } else if (foreignType.getPersistenceType() == Type.PersistenceType.EMBEDDABLE) { EmbeddableType embeddableType = EmbeddableType.class.cast(foreignType); - GraphQLType graphQLType = getEmbeddableType(embeddableType, - input); + GraphQLType graphQLType = getEmbeddableType(embeddableType, input, searchByIdArgType); return input ? graphQLType : new GraphQLList(graphQLType); } diff --git a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/embeddedid/EntityWithEmbeddedIdTest.java b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/embeddedid/EntityWithEmbeddedIdTest.java index 1f8b6ff06..020cb4efe 100644 --- a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/embeddedid/EntityWithEmbeddedIdTest.java +++ b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/embeddedid/EntityWithEmbeddedIdTest.java @@ -94,16 +94,14 @@ public void queryBooksWithyWithEmbeddedId() { } @Test - public void queryBooksWithyWithEmbeddedIdWhereCriteriaExpression() { + public void queryBooksWithWithEmbeddedIdWhereCriteriaExpression() { //given String query = "query {" + " Books( " + " where: {" + " bookId: {" + - " EQ: {" + - " title: \"War and Piece\"" + - " language: \"Russian\"" + - " }" + + " title: {LIKE: \"War % Piece\"}" + + " language: {EQ: \"Russian\"}" + " }" + " }" + " ){" + diff --git a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorTests.java b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorTests.java index fbe38f03d..3d8737aea 100644 --- a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorTests.java +++ b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorTests.java @@ -770,6 +770,20 @@ public void queryForEntityWithMappedSuperclass() { assertThat(result.toString()).isEqualTo(expected); } + @Test + public void whereQueryWithSeparateEmbeddedEntityFieldsCriterias() { + //given + String query = "query { Cars(where: { engine: {identification:{LIKE:\"%\"}, hp:{GE:200}} }) { select {id brand} } }"; + + String expected = "{Cars={select=[{id=2, brand=Cadillac}]}}"; + + //when + Object result = executor.execute(query).getData(); + + // then + assertThat(result.toString()).isEqualTo(expected); + } + // https://github.com/introproventures/graphql-jpa-query/issues/30 @Test public void queryForEntityWithEmbeddedIdAndEmbeddedField() { diff --git a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/model/embedded/Engine.java b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/model/embedded/Engine.java index 5235ea139..36c3ef57a 100644 --- a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/model/embedded/Engine.java +++ b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/model/embedded/Engine.java @@ -6,5 +6,6 @@ public class Engine { String identification; + Long hp; } diff --git a/graphql-jpa-query-schema/src/test/resources/data.sql b/graphql-jpa-query-schema/src/test/resources/data.sql index 1af705250..8317d8fbc 100644 --- a/graphql-jpa-query-schema/src/test/resources/data.sql +++ b/graphql-jpa-query-schema/src/test/resources/data.sql @@ -143,10 +143,10 @@ insert into book_publishers(book_id, name, country) values (2, 'Willey', 'US'), (2, 'Simon', 'EU'); -- Car -insert into Car (id, brand, identification) values - (1, 'Ford', 'xxxxx'), - (2, 'Cadillac', 'yyyyy'), - (3, 'Toyota', 'zzzzz'); +insert into Car (id, brand, identification, hp) values + (1, 'Ford', 'xxxxx', 160), + (2, 'Cadillac', 'yyyyy', 250), + (3, 'Toyota', 'zzzzz', 180); -- Boat