[GraphQL] making item_query and collection_query as top level operati…

…on (#3015) * making item_query and collection query as top level operations so user can set different security and serialization groups * fixing php unit tests * fixing review changes requested: - typo in FieldsBuilder - using if instead of elseif in TypeBuilder - Concatenation place in TypeBuilder * fixing requested review changes: - reorder authorization tests - moving item_query and collection_query conditions to SchemaBuilder - updating changelog.md
api-platform · Aug 26, 2019 · b6ddb27 · b6ddb27
1 parent 7ac5857
commit b6ddb27
Show file tree

Hide file tree

Showing 23 changed files with 169 additions and 52 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,9 @@
 * GraphQL: Allow to use a search and an exist filter on the same resource
 * GraphQL: Refactor the architecture of the whole system to allow the decoration of useful services (`TypeConverter` to manage custom types, `SerializerContextBuilder` to modify the (de)serialization context dynamically, etc.)
 
+**BC Break** 
+* GraphQL: Separate item_query and collection_query operations so user can use different security and serialization groups for them
+
 ## 2.4.6
 
 * GraphQL: Use correct resource configuration for filter arguments of nested collection

diff --git a/features/graphql/authorization.feature b/features/graphql/authorization.feature
@@ -40,6 +40,47 @@ Feature: Authorization checking
     And the header "Content-Type" should be equal to "application/json"
     And the JSON node "errors[0].message" should be equal to "Access Denied."
 
+  Scenario: An admin can retrieve collection resource
+    When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
+    And I send the following GraphQL request:
+    """
+    {
+      securedDummies {
+        edges {
+          node {
+            title
+            description
+          }
+        }
+      }
+    }
+    """
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/json"
+    And the JSON node "data.securedDummies" should not be null
+
+  Scenario: A regular user can't retrieve collection resource
+    When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
+    And I send the following GraphQL request:
+    """
+    {
+      securedDummies {
+        edges {
+          node {
+            title
+            description
+          }
+        }
+      }
+    }
+    """
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/json"
+    And the JSON node "data.securedDummies" should be null
+    And the JSON node "errors[0].message" should be equal to "Access Denied."
+
   Scenario: An anonymous user tries to create a resource they are not allowed to
     When I send the following GraphQL request:
     """
@@ -165,3 +206,4 @@ Feature: Authorization checking
     And the response should be in JSON
     And the header "Content-Type" should be equal to "application/json"
     And the JSON node "data.updateSecuredDummy.securedDummy.owner" should be equal to the string "vincent"
+
diff --git a/features/graphql/collection.feature b/features/graphql/collection.feature
@@ -9,7 +9,7 @@ Feature: GraphQL collection support
         ...dummyFields
       }
     }
-    fragment dummyFields on DummyConnection {
+    fragment dummyFields on DummyCollectionConnection {
       edges {
         node {
           id

diff --git a/features/graphql/introspection.feature b/features/graphql/introspection.feature
@@ -21,7 +21,7 @@ Feature: GraphQL introspection support
     When I send the following GraphQL request:
     """
     {
-      type1: __type(name: "DummyProduct") {
+      type1: __type(name: "DummyProductItem") {
         description,
         fields {
           name
@@ -35,7 +35,7 @@ Feature: GraphQL introspection support
           }
         }
       }
-      type2: __type(name: "DummyAggregateOfferConnection") {
+      type2: __type(name: "DummyAggregateOfferItemConnection") {
         description,
         fields {
           name
@@ -49,7 +49,7 @@ Feature: GraphQL introspection support
           }
         }
       }
-      type3: __type(name: "DummyAggregateOfferEdge") {
+      type3: __type(name: "DummyAggregateOfferItemEdge") {
         description,
         fields {
           name
@@ -69,12 +69,12 @@ Feature: GraphQL introspection support
     And the response should be in JSON
     And the header "Content-Type" should be equal to "application/json"
     And the JSON node "data.type1.description" should be equal to "Dummy Product."
-    And the JSON node "data.type1.fields[1].type.name" should be equal to "DummyAggregateOfferConnection"
+    And the JSON node "data.type1.fields[1].type.name" should be equal to "DummyAggregateOfferItemConnection"
     And the JSON node "data.type2.fields[0].name" should be equal to "edges"
-    And the JSON node "data.type2.fields[0].type.ofType.name" should be equal to "DummyAggregateOfferEdge"
+    And the JSON node "data.type2.fields[0].type.ofType.name" should be equal to "DummyAggregateOfferItemEdge"
     And the JSON node "data.type3.fields[0].name" should be equal to "node"
     And the JSON node "data.type3.fields[1].name" should be equal to "cursor"
-    And the JSON node "data.type3.fields[0].type.name" should be equal to "DummyAggregateOffer"
+    And the JSON node "data.type3.fields[0].type.name" should be equal to "DummyAggregateOfferItem"
 
   Scenario: Introspect deprecated queries
     When I send the following GraphQL request:
@@ -121,7 +121,7 @@ Feature: GraphQL introspection support
     When I send the following GraphQL request:
     """
     {
-      __type(name: "DeprecatedResource") {
+      __type(name: "DeprecatedResourceItem") {
         fields(includeDeprecated: true) {
           name
           isDeprecated
@@ -224,7 +224,7 @@ Feature: GraphQL introspection support
     When I send the following GraphQL request:
     """
     {
-      __type(name: "Dummy") {
+      __type(name: "DummyItem") {
         description,
         fields {
           name
@@ -250,7 +250,7 @@ Feature: GraphQL introspection support
     When I send the following GraphQL request:
     """
     {
-      typeQuery: __type(name: "DummyGroup") {
+      typeQuery: __type(name: "DummyGroupItem") {
         description,
         fields {
           name
@@ -405,4 +405,4 @@ Feature: GraphQL introspection support
     And the header "Content-Type" should be equal to "application/json"
     And the JSON node "data.dummyItem.name" should be equal to "Dummy #3"
     And the JSON node "data.dummyItem.relatedDummy.name" should be equal to "RelatedDummy #3"
-    And the JSON node "data.dummyItem.relatedDummy.__typename" should be equal to "RelatedDummy"
+    And the JSON node "data.dummyItem.relatedDummy.__typename" should be equal to "RelatedDummyItem"
diff --git a/features/graphql/query.feature b/features/graphql/query.feature
@@ -23,7 +23,7 @@ Feature: GraphQL query support
     {
       node(id: "/dummies/1") {
         id
-        ... on Dummy {
+        ... on DummyItem {
           name
         }
       }

diff --git a/features/graphql/schema.feature b/features/graphql/schema.feature
@@ -5,7 +5,7 @@ Feature: GraphQL schema-related features
     When I run the command "api:graphql:export"
     Then the command output should contain:
     """
-    ###Dummy Friend.###
+   ###Dummy Friend.###
     type DummyFriend implements Node {
       id: ID!
 
@@ -16,26 +16,48 @@ Feature: GraphQL schema-related features
       name: String!
     }
 
-    ###Connection for DummyFriend.###
-    type DummyFriendConnection {
-      edges: [DummyFriendEdge]
-      pageInfo: DummyFriendPageInfo!
+    ###Dummy Friend.###
+    type DummyFriendCollection implements Node {
+      id: ID!
+
+      ###The id###
+      _id: Int!
+
+      ###The dummy name###
+      name: String!
+    }
+
+    ###Connection for DummyFriendCollection.###
+    type DummyFriendCollectionConnection {
+      edges: [DummyFriendCollectionEdge]
+      pageInfo: DummyFriendCollectionPageInfo!
       totalCount: Int!
     }
 
-    ###Edge of DummyFriend.###
-    type DummyFriendEdge {
-      node: DummyFriend
+    ###Edge of DummyFriendCollection.###
+    type DummyFriendCollectionEdge {
+      node: DummyFriendCollection
       cursor: String!
     }
 
     ###Information about the current page.###
-    type DummyFriendPageInfo {
+    type DummyFriendCollectionPageInfo {
       endCursor: String
       startCursor: String
       hasNextPage: Boolean!
       hasPreviousPage: Boolean!
     }
+
+    ###Dummy Friend.###
+    type DummyFriendItem implements Node {
+      id: ID!
+
+      ###The id###
+      _id: Int!
+
+      ###The dummy name###
+      name: String!
+    }
     """
 
   Scenario: Export the GraphQL schema in SDL with comment descriptions
@@ -54,24 +76,46 @@ Feature: GraphQL schema-related features
       name: String!
     }
 
-    # Connection for DummyFriend.
-    type DummyFriendConnection {
-      edges: [DummyFriendEdge]
-      pageInfo: DummyFriendPageInfo!
+    # Dummy Friend.
+    type DummyFriendCollection implements Node {
+      id: ID!
+
+      # The id
+      _id: Int!
+
+      # The dummy name
+      name: String!
+    }
+
+    # Connection for DummyFriendCollection.
+    type DummyFriendCollectionConnection {
+      edges: [DummyFriendCollectionEdge]
+      pageInfo: DummyFriendCollectionPageInfo!
       totalCount: Int!
     }
 
-    # Edge of DummyFriend.
-    type DummyFriendEdge {
-      node: DummyFriend
+    # Edge of DummyFriendCollection.
+    type DummyFriendCollectionEdge {
+      node: DummyFriendCollection
       cursor: String!
     }
 
     # Information about the current page.
-    type DummyFriendPageInfo {
+    type DummyFriendCollectionPageInfo {
       endCursor: String
       startCursor: String
       hasNextPage: Boolean!
       hasPreviousPage: Boolean!
     }
+
+    # Dummy Friend.
+    type DummyFriendItem implements Node {
+      id: ID!
+
+      # The id
+      _id: Int!
+
+      # The dummy name
+      name: String!
+    }
     """
diff --git a/src/GraphQl/Resolver/Factory/CollectionResolverFactory.php b/src/GraphQl/Resolver/Factory/CollectionResolverFactory.php
@@ -70,7 +70,7 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul
                 );
             }
 
-            $operationName = $operationName ?? 'query';
+            $operationName = $operationName ?? 'collection_query';
             $resolverContext = ['source' => $source, 'args' => $args, 'info' => $info, 'is_collection' => true, 'is_mutation' => false];
 
             $collection = ($this->readStage)($resourceClass, $rootClass, $operationName, $resolverContext);

diff --git a/src/GraphQl/Resolver/Factory/ItemResolverFactory.php b/src/GraphQl/Resolver/Factory/ItemResolverFactory.php
@@ -64,7 +64,7 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul
                 return $source[$info->fieldName];
             }
 
-            $operationName = $operationName ?? 'query';
+            $operationName = $operationName ?? 'item_query';
             $resolverContext = ['source' => $source, 'args' => $args, 'info' => $info, 'is_collection' => false, 'is_mutation' => false];
 
             $item = ($this->readStage)($resourceClass, $rootClass, $operationName, $resolverContext);

diff --git a/src/GraphQl/Type/FieldsBuilder.php b/src/GraphQl/Type/FieldsBuilder.php
@@ -86,7 +86,7 @@ public function getQueryFields(string $resourceClass, ResourceMetadata $resource
     {
         $queryFields = [];
         $shortName = $resourceMetadata->getShortName();
-        $fieldName = lcfirst('query' === $queryName ? $shortName : $queryName.$shortName);
+        $fieldName = lcfirst('item_query' === $queryName || 'collection_query' === $queryName ? $shortName : $queryName.$shortName);
 
         $deprecationReason = $resourceMetadata->getGraphqlAttribute($queryName, 'deprecation_reason', '', true);
 
@@ -167,7 +167,7 @@ public function getResourceObjectTypeFields(?string $resourceClass, ResourceMeta
 
         if (null !== $resourceClass) {
             foreach ($this->propertyNameCollectionFactory->create($resourceClass) as $property) {
-                $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $property, ['graphql_operation_name' => $mutationName ?? $queryName ?? 'query']);
+                $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $property, ['graphql_operation_name' => $mutationName ?? $queryName ?? 'item_query']);
                 if (
                     null === ($propertyType = $propertyMetadata->getType())
                     || (!$input && false === $propertyMetadata->isReadable())

diff --git a/src/GraphQl/Type/SchemaBuilder.php b/src/GraphQl/Type/SchemaBuilder.php
@@ -60,8 +60,14 @@ public function getSchema(): Schema
             /** @var array<string, mixed> $graphqlConfiguration */
             $graphqlConfiguration = $resourceMetadata->getGraphql() ?? [];
             foreach ($graphqlConfiguration as $operationName => $value) {
-                if ('query' === $operationName) {
-                    $queryFields += $this->fieldsBuilder->getQueryFields($resourceClass, $resourceMetadata, $operationName, [], []);
+                if ('item_query' === $operationName) {
+                    $queryFields += $this->fieldsBuilder->getQueryFields($resourceClass, $resourceMetadata, $operationName, [], false);
+
+                    continue;
+                }
+
+                if ('collection_query' === $operationName) {
+                    $queryFields += $this->fieldsBuilder->getQueryFields($resourceClass, $resourceMetadata, $operationName, false, []);
 
                     continue;
                 }

diff --git a/src/GraphQl/Type/TypeBuilder.php b/src/GraphQl/Type/TypeBuilder.php
@@ -61,6 +61,12 @@ public function getResourceObjectType(?string $resourceClass, ResourceMetadata $
             }
             $shortName .= 'Payload';
         }
+        if ('item_query' === $queryName) {
+            $shortName .= 'Item';
+        }
+        if ('collection_query' === $queryName) {
+            $shortName .= 'Collection';
+        }
         if ($wrapped && null !== $mutationName) {
             $shortName .= 'Data';
         }
@@ -152,7 +158,7 @@ public function getNodeInterface(): InterfaceType
                     return null;
                 }
 
-                $shortName = (new \ReflectionClass($value[ItemNormalizer::ITEM_RESOURCE_CLASS_KEY]))->getShortName();
+                $shortName = (new \ReflectionClass($value[ItemNormalizer::ITEM_RESOURCE_CLASS_KEY]))->getShortName().'Item';
 
                 return $this->typesContainer->has($shortName) ? $this->typesContainer->get($shortName) : null;
             },

diff --git a/src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php b/src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php
@@ -83,7 +83,7 @@ public function create(string $resourceClass): ResourceMetadata
 
         $graphql = $resourceMetadata->getGraphql();
         if (null === $graphql) {
-            $resourceMetadata = $resourceMetadata->withGraphql(['query' => [], 'delete' => [], 'update' => [], 'create' => []]);
+            $resourceMetadata = $resourceMetadata->withGraphql(['item_query' => [], 'collection_query' => [], 'delete' => [], 'update' => [], 'create' => []]);
         } else {
             $resourceMetadata = $this->normalizeGraphQl($resourceMetadata, $graphql);
         }

diff --git a/tests/Fixtures/TestBundle/Document/DummyGroup.php b/tests/Fixtures/TestBundle/Document/DummyGroup.php
@@ -37,7 +37,8 @@
  *         }
  *     },
  *     graphql={
- *         "query"={"normalization_context"={"groups"={"dummy_foo"}}},
+ *         "item_query"={"normalization_context"={"groups"={"dummy_foo"}}},
+ *         "collection_query"={"normalization_context"={"groups"={"dummy_foo"}}},
  *         "delete",
  *         "create"={
  *             "normalization_context"={"groups"={"dummy_bar"}},

diff --git a/tests/Fixtures/TestBundle/Document/DummyProperty.php b/tests/Fixtures/TestBundle/Document/DummyProperty.php
@@ -35,7 +35,8 @@
  *         }
  *     },
  *     graphql={
- *         "query",
+ *         "item_query",
+ *         "collection_query",
  *         "update",
  *         "delete",
  *         "create"={

diff --git a/tests/Fixtures/TestBundle/Document/LegacySecuredDummy.php b/tests/Fixtures/TestBundle/Document/LegacySecuredDummy.php
@@ -33,7 +33,7 @@
  *         "put"={"access_control"="is_granted('ROLE_USER') and previous_object.getOwner() == user"},
  *     },
  *     graphql={
- *         "query"={"access_control"="is_granted('ROLE_USER') and object.getOwner() == user"},
+ *         "item_query"={"access_control"="is_granted('ROLE_USER') and object.getOwner() == user"},
  *         "delete"={},
  *         "update"={"access_control"="is_granted('ROLE_USER') and previous_object.getOwner() ==  user"},
  *         "create"={"access_control"="is_granted('ROLE_ADMIN')", "access_control_message"="Only admins can create a secured dummy."}