Merge branch '2.3' into merge-2.3

teohhanhui · teohhanhui · commit c539432fdfd7 · 2018-07-17T11:49:31.000+02:00
diff --git a/.travis.yml b/.travis.yml
@@ -11,7 +11,7 @@ env:
     - SYMFONY_DEPRECATIONS_HELPER=weak_vendors
     - APP_ENV=test
 
-matrix:
+jobs:
   include:
     - php: '7.1'
     - php: '7.2'
@@ -32,7 +32,8 @@ matrix:
         - mysql -e 'CREATE DATABASE api_platform_test;'
       env: APP_ENV=mysql
   allow_failures:
-      env: SYMFONY_DEPRECATIONS_HELPER=0
+    - env: SYMFONY_DEPRECATIONS_HELPER=0
+  fast_finish: true
 
 before_install:
   - phpenv config-rm xdebug.ini || echo "xdebug not available"
diff --git a/appveyor.yml b/appveyor.yml
@@ -6,7 +6,8 @@ cache:
   - '%LOCALAPPDATA%\Composer\files'
 
 environment:
-    APP_ENV: 'test'
+  SYMFONY_DEPRECATIONS_HELPER: weak_vendors
+  APP_ENV: test
 
 install:
   - ps: Set-Service wuauserv -StartupType Manual
diff --git a/features/bootstrap/FeatureContext.php b/features/bootstrap/FeatureContext.php
@@ -219,6 +219,31 @@ public function thereAreDummyPropertyObjects(int $nb)
         $this->manager->flush();
     }
 
+    /**
+     * @Given there are :nb dummy property objects with a shared group
+     */
+    public function thereAreDummyPropertyObjectsWithASharedGroup(int $nb)
+    {
+        $dummyGroup = new DummyGroup();
+        foreach (['foo', 'bar', 'baz'] as $property) {
+            $dummyGroup->$property = ucfirst($property).' #shared';
+        }
+        $this->manager->persist($dummyGroup);
+
+        for ($i = 1; $i <= $nb; ++$i) {
+            $dummyProperty = new DummyProperty();
+
+            foreach (['foo', 'bar', 'baz'] as $property) {
+                $dummyProperty->$property = ucfirst($property).' #'.$i;
+            }
+
+            $dummyProperty->group = $dummyGroup;
+            $this->manager->persist($dummyProperty);
+        }
+
+        $this->manager->flush();
+    }
+
     /**
      * @Given there are :nb dummy property objects with :nb2 groups
      */
diff --git a/features/graphql/filters.feature b/features/graphql/filters.feature
@@ -154,3 +154,28 @@ Feature: Collections filtering
     And the header "Content-Type" should be equal to "application/json"
     And the JSON node "data.dummies.edges[0].node.name" should be equal to "Dummy #2"
     And the JSON node "data.dummies.edges[1].node.name" should be equal to "Dummy #1"
+
+  @createSchema
+  Scenario: Retrieve a collection filtered using the related search filter with two values and exact strategy
+    Given there are 3 dummy objects with relatedDummy
+    When  I send the following GraphQL request:
+    """
+    {
+      dummies(relatedDummy_name_list: ["RelatedDummy #1", "RelatedDummy #2"]) {
+        edges {
+          node {
+            id
+            name
+            relatedDummy {
+              name
+            }
+          }
+        }
+      }
+    }
+    """
+    Then the response status code should be 200
+    And the header "Content-Type" should be equal to "application/json"
+    And the JSON node "data.dummies.edges" should have 2 element
+    And the JSON node "data.dummies.edges[0].node.relatedDummy.name" should be equal to "RelatedDummy #1"
+    And the JSON node "data.dummies.edges[1].node.relatedDummy.name" should be equal to "RelatedDummy #2"
diff --git a/features/jsonapi/related-resouces-inclusion.feature b/features/jsonapi/related-resouces-inclusion.feature
@@ -496,3 +496,91 @@ Feature: JSON API Inclusion of Related Resources
     }
   """
 
+  @createSchema
+  Scenario: Request inclusion of a related resources on collection should not duplicated included object
+    Given there are 3 dummy property objects with a shared group
+    When I send a "GET" request to "/dummy_properties?include=group"
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the JSON should be valid according to the JSON API schema
+    And the JSON should be deep equal to:
+    """
+    {
+        "links": {
+            "self": "\/dummy_properties?include=group"
+        },
+        "meta": {
+            "totalItems": 3,
+            "itemsPerPage": 3,
+            "currentPage": 1
+        },
+        "data": [
+            {
+                "id": "/dummy_properties/1",
+                "type": "DummyProperty",
+                "attributes": {
+                    "_id": 1,
+                    "foo": "Foo #1",
+                    "bar": "Bar #1",
+                    "baz": "Baz #1"
+                },
+                "relationships": {
+                    "group": {
+                        "data": {
+                            "type": "DummyGroup",
+                            "id": "/dummy_groups/1"
+                        }
+                    }
+                }
+            },
+            {
+                "id": "/dummy_properties/2",
+                "type": "DummyProperty",
+                "attributes": {
+                    "_id": 2,
+                    "foo": "Foo #2",
+                    "bar": "Bar #2",
+                    "baz": "Baz #2"
+                },
+                "relationships": {
+                    "group": {
+                        "data": {
+                            "type": "DummyGroup",
+                            "id": "/dummy_groups/1"
+                        }
+                    }
+                }
+            },
+            {
+                "id": "/dummy_properties/3",
+                "type": "DummyProperty",
+                "attributes": {
+                    "_id": 3,
+                    "foo": "Foo #3",
+                    "bar": "Bar #3",
+                    "baz": "Baz #3"
+                },
+                "relationships": {
+                    "group": {
+                        "data": {
+                            "type": "DummyGroup",
+                            "id": "/dummy_groups/1"
+                        }
+                    }
+                }
+            }
+        ],
+        "included": [
+            {
+                "id": "\/dummy_groups\/1",
+                "type": "DummyGroup",
+                "attributes": {
+                    "_id": 1,
+                    "foo": "Foo #shared",
+                    "bar": "Bar #shared",
+                    "baz": "Baz #shared"
+                }
+            }
+        ]
+    }
+  """
diff --git a/src/Annotation/AttributesHydratorTrait.php b/src/Annotation/AttributesHydratorTrait.php
@@ -22,7 +22,7 @@
 namespace ApiPlatform\Core\Annotation;
 
 use ApiPlatform\Core\Exception\InvalidArgumentException;
-use Doctrine\Common\Util\Inflector;
+use Doctrine\Common\Inflector\Inflector;
 
 /**
  * Hydrates attributes from annotation's parameters.
diff --git a/src/Bridge/Symfony/Routing/RouteNameGenerator.php b/src/Bridge/Symfony/Routing/RouteNameGenerator.php
@@ -16,7 +16,7 @@
 use ApiPlatform\Core\Api\OperationType;
 use ApiPlatform\Core\Api\OperationTypeDeprecationHelper;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
-use Doctrine\Common\Util\Inflector;
+use Doctrine\Common\Inflector\Inflector;
 
 /**
  * Generates the Symfony route name associated with an operation name and a resource short name.
diff --git a/src/GraphQl/Resolver/Factory/CollectionResolverFactory.php b/src/GraphQl/Resolver/Factory/CollectionResolverFactory.php
@@ -162,13 +162,16 @@ private function getSubresource(string $rootClass, array $rootResolvedFields, ar
     private function getNormalizedFilters(array $args): array
     {
         $filters = $args;
+
         foreach ($filters as $name => $value) {
             if (\is_array($value)) {
+                if (strpos($name, '_list')) {
+                    $name = substr($name, 0, \strlen($name) - \strlen('_list'));
+                }
                 $filters[$name] = $this->getNormalizedFilters($value);
-                continue;
             }
 
-            if (strpos($name, '_')) {
+            if (\is_string($name) && strpos($name, '_')) {
                 // Gives a chance to relations/nested fields.
                 $filters[str_replace('_', '.', $name)] = $value;
             }
diff --git a/src/GraphQl/Type/SchemaBuilder.php b/src/GraphQl/Type/SchemaBuilder.php
@@ -23,7 +23,7 @@
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface;
 use ApiPlatform\Core\Metadata\Resource\ResourceMetadata;
 use ApiPlatform\Core\Util\ClassInfoTrait;
-use Doctrine\Common\Util\Inflector;
+use Doctrine\Common\Inflector\Inflector;
 use GraphQL\Type\Definition\InputObjectType;
 use GraphQL\Type\Definition\InterfaceType;
 use GraphQL\Type\Definition\ObjectType;
@@ -231,12 +231,15 @@ private function getResourceFieldConfiguration(string $resourceClass, ResourceMe
                         $filterType = \in_array($value['type'], Type::$builtinTypes, true) ? new Type($value['type'], $nullable) : new Type('object', $nullable, $value['type']);
                         $graphqlFilterType = $this->convertType($filterType, false, null, $depth);
 
-                        if ('[]' === $newKey = substr($key, -2)) {
-                            $key = $newKey;
+                        if ('[]' === substr($key, -2)) {
                             $graphqlFilterType = GraphQLType::listOf($graphqlFilterType);
+                            $key = substr($key, 0, -2).'_list';
                         }
 
                         parse_str($key, $parsed);
+                        if (array_key_exists($key, $parsed) && \is_array($parsed[$key])) {
+                            $parsed = [$key => ''];
+                        }
                         array_walk_recursive($parsed, function (&$value) use ($graphqlFilterType) {
                             $value = $graphqlFilterType;
                         });
@@ -297,6 +300,7 @@ private function convertFilterArgsToTypes(array $args): array
             if (strpos($key, '.')) {
                 // Declare relations/nested fields in a GraphQL compatible syntax.
                 $args[str_replace('.', '_', $key)] = $value;
+                unset($args[$key]);
             }
         }
 
diff --git a/src/HttpCache/EventListener/AddTagsListener.php b/src/HttpCache/EventListener/AddTagsListener.php
@@ -56,7 +56,7 @@ public function onKernelResponse(FilterResponseEvent $event)
         }
 
         $resources = $request->attributes->get('_resources');
-        if (isset($attributes['collection_operation_name'])) {
+        if (isset($attributes['collection_operation_name']) || ($attributes['subresource_context']['collection'] ?? false)) {
             // Allows to purge collections
             $iri = $this->iriConverter->getIriFromResourceClass($attributes['resource_class']);
             $resources[$iri] = $iri;
diff --git a/src/JsonApi/Serializer/CollectionNormalizer.php b/src/JsonApi/Serializer/CollectionNormalizer.php
@@ -90,7 +90,7 @@ protected function getItemsData($object, string $format = null, array $context =
             $data['data'][] = $item['data'];
 
             if (isset($item['included'])) {
-                $data['included'] = array_merge($data['included'] ?? [], $item['included']);
+                $data['included'] = array_unique(array_merge($data['included'] ?? [], $item['included']), SORT_REGULAR);
             }
         }
 
diff --git a/src/Operation/DashPathSegmentNameGenerator.php b/src/Operation/DashPathSegmentNameGenerator.php
@@ -13,7 +13,7 @@
 
 namespace ApiPlatform\Core\Operation;
 
-use Doctrine\Common\Util\Inflector;
+use Doctrine\Common\Inflector\Inflector;
 
 /**
  * Generate a path name with a dash separator according to a string and whether it's a collection or not.
diff --git a/src/Operation/UnderscorePathSegmentNameGenerator.php b/src/Operation/UnderscorePathSegmentNameGenerator.php
@@ -13,7 +13,7 @@
 
 namespace ApiPlatform\Core\Operation;
 
-use Doctrine\Common\Util\Inflector;
+use Doctrine\Common\Inflector\Inflector;
 
 /**
  * Generate a path name with an underscore separator according to a string and whether it's a collection or not.
diff --git a/tests/GraphQl/Type/SchemaBuilderTest.php b/tests/GraphQl/Type/SchemaBuilderTest.php
@@ -95,6 +95,32 @@ public function testGetSchemaResourceClassNotFound()
         $this->assertArrayNotHasKey('objectProperty', $type->getFields());
     }
 
+    public function testConvertFilterArgsToTypes()
+    {
+        $propertyMetadataMockBuilder = function ($builtinType, $resourceClassName) {
+            return new PropertyMetadata();
+        };
+        $mockedSchemaBuilder = $this->createSchemaBuilder($propertyMetadataMockBuilder, false);
+
+        $reflectionClass = new \ReflectionClass(SchemaBuilder::class);
+        $method = $reflectionClass->getMethod('convertFilterArgsToTypes');
+        $method->setAccessible(true);
+        $filterArgs = [
+            'aField' => 'string',
+            'GraphqlRelatedResource.nestedFieldA' => 'string',
+            'GraphqlRelatedResource.nestedFieldB' => 'string',
+        ];
+
+        $this->assertSame(
+            [
+                'aField' => 'string',
+                'GraphqlRelatedResource_nestedFieldA' => 'string',
+                'GraphqlRelatedResource_nestedFieldB' => 'string',
+            ],
+            $method->invoke($mockedSchemaBuilder, $filterArgs)
+        );
+    }
+
     /**
      * @dataProvider paginationProvider
      */
@@ -230,8 +256,10 @@ private function createSchemaBuilder($propertyMetadataMockBuilder, bool $paginat
         $resourceNameCollection = new ResourceNameCollection($resourceClassNames);
         $resourceNameCollectionFactoryProphecy->create()->willReturn($resourceNameCollection);
 
-        $collectionResolverFactoryProphecy->__invoke(Argument::cetera())->willReturn(function () {});
-        $itemMutationResolverFactoryProphecy->__invoke(Argument::cetera())->willReturn(function () {});
+        $collectionResolverFactoryProphecy->__invoke(Argument::cetera())->willReturn(function () {
+        });
+        $itemMutationResolverFactoryProphecy->__invoke(Argument::cetera())->willReturn(function () {
+        });
 
         return new SchemaBuilder(
             $propertyNameCollectionFactoryProphecy->reveal(),
@@ -240,8 +268,10 @@ private function createSchemaBuilder($propertyMetadataMockBuilder, bool $paginat
             $resourceMetadataFactoryProphecy->reveal(),
             $collectionResolverFactoryProphecy->reveal(),
             $itemMutationResolverFactoryProphecy->reveal(),
-            function () {},
-            function () {},
+            function () {
+            },
+            function () {
+            },
             null,
             $paginationEnabled
         );
diff --git a/tests/HttpCache/EventListener/AddTagsListenerTest.php b/tests/HttpCache/EventListener/AddTagsListenerTest.php
@@ -166,4 +166,25 @@ public function testAddCollectionIriWhenCollectionIsEmpty()
 
         $this->assertSame('/dummies', $response->headers->get('Cache-Tags'));
     }
+
+    public function testAddSubResourceCollectionIri()
+    {
+        $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
+        $iriConverterProphecy->getIriFromResourceClass(Dummy::class)->willReturn('/dummies')->shouldBeCalled();
+
+        $request = new Request([], [], ['_resources' => ['/foo', '/bar'], '_api_resource_class' => Dummy::class, '_api_subresource_operation_name' => 'api_dummies_relatedDummies_get_subresource', '_api_subresource_context' => ['collection' => true]]);
+
+        $response = new Response();
+        $response->setPublic();
+        $response->setEtag('foo');
+
+        $event = $this->prophesize(FilterResponseEvent::class);
+        $event->getRequest()->willReturn($request)->shouldBeCalled();
+        $event->getResponse()->willReturn($response)->shouldBeCalled();
+
+        $listener = new AddTagsListener($iriConverterProphecy->reveal());
+        $listener->onKernelResponse($event->reveal());
+
+        $this->assertSame('/foo,/bar,/dummies', $response->headers->get('Cache-Tags'));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -162,13 +162,16 @@ private function getSubresource(string $rootClass, array $rootResolvedFields, ar`
`162`	`162`	`private function getNormalizedFilters(array $args): array`
`163`	`163`	`{`
`164`	`164`	`$filters = $args;`
	`165`	`+`
`165`	`166`	`foreach ($filters as $name => $value) {`
`166`	`167`	`if (\is_array($value)) {`
	`168`	`+ if (strpos($name, '_list')) {`
	`169`	`+ $name = substr($name, 0, \strlen($name) - \strlen('_list'));`
	`170`	`+ }`
`167`	`171`	`$filters[$name] = $this->getNormalizedFilters($value);`
`168`		`- continue;`
`169`	`172`	`}`
`170`	`173`
`171`		`- if (strpos($name, '_')) {`
	`174`	`+ if (\is_string($name) && strpos($name, '_')) {`
`172`	`175`	`// Gives a chance to relations/nested fields.`
`173`	`176`	`$filters[str_replace('_', '.', $name)] = $value;`
`174`	`177`	`}`
Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ public function onKernelResponse(FilterResponseEvent $event)`
`56`	`56`	`}`
`57`	`57`
`58`	`58`	`$resources = $request->attributes->get('_resources');`
`59`		`- if (isset($attributes['collection_operation_name'])) {`
	`59`	`+ if (isset($attributes['collection_operation_name']) \|\| ($attributes['subresource_context']['collection'] ?? false)) {`
`60`	`60`	`// Allows to purge collections`
`61`	`61`	`$iri = $this->iriConverter->getIriFromResourceClass($attributes['resource_class']);`
`62`	`62`	`$resources[$iri] = $iri;`