From f13c31a1952b55f0bbfa153963b53a9f6ce61954 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sun, 26 Mar 2017 18:32:36 +0200 Subject: [PATCH 01/37] Add basic support for query parameters including the include param --- src/Listener/JsonApiListener.php | 83 ++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 7c1104071..44422ead3 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -5,6 +5,8 @@ use Cake\Event\Event; use Cake\Network\Exception\BadRequestException; use Cake\ORM\Association; +use Cake\ORM\Table; +use Cake\Utility\Hash; use Cake\Utility\Inflector; use Crud\Error\Exception\CrudException; use Crud\Event\Subject; @@ -55,6 +57,7 @@ class JsonApiListener extends ApiListener 'include' => [], 'fieldSets' => [], // hash to limit fields shown (can be used for both `data` and `included` members) 'docValidatorAboutLinks' => false, // true to show links to JSON API specification clarifying the document validation error + 'queryParameters' => [], //Array of query parameters and associated transformers ]; /** @@ -89,6 +92,8 @@ public function implementedEvents() 'Crud.afterDelete' => ['callable' => [$this, 'afterDelete'], 'priority' => 90], 'Crud.beforeRender' => ['callable' => [$this, 'respond'], 'priority' => 100], 'Crud.beforeRedirect' => ['callable' => [$this, 'beforeRedirect'], 'priority' => 100], + 'Crud.beforePaginate' => ['callable' => [$this, 'beforeFind'], 'priority' => 10], + 'Crud.beforeFind' => ['callable' => [$this, 'beforeFind'], 'priority' => 10], ]; } @@ -166,6 +171,79 @@ public function beforeRedirect(Event $event) $event->stopPropagation(); } + protected function _getIncludes() + { + $includes = Hash::filter(explode(',', $this->request->getQuery('include'))); + if (empty($includes)) { + return; + } + + return array_map(function ($include) { + return Inflector::camelize($include, '-'); + }, $includes); + } + + protected function _parseInclude($include, Table $repository = null) + { + $includes = explode('.', $include); + $includeString = []; + $currentRepository = $repository; + foreach ($includes as $include) { + $associationName = Inflector::camelize($include); + + $association = $currentRepository->association($associationName); + if ($association === null) { + throw new BadRequestException('Invalid relationship path supplied in include parameter'); + } + + $currentRepository = $association->target(); + $includeString[] = $associationName; + } + + return implode('.', $includeString); + } + + protected function _parseIncludes($parameter, Subject $subject) + { + $includes = Hash::filter(explode(',', $this->_request()->getQuery($parameter))); + + if (empty($includes)) { + return; + } + + $contains = []; + foreach ($includes as $include) { + $contain = $this->_parseInclude($include, $subject->query->repository()); + + if (!empty($contain)) { + $contains[] = $contain; + } + } + + $subject->query->contain($contains, true); + } + + /** + * BeforeFind event listener to parse any supplied query parameters + * + * @param \Cake\Event\Event $event Event + * @return void + */ + public function beforeFind(Event $event) + { + $queryParameters = $this->config('queryParameters') + [ + 'include' => [$this, '_parseIncludes'] + ]; + + foreach ($queryParameters as $parameter => $callable) { + if (!is_callable($callable)) { + throw new \InvalidArgumentException('Invalid callable supplied for query parameter ' . $parameter); + } + + $callable($parameter, $event->subject()); + } + } + /** * respond() event. * @@ -379,6 +457,11 @@ protected function _validateConfigOptions() if (!is_bool($this->config('debugPrettyPrint'))) { throw new CrudException('JsonApiListener configuration option `debugPrettyPrint` only accepts a boolean'); } + + if (!is_array($this->config('queryParameters'))) { + throw new CrudException('JsonApiListener configuration option `queryParameters` only accepts an array'); + } + } /** From 149498300c5a582f2b0d18f8c0c68aba68efa784 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sun, 26 Mar 2017 18:34:50 +0200 Subject: [PATCH 02/37] Base associations on contain value instead of entity key value --- src/Listener/JsonApiListener.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 44422ead3..5eced93d8 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -382,7 +382,7 @@ protected function _renderWithResources($subject) // Remove associations not found in the `find()` result $entity = $this->_getSingleEntity($subject); - $strippedAssociations = $this->_stripNonContainedAssociations($repository, $entity); + $strippedAssociations = $this->_stripNonContainedAssociations($repository, $subject->query); // Set data before rendering the view $this->_controller()->set([ @@ -530,15 +530,15 @@ protected function _getSingleEntity($subject) * @param \Cake\ORM\Entity $entity Entity * @return \Cake\ORM\AssociationCollection */ - protected function _stripNonContainedAssociations($repository, $entity) + protected function _stripNonContainedAssociations($repository, $query) { $associations = $repository->associations(); + $contains = $query->contain(); foreach ($associations as $association) { $associationKey = strtolower($association->name()); - $entityKey = $association->property(); - if (empty($entity->$entityKey)) { + if (!isset($contains[$association->name()])) { $associations->remove($associationKey); } } From c7469bc82fafa511fcb3508de28ca36b672c0892 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sun, 26 Mar 2017 18:35:18 +0200 Subject: [PATCH 03/37] Do not override already set schemas --- src/View/JsonApiView.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/View/JsonApiView.php b/src/View/JsonApiView.php index 777321ee6..3220ea256 100644 --- a/src/View/JsonApiView.php +++ b/src/View/JsonApiView.php @@ -206,6 +206,10 @@ protected function _entitiesToNeoMerxSchema(array $repositories) { $schemas = []; foreach ($repositories as $repositoryName => $repository) { + if (isset($schemas[$repository->entityClass()])) { + continue; + } + $entityClass = $repository->entityClass(); // Turn full class name back into plugin split format From bdf8b201246698a6023d896e886c383b123c905a Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sun, 26 Mar 2017 19:21:29 +0200 Subject: [PATCH 04/37] Update tests to pass --- tests/TestCase/Listener/JsonApiListenerTest.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index edb900828..9453b6f79 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -7,6 +7,7 @@ use Cake\Filesystem\File; use Cake\Network\Request; use Cake\Network\Response; +use Cake\ORM\Query; use Cake\ORM\TableRegistry; use Crud\Event\Subject; use Crud\Listener\JsonApiListener; @@ -59,6 +60,7 @@ public function testDefaultConfig() 'include' => [], 'fieldSets' => [], 'docValidatorAboutLinks' => false, + 'queryParameters' => [], ]; $this->assertSame($expected, $listener->config()); @@ -117,7 +119,9 @@ public function testImplementedEvents() 'Crud.afterSave' => ['callable' => [$listener, 'afterSave'], 'priority' => 90], 'Crud.afterDelete' => ['callable' => [$listener, 'afterDelete'], 'priority' => 90], 'Crud.beforeRender' => ['callable' => [$listener, 'respond'], 'priority' => 100], - 'Crud.beforeRedirect' => ['callable' => [$listener, 'beforeRedirect'], 'priority' => 100] + 'Crud.beforeRedirect' => ['callable' => [$listener, 'beforeRedirect'], 'priority' => 100], + 'Crud.beforePaginate' => ['callable' => [$listener, 'beforeFind'], 'priority' => 10], + 'Crud.beforeFind' => ['callable' => [$listener, 'beforeFind'], 'priority' => 10], ]; $this->assertSame($expected, $result); @@ -571,6 +575,7 @@ public function testRenderWithResources() $subject = $this ->getMockBuilder('\Crud\Event\Subject') ->getMock(); + $subject->query = $this->createMock(Query::class); $subject->entity = new Country(); $listener->render($subject); @@ -1098,7 +1103,7 @@ public function testStripNonContainedAssociations() // make sure cultures are removed from AssociationCollection $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); - $associationsAfter = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $entity], $listener); + $associationsAfter = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $query], $listener); $this->assertNotEmpty($associationsAfter->get('currencies')); $this->assertNull($associationsAfter->get('cultures')); From ae171f374a75f7951528ab3998a26f7b980f25d8 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sun, 26 Mar 2017 19:43:01 +0200 Subject: [PATCH 05/37] Test that query param config test works --- .../TestCase/Listener/JsonApiListenerTest.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index 9453b6f79..09145b6ed 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -925,6 +925,26 @@ public function testValidateConfigOptionDebugPrettyPrintFailWithString() $this->callProtectedMethod('_validateConfigOptions', [], $listener); } + /** + * Make sure config option `queryParameters` does not accept a string + * + * @expectedException \Crud\Error\Exception\CrudException + * @expectedExceptionMessage JsonApiListener configuration option `queryParameters` only accepts an array + */ + public function testValidateConfigOptionQueryParametersPrintFailWithString() + { + $listener = $this->getMockBuilder('\Crud\Listener\JsonApiListener') + ->disableOriginalConstructor() + ->setMethods(null) + ->getMock(); + + $listener->config([ + 'queryParameters' => 'string-not-accepted' + ]); + + $this->setReflectionClassInstance($listener); + $this->callProtectedMethod('_validateConfigOptions', [], $listener); + } /** * Make sure the listener accepts the correct request headers * From 71698e14e6811a1e14732ebd297e8b430fe4f6ab Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sun, 26 Mar 2017 20:11:45 +0200 Subject: [PATCH 06/37] Update tests --- src/Listener/JsonApiListener.php | 2 +- tests/TestCase/Schema/DynamicEntitySchemaTest.php | 4 ++-- tests/TestCase/View/JsonApiViewTest.php | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 5eced93d8..441907e67 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -527,7 +527,7 @@ protected function _getSingleEntity($subject) * prevent `null` entries appearing in the json api `relationships` node. * * @param \Cake\Datasource\RepositoryInterface $repository Repository - * @param \Cake\ORM\Entity $entity Entity + * @param \Cake\ORM\Query $query Query * @return \Cake\ORM\AssociationCollection */ protected function _stripNonContainedAssociations($repository, $query) diff --git a/tests/TestCase/Schema/DynamicEntitySchemaTest.php b/tests/TestCase/Schema/DynamicEntitySchemaTest.php index 7eb952b3b..c23ccf4f1 100644 --- a/tests/TestCase/Schema/DynamicEntitySchemaTest.php +++ b/tests/TestCase/Schema/DynamicEntitySchemaTest.php @@ -61,7 +61,7 @@ public function testGetAttributes() // get required AssociationsCollection $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); - $associations = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $entity], $listener); + $associations = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $query], $listener); // make view return associations on get('_associations') call $view = $this @@ -130,7 +130,7 @@ public function testRelationships() // get required AssociationsCollection $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); - $associations = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $entity], $listener); + $associations = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $query], $listener); // make view return associations on get('_associations') call $view = $this diff --git a/tests/TestCase/View/JsonApiViewTest.php b/tests/TestCase/View/JsonApiViewTest.php index 17c2f0c3a..62a069bd0 100644 --- a/tests/TestCase/View/JsonApiViewTest.php +++ b/tests/TestCase/View/JsonApiViewTest.php @@ -8,6 +8,7 @@ use Cake\Filesystem\File; use Cake\Network\Request; use Cake\Network\Response; +use Cake\ORM\Query; use Cake\ORM\TableRegistry; use Cake\Utility\Inflector; use Crud\Event\Subject; @@ -142,6 +143,7 @@ protected function _getView($tableName, $viewVars) } else { $subject->entity = $findResult; } + $subject->query = $table->query(); // create required '_entities' and '_associations' viewVars normally // produced and set by the JsonApiListener @@ -153,7 +155,7 @@ protected function _getView($tableName, $viewVars) $this->setReflectionClassInstance($listener); $entity = $this->callProtectedMethod('_getSingleEntity', [$subject], $listener); - $associations = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $entity], $listener); + $associations = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $subject->query], $listener); $repositories = $this->callProtectedMethod('_getRepositoryList', [$table, $associations], $listener); $viewVars['_repositories'] = $repositories; From b4fc11c9ac65111a9262eb757d6cb6bb0d6f72a8 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 30 Mar 2017 08:36:49 +0200 Subject: [PATCH 07/37] Add some comments and small code cleanup --- src/Listener/JsonApiListener.php | 48 +++++++++++++++++++------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 441907e67..890a55214 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -171,18 +171,14 @@ public function beforeRedirect(Event $event) $event->stopPropagation(); } - protected function _getIncludes() - { - $includes = Hash::filter(explode(',', $this->request->getQuery('include'))); - if (empty($includes)) { - return; - } - - return array_map(function ($include) { - return Inflector::camelize($include, '-'); - }, $includes); - } - + /** + * Takes a "include" string and converts it into a correct CakePHP ORM association alias + * + * @param string $include The relationship to include + * @param \Cake\ORM\Table|null $repository The repository + * @return string + * @throws \Cake\Network\Exception\BadRequestException + */ protected function _parseInclude($include, Table $repository = null) { $includes = explode('.', $include); @@ -191,21 +187,33 @@ protected function _parseInclude($include, Table $repository = null) foreach ($includes as $include) { $associationName = Inflector::camelize($include); - $association = $currentRepository->association($associationName); - if ($association === null) { - throw new BadRequestException('Invalid relationship path supplied in include parameter'); - } + if ($currentRepository !== null) { + $association = $currentRepository->association($associationName); + if ($association === null) { + throw new BadRequestException('Invalid relationship path supplied in include parameter'); + } - $currentRepository = $association->target(); + $currentRepository = $association->target(); + } $includeString[] = $associationName; } return implode('.', $includeString); } - protected function _parseIncludes($parameter, Subject $subject) + /** + * Parses out include query parameter into a containable array, and contains the query. + * + * @param string|array $includes The query data + * @param \Crud\Event\Subject $subject The subject + * @return void + */ + protected function _parseIncludes($includes, Subject $subject) { - $includes = Hash::filter(explode(',', $this->_request()->getQuery($parameter))); + if (is_string($includes)) { + $includes = explode(',', $includes); + } + $includes = Hash::filter($includes); if (empty($includes)) { return; @@ -240,7 +248,7 @@ public function beforeFind(Event $event) throw new \InvalidArgumentException('Invalid callable supplied for query parameter ' . $parameter); } - $callable($parameter, $event->subject()); + $callable($this->_request()->getQuery($parameter), $event->subject()); } } From 759d5094a275856a3b74d5727fdb3536f9e3e5c2 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 30 Mar 2017 08:37:02 +0200 Subject: [PATCH 08/37] Add test for includes parser --- .../TestCase/Listener/JsonApiListenerTest.php | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index 09145b6ed..dd6463ffa 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -1367,4 +1367,28 @@ public function testConvertJsonApiDataArray() $result = $this->callProtectedMethod('_convertJsonApiDocumentArray', [$jsonApiArray], $listener); $this->assertSame($expected, $result); } + + /** + * Make sure that the include query correct splits include string into a containable format + * + * @return void + */ + public function testIncludeQuery() + { + $listener = new JsonApiListener(new Controller()); + $this->setReflectionClassInstance($listener); + + $expected = [ + 'Articles', + 'Comments.Author' + ]; + $subject = new Subject(); + $subject->query = $this->createMock(Query::class); + $subject->query + ->expects($this->once()) + ->method('contain') + ->with($expected, true); + + $this->callProtectedMethod('_parseIncludes', ['articles,comments.author', $subject], $listener); + } } From ff033cb0101c2952a47a0265daabd8776acaa913 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sun, 2 Apr 2017 17:30:43 +0200 Subject: [PATCH 09/37] Add white and black lists for include parameter --- src/Listener/JsonApiListener.php | 86 +++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 28 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 890a55214..997186433 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -57,7 +57,12 @@ class JsonApiListener extends ApiListener 'include' => [], 'fieldSets' => [], // hash to limit fields shown (can be used for both `data` and `included` members) 'docValidatorAboutLinks' => false, // true to show links to JSON API specification clarifying the document validation error - 'queryParameters' => [], //Array of query parameters and associated transformers + 'queryParameters' => [ + 'include' => [ + 'whitelist' => [], + 'blacklist' => [], + ] + ], //Array of query parameters and associated transformers ]; /** @@ -174,41 +179,60 @@ public function beforeRedirect(Event $event) /** * Takes a "include" string and converts it into a correct CakePHP ORM association alias * - * @param string $include The relationship to include + * @param string $includes The relationships to include + * @param array $blacklist Blacklisted includes + * @param array $whitelist Whitelisted options * @param \Cake\ORM\Table|null $repository The repository * @return string * @throws \Cake\Network\Exception\BadRequestException */ - protected function _parseInclude($include, Table $repository = null) + protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repository = null) { - $includes = explode('.', $include); - $includeString = []; - $currentRepository = $repository; - foreach ($includes as $include) { + $contains = []; + foreach ($includes as $include => $nestedIncludes) { + if (array_key_exists($include, $blacklist) && empty($blacklist[$include])) { + continue; + } + + if (!empty($whitelist) && !array_key_exists($include, $whitelist)) { + continue; + } + $associationName = Inflector::camelize($include); - if ($currentRepository !== null) { - $association = $currentRepository->association($associationName); + if ($repository !== null) { + $association = $repository->association($associationName); if ($association === null) { throw new BadRequestException('Invalid relationship path supplied in include parameter'); } + } - $currentRepository = $association->target(); + if (!empty($nestedIncludes)) { + $contains[$associationName] = $this->_parseIncludes( + $nestedIncludes, + empty($blacklist[$include]) ? [] : $blacklist[$include], + empty($whitelist[$include]) ? [] : $whitelist[$include], + $association ? $association->target() : null + ); + } else { + $contains[] = $associationName; } - $includeString[] = $associationName; } - return implode('.', $includeString); + return $contains; } /** * Parses out include query parameter into a containable array, and contains the query. * + * Supported options is "Whitelist" and "Blacklist" + * * @param string|array $includes The query data * @param \Crud\Event\Subject $subject The subject + * @param array $options Array of options for includes. * @return void */ - protected function _parseIncludes($includes, Subject $subject) + protected function _includeParameter($includes, Subject $subject, $options) { if (is_string($includes)) { $includes = explode(',', $includes); @@ -219,16 +243,13 @@ protected function _parseIncludes($includes, Subject $subject) return; } - $contains = []; - foreach ($includes as $include) { - $contain = $this->_parseInclude($include, $subject->query->repository()); + $includes = Hash::expand(Hash::normalize($includes)); + $blacklist = Hash::expand(Hash::normalize($options['blacklist'])); + $whitelist = Hash::expand(Hash::normalize($options['whitelist'])); + $contains = $this->_parseIncludes($includes, $blacklist, $whitelist, $subject->query->repository()); - if (!empty($contain)) { - $contains[] = $contain; - } - } - - $subject->query->contain($contains, true); + $this->config('include', array_keys(Hash::flatten($includes))); + $subject->query->contain($contains); } /** @@ -239,16 +260,25 @@ protected function _parseIncludes($includes, Subject $subject) */ public function beforeFind(Event $event) { - $queryParameters = $this->config('queryParameters') + [ - 'include' => [$this, '_parseIncludes'] - ]; + //Inject default query handlers + $queryParameters = Hash::merge($this->config('queryParameters'), [ + 'include' => [ + 'callable' => [$this, '_includeParameter'] + ] + ]); + + foreach ($queryParameters as $parameter => $options) { + if (is_callable($options)) { + $options = [ + 'callable' => $options + ]; + } - foreach ($queryParameters as $parameter => $callable) { - if (!is_callable($callable)) { + if (!is_callable($options['callable'])) { throw new \InvalidArgumentException('Invalid callable supplied for query parameter ' . $parameter); } - $callable($this->_request()->getQuery($parameter), $event->subject()); + $options['callable']($this->_request()->getQuery($parameter), $event->subject(), $options); } } From db7ecc71740fb8390099e3a3f23acbfae49ebeac Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sun, 2 Apr 2017 17:34:22 +0200 Subject: [PATCH 10/37] Update test to pass with include changes --- src/Listener/JsonApiListener.php | 1 + tests/TestCase/Listener/JsonApiListenerTest.php | 13 +++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 997186433..93e90272b 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -198,6 +198,7 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo continue; } + $association = null; $associationName = Inflector::camelize($include); if ($repository !== null) { diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index dd6463ffa..7138010f3 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -60,7 +60,12 @@ public function testDefaultConfig() 'include' => [], 'fieldSets' => [], 'docValidatorAboutLinks' => false, - 'queryParameters' => [], + 'queryParameters' => [ + 'include' => [ + 'whitelist' => [], + 'blacklist' => [] + ] + ], ]; $this->assertSame($expected, $listener->config()); @@ -1380,15 +1385,15 @@ public function testIncludeQuery() $expected = [ 'Articles', - 'Comments.Author' + 'Comments' => ['Author'] ]; $subject = new Subject(); $subject->query = $this->createMock(Query::class); $subject->query ->expects($this->once()) ->method('contain') - ->with($expected, true); + ->with($expected); - $this->callProtectedMethod('_parseIncludes', ['articles,comments.author', $subject], $listener); + $this->callProtectedMethod('_includeParameter', ['articles,comments.author', $subject, ['blacklist' => [], 'whitelist' => []]], $listener); } } From ae5ffc10af3ac7fd58d206adbf48858a2488fbec Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sun, 2 Apr 2017 19:41:17 +0200 Subject: [PATCH 11/37] Add tests for include parsing and support for wildcards --- src/Listener/JsonApiListener.php | 26 ++++-- .../TestCase/Listener/JsonApiListenerTest.php | 90 +++++++++++++++++-- 2 files changed, 102 insertions(+), 14 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 93e90272b..b68e265bc 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -186,15 +186,24 @@ public function beforeRedirect(Event $event) * @return string * @throws \Cake\Network\Exception\BadRequestException */ - protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repository = null) + protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repository = null, $path = []) { + $wildcard = implode('.', array_merge($path, ['*'])); + $wildcardWhitelist = Hash::get($whitelist, $wildcard); + $wildcardBlacklist = Hash::get($blacklist, $wildcard); $contains = []; foreach ($includes as $include => $nestedIncludes) { - if (array_key_exists($include, $blacklist) && empty($blacklist[$include])) { + $includePath = array_merge($path, [$include]); + $includeDotPath = implode('.', $includePath); + + if (!$wildcardBlacklist && Hash::get($blacklist, $includeDotPath) === true) { continue; } - if (!empty($whitelist) && !array_key_exists($include, $whitelist)) { + if (!$wildcardWhitelist && + !empty($whitelist) && + Hash::get($whitelist, $includeDotPath) === null + ) { continue; } @@ -211,9 +220,10 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo if (!empty($nestedIncludes)) { $contains[$associationName] = $this->_parseIncludes( $nestedIncludes, - empty($blacklist[$include]) ? [] : $blacklist[$include], - empty($whitelist[$include]) ? [] : $whitelist[$include], - $association ? $association->target() : null + $blacklist, + $whitelist, + $association ? $association->target() : null, + $includePath ); } else { $contains[] = $associationName; @@ -245,8 +255,8 @@ protected function _includeParameter($includes, Subject $subject, $options) } $includes = Hash::expand(Hash::normalize($includes)); - $blacklist = Hash::expand(Hash::normalize($options['blacklist'])); - $whitelist = Hash::expand(Hash::normalize($options['whitelist'])); + $blacklist = Hash::expand(Hash::normalize(array_fill_keys($options['blacklist'], true))); + $whitelist = Hash::expand(Hash::normalize(array_fill_keys($options['whitelist'], true))); $contains = $this->_parseIncludes($includes, $blacklist, $whitelist, $subject->query->repository()); $this->config('include', array_keys(Hash::flatten($includes))); diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index 7138010f3..73923c43e 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -1373,20 +1373,98 @@ public function testConvertJsonApiDataArray() $this->assertSame($expected, $result); } + public function includeQueryProvider() + { + return [ + 'standardInclude' => [ + 'articles,comments.author', + ['blacklist' => [], 'whitelist' => []], + [ + 'Articles', + 'Comments' => ['Author'] + ] + ], + 'blacklist' => [ + 'articles,comments.author,comments.images', + ['blacklist' => ['comments.author'], 'whitelist' => []], + [ + 'Articles', + 'Comments' => [ + 'Images' + ] + ] + ], + 'whitelist' => [ + 'articles,comments.author,comments.images', + ['blacklist' => [], 'whitelist' => ['comments']], + [ + 'Comments' => [] + ] + ], + 'multiWhitelist' => [ + 'articles,comments.author,comments.images,comments.links', + ['blacklist' => [], 'whitelist' => ['comments.author', 'comments.images']], + [ + 'Comments' => [ + 'Author', + 'Images' + ] + ] + ], + 'whitelistWildcard' => [ + 'articles,comments.author,comments.images', + ['blacklist' => [], 'whitelist' => ['comments.*']], + [ + 'Comments' => [ + 'Author', + 'Images' + ] + ] + ], + 'blacklistWildcard' => [ + 'articles,comments.author,comments.images', + ['blacklist' => ['comments.*'], 'whitelist' => []], + [ + 'Comments' => [ + 'Author', + 'Images' + ], + 'Articles', + ] + ], + 'whitelistBlacklistWildcard' => [ + 'articles,comments.author,comments.images,comments.links', + ['blacklist' => ['comments.author'], 'whitelist' => ['articles', 'comments.*']], + [ + 'Comments' => [ + 'Images', + 'Links' + ], + 'Articles', + ] + ], + 'blacklist is more important' => [ + 'articles,comments.author', + ['blacklist' => ['comments.author'], 'whitelist' => ['articles', 'comments.author']], + [ + 'Comments' => [], + 'Articles', + ] + ], + ]; + } + /** * Make sure that the include query correct splits include string into a containable format * * @return void + * @dataProvider includeQueryProvider */ - public function testIncludeQuery() + public function testIncludeQuery($include, $options, $expected) { $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); - $expected = [ - 'Articles', - 'Comments' => ['Author'] - ]; $subject = new Subject(); $subject->query = $this->createMock(Query::class); $subject->query @@ -1394,6 +1472,6 @@ public function testIncludeQuery() ->method('contain') ->with($expected); - $this->callProtectedMethod('_includeParameter', ['articles,comments.author', $subject, ['blacklist' => [], 'whitelist' => []]], $listener); + $this->callProtectedMethod('_includeParameter', [$include, $subject, $options], $listener); } } From eb222eb1c2f31c4240a96b8798d86299877f3fc2 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Mon, 3 Apr 2017 09:51:54 +0200 Subject: [PATCH 12/37] Allow blacklisting/whitelisting everything --- src/Listener/JsonApiListener.php | 25 ++++++------- .../TestCase/Listener/JsonApiListenerTest.php | 36 ++++++++++++------- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index b68e265bc..d3902d588 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -59,8 +59,8 @@ class JsonApiListener extends ApiListener 'docValidatorAboutLinks' => false, // true to show links to JSON API specification clarifying the document validation error 'queryParameters' => [ 'include' => [ - 'whitelist' => [], - 'blacklist' => [], + 'whitelist' => true, + 'blacklist' => false, ] ], //Array of query parameters and associated transformers ]; @@ -180,8 +180,8 @@ public function beforeRedirect(Event $event) * Takes a "include" string and converts it into a correct CakePHP ORM association alias * * @param string $includes The relationships to include - * @param array $blacklist Blacklisted includes - * @param array $whitelist Whitelisted options + * @param array|bool $blacklist Blacklisted includes + * @param array|bool $whitelist Whitelisted options * @param \Cake\ORM\Table|null $repository The repository * @return string * @throws \Cake\Network\Exception\BadRequestException @@ -189,21 +189,22 @@ public function beforeRedirect(Event $event) protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repository = null, $path = []) { $wildcard = implode('.', array_merge($path, ['*'])); - $wildcardWhitelist = Hash::get($whitelist, $wildcard); - $wildcardBlacklist = Hash::get($blacklist, $wildcard); + $wildcardWhitelist = Hash::get((array)$whitelist, $wildcard); + $wildcardBlacklist = Hash::get((array)$blacklist, $wildcard); $contains = []; foreach ($includes as $include => $nestedIncludes) { $includePath = array_merge($path, [$include]); $includeDotPath = implode('.', $includePath); - if (!$wildcardBlacklist && Hash::get($blacklist, $includeDotPath) === true) { + if ($blacklist === true || ($blacklist !== false && !$wildcardBlacklist && Hash::get($blacklist, $includeDotPath) === true)) { continue; } - if (!$wildcardWhitelist && - !empty($whitelist) && + if ($whitelist === false || ( + $whitelist !== true && + !$wildcardWhitelist && Hash::get($whitelist, $includeDotPath) === null - ) { + )) { continue; } @@ -255,8 +256,8 @@ protected function _includeParameter($includes, Subject $subject, $options) } $includes = Hash::expand(Hash::normalize($includes)); - $blacklist = Hash::expand(Hash::normalize(array_fill_keys($options['blacklist'], true))); - $whitelist = Hash::expand(Hash::normalize(array_fill_keys($options['whitelist'], true))); + $blacklist = is_array($options['blacklist']) ? Hash::expand(Hash::normalize(array_fill_keys($options['blacklist'], true))) : $options['blacklist']; + $whitelist = is_array($options['whitelist']) ? Hash::expand(Hash::normalize(array_fill_keys($options['whitelist'], true))) : $options['whitelist']; $contains = $this->_parseIncludes($includes, $blacklist, $whitelist, $subject->query->repository()); $this->config('include', array_keys(Hash::flatten($includes))); diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index 73923c43e..df1c0ad98 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -62,8 +62,8 @@ public function testDefaultConfig() 'docValidatorAboutLinks' => false, 'queryParameters' => [ 'include' => [ - 'whitelist' => [], - 'blacklist' => [] + 'whitelist' => true, + 'blacklist' => false ] ], ]; @@ -1376,9 +1376,9 @@ public function testConvertJsonApiDataArray() public function includeQueryProvider() { return [ - 'standardInclude' => [ + 'standard' => [ 'articles,comments.author', - ['blacklist' => [], 'whitelist' => []], + ['blacklist' => false, 'whitelist' => true], [ 'Articles', 'Comments' => ['Author'] @@ -1386,7 +1386,7 @@ public function includeQueryProvider() ], 'blacklist' => [ 'articles,comments.author,comments.images', - ['blacklist' => ['comments.author'], 'whitelist' => []], + ['blacklist' => ['comments.author'], 'whitelist' => true], [ 'Articles', 'Comments' => [ @@ -1396,14 +1396,14 @@ public function includeQueryProvider() ], 'whitelist' => [ 'articles,comments.author,comments.images', - ['blacklist' => [], 'whitelist' => ['comments']], + ['blacklist' => false, 'whitelist' => ['comments']], [ 'Comments' => [] ] ], - 'multiWhitelist' => [ + 'multiple whitelists' => [ 'articles,comments.author,comments.images,comments.links', - ['blacklist' => [], 'whitelist' => ['comments.author', 'comments.images']], + ['blacklist' => false, 'whitelist' => ['comments.author', 'comments.images']], [ 'Comments' => [ 'Author', @@ -1411,9 +1411,9 @@ public function includeQueryProvider() ] ] ], - 'whitelistWildcard' => [ + 'whitelist wildcard' => [ 'articles,comments.author,comments.images', - ['blacklist' => [], 'whitelist' => ['comments.*']], + ['blacklist' => false, 'whitelist' => ['comments.*']], [ 'Comments' => [ 'Author', @@ -1421,9 +1421,9 @@ public function includeQueryProvider() ] ] ], - 'blacklistWildcard' => [ + 'blacklist wildcard' => [ 'articles,comments.author,comments.images', - ['blacklist' => ['comments.*'], 'whitelist' => []], + ['blacklist' => ['comments.*'], 'whitelist' => true], [ 'Comments' => [ 'Author', @@ -1432,7 +1432,7 @@ public function includeQueryProvider() 'Articles', ] ], - 'whitelistBlacklistWildcard' => [ + 'blacklist with a whitelist wildcard' => [ 'articles,comments.author,comments.images,comments.links', ['blacklist' => ['comments.author'], 'whitelist' => ['articles', 'comments.*']], [ @@ -1451,6 +1451,16 @@ public function includeQueryProvider() 'Articles', ] ], + 'blacklist everything' => [ + 'articles,comments.author', + ['blacklist' => true, 'whitelist' => ['articles', 'comments.author']], + [] + ], + 'whitelist nothing' => [ + 'articles,comments.author', + ['blacklist' => false, 'whitelist' => false], + [] + ], ]; } From 1183fcb543241cd0a799263c44f93921f1b838e8 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Mon, 3 Apr 2017 10:06:00 +0200 Subject: [PATCH 13/37] Only include what is actually included --- src/Listener/JsonApiListener.php | 21 +++++---- .../TestCase/Listener/JsonApiListenerTest.php | 47 ++++++++++++++----- 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index d3902d588..aa7cd684d 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -192,7 +192,9 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo $wildcardWhitelist = Hash::get((array)$whitelist, $wildcard); $wildcardBlacklist = Hash::get((array)$blacklist, $wildcard); $contains = []; + $includeConfig = $this->config('include'); foreach ($includes as $include => $nestedIncludes) { + $nestedContains = []; $includePath = array_merge($path, [$include]); $includeDotPath = implode('.', $includePath); @@ -219,18 +221,19 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo } if (!empty($nestedIncludes)) { - $contains[$associationName] = $this->_parseIncludes( - $nestedIncludes, - $blacklist, - $whitelist, - $association ? $association->target() : null, - $includePath - ); + $nestedContains = $this->_parseIncludes($nestedIncludes, $blacklist, $whitelist, $association ? $association->target() : null, $includePath);; + } + + if (!empty($nestedContains)) { + $contains[$associationName] = $nestedContains; } else { $contains[] = $associationName; + $includeConfig[] = $includeDotPath; } } + $this->config('include', $includeConfig); + return $contains; } @@ -251,16 +254,16 @@ protected function _includeParameter($includes, Subject $subject, $options) } $includes = Hash::filter($includes); - if (empty($includes)) { + if (empty($includes) || $options['blacklist'] === true || $options['whitelist'] === false) { return; } + $this->config('include', []); $includes = Hash::expand(Hash::normalize($includes)); $blacklist = is_array($options['blacklist']) ? Hash::expand(Hash::normalize(array_fill_keys($options['blacklist'], true))) : $options['blacklist']; $whitelist = is_array($options['whitelist']) ? Hash::expand(Hash::normalize(array_fill_keys($options['whitelist'], true))) : $options['whitelist']; $contains = $this->_parseIncludes($includes, $blacklist, $whitelist, $subject->query->repository()); - $this->config('include', array_keys(Hash::flatten($includes))); $subject->query->contain($contains); } diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index df1c0ad98..210ca7232 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -1382,7 +1382,10 @@ public function includeQueryProvider() [ 'Articles', 'Comments' => ['Author'] - ] + ], + [ + 'comments.author', 'articles', + ], ], 'blacklist' => [ 'articles,comments.author,comments.images', @@ -1392,14 +1395,21 @@ public function includeQueryProvider() 'Comments' => [ 'Images' ] - ] + ], + [ + 'comments.images', + 'articles', + ], ], 'whitelist' => [ 'articles,comments.author,comments.images', ['blacklist' => false, 'whitelist' => ['comments']], [ - 'Comments' => [] - ] + 'Comments' + ], + [ + 'comments' + ], ], 'multiple whitelists' => [ 'articles,comments.author,comments.images,comments.links', @@ -1409,7 +1419,11 @@ public function includeQueryProvider() 'Author', 'Images' ] - ] + ], + [ + 'comments.author', + 'comments.images' + ], ], 'whitelist wildcard' => [ 'articles,comments.author,comments.images', @@ -1419,7 +1433,8 @@ public function includeQueryProvider() 'Author', 'Images' ] - ] + ], + ['comments.author', 'comments.images'], ], 'blacklist wildcard' => [ 'articles,comments.author,comments.images', @@ -1430,7 +1445,8 @@ public function includeQueryProvider() 'Images' ], 'Articles', - ] + ], + ['comments.author', 'comments.images', 'articles'] ], 'blacklist with a whitelist wildcard' => [ 'articles,comments.author,comments.images,comments.links', @@ -1441,24 +1457,28 @@ public function includeQueryProvider() 'Links' ], 'Articles', - ] + ], + ['comments.images', 'comments.links', 'articles'] ], 'blacklist is more important' => [ 'articles,comments.author', ['blacklist' => ['comments.author'], 'whitelist' => ['articles', 'comments.author']], [ - 'Comments' => [], 'Articles', - ] + 'Comments', + ], + ['articles', 'comments'] ], 'blacklist everything' => [ 'articles,comments.author', ['blacklist' => true, 'whitelist' => ['articles', 'comments.author']], + [], [] ], 'whitelist nothing' => [ 'articles,comments.author', ['blacklist' => false, 'whitelist' => false], + [], [] ], ]; @@ -1470,7 +1490,7 @@ public function includeQueryProvider() * @return void * @dataProvider includeQueryProvider */ - public function testIncludeQuery($include, $options, $expected) + public function testIncludeQuery($include, $options, $expectedContain, $expectedInclude) { $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); @@ -1478,10 +1498,11 @@ public function testIncludeQuery($include, $options, $expected) $subject = new Subject(); $subject->query = $this->createMock(Query::class); $subject->query - ->expects($this->once()) + ->expects($options['blacklist'] !== true && $options['whitelist'] !== false ? $this->once() : $this->never()) ->method('contain') - ->with($expected); + ->with($expectedContain); $this->callProtectedMethod('_includeParameter', [$include, $subject, $options], $listener); + $this->assertSame($expectedInclude, $listener->config('include')); } } From 732d24b6d4f20214aa43a216ec41de57b7d1ea82 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Mon, 3 Apr 2017 10:29:17 +0200 Subject: [PATCH 14/37] Include blacklist wildcard --- src/Listener/JsonApiListener.php | 2 +- tests/TestCase/Listener/JsonApiListenerTest.php | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index aa7cd684d..d777a8e9f 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -198,7 +198,7 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo $includePath = array_merge($path, [$include]); $includeDotPath = implode('.', $includePath); - if ($blacklist === true || ($blacklist !== false && !$wildcardBlacklist && Hash::get($blacklist, $includeDotPath) === true)) { + if ($blacklist === true || ($blacklist !== false && ($wildcardBlacklist === true || Hash::get($blacklist, $includeDotPath) === true))) { continue; } diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index 210ca7232..4447f6de4 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -1440,13 +1440,10 @@ public function includeQueryProvider() 'articles,comments.author,comments.images', ['blacklist' => ['comments.*'], 'whitelist' => true], [ - 'Comments' => [ - 'Author', - 'Images' - ], 'Articles', + 'Comments', ], - ['comments.author', 'comments.images', 'articles'] + ['articles', 'comments',] ], 'blacklist with a whitelist wildcard' => [ 'articles,comments.author,comments.images,comments.links', From d834076e3372869cefd1384f967095144d0f47cb Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Mon, 3 Apr 2017 10:29:33 +0200 Subject: [PATCH 15/37] Add some documentation for include parameter --- docs/listeners/jsonapi.rst | 74 +++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/docs/listeners/jsonapi.rst b/docs/listeners/jsonapi.rst index c8d14778b..c07f9089a 100644 --- a/docs/listeners/jsonapi.rst +++ b/docs/listeners/jsonapi.rst @@ -31,7 +31,7 @@ Setup Routing ^^^^^^^ -Only controllers explicitely mapped can be exposed as API resources so make sure +Only controllers explicitly mapped can be exposed as API resources so make sure to configure your global routing scope in ``config/router.php`` similar to: .. code-block:: phpinline @@ -171,7 +171,7 @@ Validation errors ^^^^^^^^^^^^^^^^^ For (422) validation errors the listener produces will produce -validation error reponses in the following JSON API format. +validation error responses in the following JSON API format. .. code-block:: json @@ -204,7 +204,7 @@ Requests to the ``index`` action **must** use: - the ``HTTP GET`` request type - an ``Accept`` header set to ``application/vnd.api+json`` -A succesful request will respond with HTTP response code ``200`` +A successful request will respond with HTTP response code ``200`` and response body similar to this output produced by ``http://example.com/countries``: @@ -245,7 +245,7 @@ Requests to the ``view`` action **must** use: - the ``HTTP GET`` request type - an ``Accept`` header set to ``application/vnd.api+json`` -A succesful request will respond with HTTP response code ``200`` +A successful request will respond with HTTP response code ``200`` and response body similar to this output produced by ````http://example.com/countries/1``: @@ -275,7 +275,7 @@ Requests to the ``add`` action **must** use: - a ``Content-Type`` header set to ``application/vnd.api+json`` - request data in valid JSON API document format -A succesful request will respond with HTTP response code ``200`` +A successful request will respond with HTTP response code ``200`` and response body containing the ``id`` of the newly created record. Request failing ORM validation will result in a (422) validation error response as described earlier. @@ -353,7 +353,7 @@ All requests to the ``edit`` action **must** use: - request data in valid JSON API document format - request data containing the ``id`` of the resource to update -A succesful request will respond with HTTP response code ``200`` +A successful request will respond with HTTP response code ``200`` and response body similar to the one produced by the ``view`` action. A valid JSON API document structure for updating the ``name`` field @@ -383,7 +383,7 @@ All requests to the ``delete`` action **must** use: - request data in valid JSON API document format - request data containing the ``id`` of the resource to delete -A succesful request will return HTTP response code ``204`` (No Content) +A successful request will return HTTP response code ``204`` (No Content) and empty response body. Failed requests will return HTTP response code ``400`` with empty response body. @@ -425,7 +425,7 @@ and a ``hasMany`` relationship with Cultures: return $this->Crud->execute(); } -Assuming a succesful find the listener would produce the +Assuming a successful find the listener would produce the following JSON API response including all associated data: .. code-block:: json @@ -505,6 +505,44 @@ following JSON API response including all associated data: ] } +The listener also supports the ``include`` parameter to allow clients to +customize related resources. Using that same example as above, the client +might request ``/countries/2?include=cultures,currencies`` to achieve the +same response. If the include parameter is provided, then only requested +relationships will be included in the ``included`` schema. + +It is possible blacklist, or whitelist what the client is allowed to include. +This is done using the listener configuration: + +.. code-block:: php + + public function view() + { + $this->Crud + ->listener('jsonApi') + ->setConfig('queryParameters.include.whitelist', ['cultures', 'cities']); + + return $this->Crud->execute(); + } + +Whitelisting will prevent all non-whitelisted associations from being +contained. Blacklisting will prevent any blacklisted associations from +being included. Blacklisting takes precedence of whitelisting (i.e +blacklisting and whitelisting the same association will prevent it from +being included). If you wish to prevent any associations, set the ``blacklist`` +config option to ``true``: + +.. code-block:: php + + public function view() + { + $this->Crud + ->listener('jsonApi') + ->setConfig('queryParameters.include.blacklist', true); + + return $this->Crud->execute(); + } + .. note:: Please note that only support for ``belongsTo`` and ``hasMany`` @@ -623,6 +661,11 @@ Please note that entity names: 'cultures' // hasMany relationship and thus plural ]); +.. note:: + +The value of the ``include`` configuration will be overwritten if the +the client uses the ``?include`` query parameter. + fieldSets ^^^^^^^^^ @@ -676,6 +719,21 @@ by a missing ``type`` node in the posted data would be: ] } +queryParameters +^^^^^^^^^^^^^^^ + +This **array** option allows you to specify query parameters to parse in your application. +Currently this listener supports the official ``include`` parameter. You can easily add your own +by specifying a callable. + +.. code-block:: phpinline + + $this->Crud->listener('jsonApi')->config('queryParameter.parent', [ + 'callable' => function ($queryData, $subject) { + $subject->query->where('parent' => $queryData); + } + ]); + Pagination ---------- From 83a5ded769ebe53b50584f26c6f2c9526466b874 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Tue, 4 Apr 2017 20:16:33 +0200 Subject: [PATCH 16/37] Support CakePHP < 3.4 --- src/Listener/JsonApiListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index d777a8e9f..f56e1390c 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -293,7 +293,7 @@ public function beforeFind(Event $event) throw new \InvalidArgumentException('Invalid callable supplied for query parameter ' . $parameter); } - $options['callable']($this->_request()->getQuery($parameter), $event->subject(), $options); + $options['callable']($this->_request()->query($parameter), $event->subject(), $options); } } From 3317cabeafdcc918a9596b32603ddbd8b751613f Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Tue, 4 Apr 2017 20:19:48 +0200 Subject: [PATCH 17/37] Fix array error --- src/Listener/JsonApiListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index f56e1390c..e1d2c1f23 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -252,7 +252,7 @@ protected function _includeParameter($includes, Subject $subject, $options) if (is_string($includes)) { $includes = explode(',', $includes); } - $includes = Hash::filter($includes); + $includes = Hash::filter((array)$includes); if (empty($includes) || $options['blacklist'] === true || $options['whitelist'] === false) { return; From 406fad4d19a1a859782cef716b963e3ed5b1a5d4 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Mon, 3 Apr 2017 15:32:46 +0200 Subject: [PATCH 18/37] Allow for deep relational links --- src/Listener/JsonApiListener.php | 26 +++++++++++-------- src/Schema/JsonApi/DynamicEntitySchema.php | 3 ++- .../TestCase/Listener/JsonApiListenerTest.php | 8 +++--- .../Schema/DynamicEntitySchemaTest.php | 4 +-- tests/TestCase/View/JsonApiViewTest.php | 2 +- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index e1d2c1f23..5154c19d7 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -327,7 +327,8 @@ protected function _insertBelongsToDataIntoEventFindResult($event) $associations = $repository->associations(); foreach ($associations as $association) { - if ($association->type() === Association::MANY_TO_ONE) { + $type = $association->type(); + if ($type === Association::MANY_TO_ONE || $type === Association::ONE_TO_ONE) { $associationTable = $association->target(); $foreignKey = $association->foreignKey(); @@ -434,8 +435,7 @@ protected function _renderWithResources($subject) $repository = $this->_controller()->loadModel(); // Default model class // Remove associations not found in the `find()` result - $entity = $this->_getSingleEntity($subject); - $strippedAssociations = $this->_stripNonContainedAssociations($repository, $subject->query); + $strippedAssociations = $this->_getContainedAssociations($repository, $subject->query->contain()); // Set data before rendering the view $this->_controller()->set([ @@ -580,19 +580,22 @@ protected function _getSingleEntity($subject) * prevent `null` entries appearing in the json api `relationships` node. * * @param \Cake\Datasource\RepositoryInterface $repository Repository - * @param \Cake\ORM\Query $query Query + * @param array $contains Array of contained associations * @return \Cake\ORM\AssociationCollection */ - protected function _stripNonContainedAssociations($repository, $query) + protected function _getContainedAssociations($repository, $contains) { - $associations = $repository->associations(); - $contains = $query->contain(); + $associationCollection = $repository->associations(); - foreach ($associations as $association) { + $associations = []; + foreach ((array)$contains as $contain => $nestedContains) { + $association = $associationCollection->get($contain); $associationKey = strtolower($association->name()); - if (!isset($contains[$association->name()])) { - $associations->remove($associationKey); + $associations += [$associationKey => $association]; + + if (!empty($nestedContains)) { + $associations += $this->_getContainedAssociations($association->target(), $nestedContains); } } @@ -639,7 +642,8 @@ protected function _getIncludeList($associations) $result = []; foreach ($associations as $association) { - if ($association->type() === Association::MANY_TO_ONE) { + $type = $association->type(); + if ($type === Association::MANY_TO_ONE || $type === Association::ONE_TO_ONE) { $include = Inflector::tableize($association->name()); $include = Inflector::singularize($include); diff --git a/src/Schema/JsonApi/DynamicEntitySchema.php b/src/Schema/JsonApi/DynamicEntitySchema.php index 5a1445298..201fcfa4f 100644 --- a/src/Schema/JsonApi/DynamicEntitySchema.php +++ b/src/Schema/JsonApi/DynamicEntitySchema.php @@ -94,7 +94,8 @@ public function getAttributes($entity) foreach ($this->_view->viewVars['_associations'] as $association) { $associationKey = Inflector::tableize($association->property()); - if ($association->type() === Association::MANY_TO_ONE) { + $type = $association->type(); + if ($type === Association::MANY_TO_ONE || $type === Association::ONE_TO_ONE) { $associationKey = Inflector::singularize($associationKey); } diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index 4447f6de4..e2fd0a7c5 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -1105,7 +1105,7 @@ public function testGetSingleEntity() * * @return void */ - public function testStripNonContainedAssociations() + public function testGetContainedAssociations() { $table = TableRegistry::get('Countries'); $table->belongsTo('Currencies'); @@ -1128,10 +1128,10 @@ public function testStripNonContainedAssociations() // make sure cultures are removed from AssociationCollection $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); - $associationsAfter = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $query], $listener); + $associationsAfter = $this->callProtectedMethod('_getContainedAssociations', [$table, $query->contain()], $listener); - $this->assertNotEmpty($associationsAfter->get('currencies')); - $this->assertNull($associationsAfter->get('cultures')); + $this->assertNotEmpty($associationsAfter['currencies']); + $this->assertArrayNotHasKey('cultures', $associationsAfter); } /** diff --git a/tests/TestCase/Schema/DynamicEntitySchemaTest.php b/tests/TestCase/Schema/DynamicEntitySchemaTest.php index c23ccf4f1..6b8b4fc21 100644 --- a/tests/TestCase/Schema/DynamicEntitySchemaTest.php +++ b/tests/TestCase/Schema/DynamicEntitySchemaTest.php @@ -61,7 +61,7 @@ public function testGetAttributes() // get required AssociationsCollection $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); - $associations = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $query], $listener); + $associations = $this->callProtectedMethod('_getContainedAssociations', [$table, $query->contain()], $listener); // make view return associations on get('_associations') call $view = $this @@ -130,7 +130,7 @@ public function testRelationships() // get required AssociationsCollection $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); - $associations = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $query], $listener); + $associations = $this->callProtectedMethod('_getContainedAssociations', [$table, $query->contain()], $listener); // make view return associations on get('_associations') call $view = $this diff --git a/tests/TestCase/View/JsonApiViewTest.php b/tests/TestCase/View/JsonApiViewTest.php index 62a069bd0..900f6ba66 100644 --- a/tests/TestCase/View/JsonApiViewTest.php +++ b/tests/TestCase/View/JsonApiViewTest.php @@ -155,7 +155,7 @@ protected function _getView($tableName, $viewVars) $this->setReflectionClassInstance($listener); $entity = $this->callProtectedMethod('_getSingleEntity', [$subject], $listener); - $associations = $this->callProtectedMethod('_stripNonContainedAssociations', [$table, $subject->query], $listener); + $associations = $this->callProtectedMethod('_getContainedAssociations', [$table, $subject->query->contain()], $listener); $repositories = $this->callProtectedMethod('_getRepositoryList', [$table, $associations], $listener); $viewVars['_repositories'] = $repositories; From fa51b947d64fd11e7a69a3ea8ea88430496f5ab5 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Wed, 5 Apr 2017 13:30:53 +0200 Subject: [PATCH 19/37] Working authentication --- src/Error/JsonApiExceptionRenderer.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Error/JsonApiExceptionRenderer.php b/src/Error/JsonApiExceptionRenderer.php index 0136e9efd..8747ac25f 100644 --- a/src/Error/JsonApiExceptionRenderer.php +++ b/src/Error/JsonApiExceptionRenderer.php @@ -16,7 +16,7 @@ * Licensed under The MIT License * For full copyright and license information, please see the LICENSE.txt */ -class JsonApiExceptionRenderer extends \Cake\Error\ExceptionRenderer +class JsonApiExceptionRenderer extends ExceptionRenderer { /** @@ -27,6 +27,10 @@ class JsonApiExceptionRenderer extends \Cake\Error\ExceptionRenderer */ protected function _outputMessage($template) { + if (!$this->controller->request->is('jsonapi')) { + return parent::_outputMessage($template); + } + $viewVars = $this->controller->viewVars; $code = $viewVars['code']; // e.g. 404 @@ -75,6 +79,10 @@ protected function _outputMessage($template) */ public function validation($exception) { + if (!$this->controller->request->is('jsonapi')) { + return parent::validation($exception); + } + $status = $exception->getCode(); try { From ece572ec96902d3dbeb783efb8ca6e5d7ee90796 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 6 Apr 2017 09:50:23 +0200 Subject: [PATCH 20/37] Add basic integration test for JsonApi --- tests/App/Controller/CountriesController.php | 10 +-- .../get_countries_with_pagination.json | 61 +++++++++++-------- .../Controller/JsonApiIntegrationTest.php | 26 +++++++- 3 files changed, 67 insertions(+), 30 deletions(-) diff --git a/tests/App/Controller/CountriesController.php b/tests/App/Controller/CountriesController.php index 906271a14..e27f307db 100644 --- a/tests/App/Controller/CountriesController.php +++ b/tests/App/Controller/CountriesController.php @@ -12,22 +12,22 @@ class CountriesController extends Controller use ControllerTrait; - public $paginate = [ - 'limit' => 3 - ]; + public $paginate = ['limit' => 3]; public $components = [ 'RequestHandler', + 'Flash', 'Crud.Crud' => [ 'actions' => [ 'Crud.Index', - 'Crud.View', 'Crud.Add', 'Crud.Edit', + 'Crud.View', 'Crud.Delete', ], 'listeners' => [ - 'Crud.JsonApi' + 'Crud.JsonApi', + 'Crud.ApiPagination', ] ] ]; diff --git a/tests/Fixture/JsonApi/get_countries_with_pagination.json b/tests/Fixture/JsonApi/get_countries_with_pagination.json index f9bd3314e..9d9767735 100644 --- a/tests/Fixture/JsonApi/get_countries_with_pagination.json +++ b/tests/Fixture/JsonApi/get_countries_with_pagination.json @@ -1,25 +1,38 @@ { - "meta": { - "record_count": 28, - "page_count": 3, - "page_limit": 10 - }, - "links": { - "self": "/countries?page=2", - "first": "/countries?page=1", - "last": "/countries?page=3", - "prev": "/countries?page=1", - "next": "/countries?page=3" - }, - "data": [ - { - "type": "countries", - "id": "1", - "attributes": { - "code": "NL", - "name": "The Netherlands", - "currency_id": 1 - } - } - ] -} \ No newline at end of file + "meta": { + "record_count": 2, + "page_count": 1, + "page_limit": null + }, + "links": { + "self": "/countries?page=1", + "first": "/countries?page=1", + "last": "/countries?page=1", + "prev": null, + "next": null + }, + "data": [ + { + "type": "countries", + "id": "1", + "attributes": { + "code": "NL", + "name": "The Netherlands" + }, + "links": { + "self": "\/countries\/view\/1" + } + }, + { + "type": "countries", + "id": "2", + "attributes": { + "code": "BE", + "name": "Belgium" + }, + "links": { + "self": "\/countries\/view\/2" + } + } + ] +} diff --git a/tests/TestCase/Controller/JsonApiIntegrationTest.php b/tests/TestCase/Controller/JsonApiIntegrationTest.php index 5722bb01b..349510dd0 100644 --- a/tests/TestCase/Controller/JsonApiIntegrationTest.php +++ b/tests/TestCase/Controller/JsonApiIntegrationTest.php @@ -9,9 +9,17 @@ class JsonApiIntegrationTest extends IntegrationTestCase public $fixtures = [ 'plugin.crud.countries', - 'plugin.crud.currencies' + 'plugin.crud.currencies', + 'plugin.crud.cultures', ]; + /** + * Path to directory holding json fixtures with trailing slash + * + * @var + */ + protected $_JsonDir; + /** * Set up required RESTful resource routes. */ @@ -24,6 +32,21 @@ public function setUp() }); } + protected function _getExpected($file) + { + return json_decode((new File($this->_JsonDir . $file))->read(), true); + } + + protected function _getResponseAsArray() + { + $this->_response->getBody() + ->rewind(); + $response = $this->_response->getBody() + ->getContents(); + + return json_decode($response, true); + } + /** * Test most basic `index` action * @@ -41,6 +64,7 @@ public function testGet() $this->_assertJsonApiResponse(); $this->assertResponseCode(200); $this->assertResponseNotEmpty(); + $this->assertSame($this->_getExpected('get_countries_with_pagination.json'), $this->_getResponseAsArray()); } /** From 7ddc779e0b08a003c2a7102b0bf06c4f7d835bc0 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 6 Apr 2017 13:26:15 +0200 Subject: [PATCH 21/37] Add a test case for viewing and including relationships --- src/Listener/JsonApiListener.php | 2 +- .../get_countries_with_pagination.json | 4 +- .../JsonApi/get_country_no_relationships.json | 5 +-- .../JsonApi/get_country_with_culture.json | 37 ++++++++++++++++ .../JsonApi/get_country_with_currency.json | 36 ++++++++++++++++ .../Controller/JsonApiIntegrationTest.php | 43 ++++++++++++++++++- 6 files changed, 120 insertions(+), 7 deletions(-) create mode 100644 tests/Fixture/JsonApi/get_country_with_culture.json create mode 100644 tests/Fixture/JsonApi/get_country_with_currency.json diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 5154c19d7..24c107aed 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -264,7 +264,7 @@ protected function _includeParameter($includes, Subject $subject, $options) $whitelist = is_array($options['whitelist']) ? Hash::expand(Hash::normalize(array_fill_keys($options['whitelist'], true))) : $options['whitelist']; $contains = $this->_parseIncludes($includes, $blacklist, $whitelist, $subject->query->repository()); - $subject->query->contain($contains); + $subject->query = $subject->query->contain($contains); } /** diff --git a/tests/Fixture/JsonApi/get_countries_with_pagination.json b/tests/Fixture/JsonApi/get_countries_with_pagination.json index 9d9767735..fed66964a 100644 --- a/tests/Fixture/JsonApi/get_countries_with_pagination.json +++ b/tests/Fixture/JsonApi/get_countries_with_pagination.json @@ -20,7 +20,7 @@ "name": "The Netherlands" }, "links": { - "self": "\/countries\/view\/1" + "self": "\/countries\/1" } }, { @@ -31,7 +31,7 @@ "name": "Belgium" }, "links": { - "self": "\/countries\/view\/2" + "self": "\/countries\/2" } } ] diff --git a/tests/Fixture/JsonApi/get_country_no_relationships.json b/tests/Fixture/JsonApi/get_country_no_relationships.json index e5de43ff5..298d39cb3 100644 --- a/tests/Fixture/JsonApi/get_country_no_relationships.json +++ b/tests/Fixture/JsonApi/get_country_no_relationships.json @@ -4,11 +4,10 @@ "id": "1", "attributes": { "code": "NL", - "name": "The Netherlands", - "currency_id": 1 + "name": "The Netherlands" }, "links": { "self": "\/countries\/1" } } -} \ No newline at end of file +} diff --git a/tests/Fixture/JsonApi/get_country_with_culture.json b/tests/Fixture/JsonApi/get_country_with_culture.json new file mode 100644 index 000000000..131aacc84 --- /dev/null +++ b/tests/Fixture/JsonApi/get_country_with_culture.json @@ -0,0 +1,37 @@ +{ + "data": { + "type": "countries", + "id": "1", + "attributes": { + "code": "NL", + "name": "The Netherlands" + }, + "relationships": { + "cultures": { + "data": [ + { + "type": "cultures", + "id": "1" + } + ], + "links": { + "self": "\/cultures?country_id=1" + } + } + }, + "links": { + "self": "\/countries\/1" + } + }, + "included": [ + { + "type": "cultures", + "id": "1", + "attributes": { + "code": "nl-NL", + "name": "Dutch", + "country_id": 1 + } + } + ] +} diff --git a/tests/Fixture/JsonApi/get_country_with_currency.json b/tests/Fixture/JsonApi/get_country_with_currency.json new file mode 100644 index 000000000..60a71b8ad --- /dev/null +++ b/tests/Fixture/JsonApi/get_country_with_currency.json @@ -0,0 +1,36 @@ +{ + "data": { + "type": "countries", + "id": "1", + "attributes": { + "code": "NL", + "name": "The Netherlands" + }, + "relationships": { + "currency": { + "data": [ + { + "type": "currencies", + "id": "1" + } + ], + "links": { + "self": "\/currencies\/1" + } + } + }, + "links": { + "self": "\/countries\/1" + } + }, + "included": [ + { + "type": "currencies", + "id": "1", + "attributes": { + "code": "EUR", + "name": "Euro" + } + } + ] +} diff --git a/tests/TestCase/Controller/JsonApiIntegrationTest.php b/tests/TestCase/Controller/JsonApiIntegrationTest.php index 349510dd0..6e7cc296f 100644 --- a/tests/TestCase/Controller/JsonApiIntegrationTest.php +++ b/tests/TestCase/Controller/JsonApiIntegrationTest.php @@ -1,8 +1,9 @@ 'dasherize' ]); }); + + // store path the the json fixtures + $this->_JsonDir = Plugin::path('Crud') . 'tests' . DS . 'Fixture' . DS . 'JsonApi' . DS; } protected function _getExpected($file) @@ -113,4 +117,41 @@ protected function _assertJsonApiResponse() $this->assertHeader('Content-Type', 'application/vnd.api+json'); $this->assertContentType('application/vnd.api+json'); } + + public function viewProvider() + { + return [ + 'no relations' => [ + '/countries/1', + 'get_country_no_relationships.json', + ], + 'include culture' => [ + '/countries/1?include=cultures', + 'get_country_with_culture.json' + ], + 'include currency' => [ + '/countries/1?include=currencies', + 'get_country_with_currency.json' + ], + ]; + } + + /** + * @param string $url The endpoint to hit + * @param string $expectedFile The file to find the expected result in + * @return void + * @dataProvider viewProvider + */ + public function testView($url, $expectedFile) + { + $this->configRequest([ + 'headers' => [ + 'Accept' => 'application/vnd.api+json' + ] + ]); + $this->get($url); + + $this->assertResponseSuccess(); + $this->assertSame($this->_getExpected($expectedFile), $this->_getResponseAsArray()); + } } From ba5ac292e3c47a419976dc42850d038b5e5265f1 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 6 Apr 2017 13:53:25 +0200 Subject: [PATCH 22/37] Nest the list of associations and includes so that deeply nested includes are possible --- src/Listener/JsonApiListener.php | 60 +++++++++++-------- .../TestCase/Listener/JsonApiListenerTest.php | 45 +++++++++++--- tests/TestCase/View/JsonApiViewTest.php | 1 - 3 files changed, 72 insertions(+), 34 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 24c107aed..d69c7404b 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -435,7 +435,7 @@ protected function _renderWithResources($subject) $repository = $this->_controller()->loadModel(); // Default model class // Remove associations not found in the `find()` result - $strippedAssociations = $this->_getContainedAssociations($repository, $subject->query->contain()); + $usedAssociations = $this->_getContainedAssociations($repository, $subject->query->contain()); // Set data before rendering the view $this->_controller()->set([ @@ -445,11 +445,11 @@ protected function _renderWithResources($subject) '_jsonApiBelongsToLinks' => $this->config('jsonApiBelongsToLinks'), '_jsonOptions' => $this->config('jsonOptions'), '_debugPrettyPrint' => $this->config('debugPrettyPrint'), - '_repositories' => $this->_getRepositoryList($repository, $strippedAssociations), - '_include' => $this->_getIncludeList($strippedAssociations), + '_repositories' => $this->_getRepositoryList($repository, $usedAssociations), + '_include' => $this->_getIncludeList($usedAssociations), '_fieldSets' => $this->config('fieldSets'), Inflector::tableize($repository->alias()) => $this->_getFindResult($subject), - '_associations' => $strippedAssociations, + '_associations' => $usedAssociations, '_serialize' => true, ]); @@ -575,9 +575,7 @@ protected function _getSingleEntity($subject) } /** - * Removes all associated models not detected (as the result of a contain - * query) in the find result from the entity's AssociationCollection to - * prevent `null` entries appearing in the json api `relationships` node. + * Creates a nested array of all associations used in the query * * @param \Cake\Datasource\RepositoryInterface $repository Repository * @param array $contains Array of contained associations @@ -592,10 +590,13 @@ protected function _getContainedAssociations($repository, $contains) $association = $associationCollection->get($contain); $associationKey = strtolower($association->name()); - $associations += [$associationKey => $association]; + $associations[$associationKey] = [ + 'association' => $association, + 'children' => [] + ]; if (!empty($nestedContains)) { - $associations += $this->_getContainedAssociations($association->target(), $nestedContains); + $associations[$associationKey]['children'] = $this->_getContainedAssociations($association->target(), $nestedContains); } } @@ -603,10 +604,10 @@ protected function _getContainedAssociations($repository, $contains) } /** - * Get a list of all repositories indexed by their registry alias. + * Get a flat list of all repositories indexed by their registry alias. * * @param RepositoryInterface $repository Current repository - * @param Association[] $associations Associations to get repository from + * @param array $associations Nested associations to get repository from * @return array Used repositories indexed by registry alias * @internal */ @@ -617,9 +618,18 @@ protected function _getRepositoryList(RepositoryInterface $repository, $associat ]; foreach ($associations as $association) { - $registryAlias = $association->target()->registryAlias(); + $association += [ + 'association' => null, + 'children' => [] + ]; - $repositories[$registryAlias] = $association->target(); + if ($association['association'] === null) { + throw new \InvalidArgumentException("Association {$name} does not have an association object set"); + } + + $associationRepository = $association['association']->target(); + + $repositories += $this->_getRepositoryList($associationRepository, $association['children'] ?: []); } return $repositories; @@ -633,29 +643,29 @@ protected function _getRepositoryList(RepositoryInterface $repository, $associat * * @param \Cake\ORM\AssociationCollection $associations AssociationCollection * @return array + * @throws \InvalidArgumentException */ - protected function _getIncludeList($associations) + protected function _getIncludeList($associations, $last = true) { if (!empty($this->config('include'))) { return $this->config('include'); } $result = []; - foreach ($associations as $association) { - $type = $association->type(); - if ($type === Association::MANY_TO_ONE || $type === Association::ONE_TO_ONE) { - $include = Inflector::tableize($association->name()); - $include = Inflector::singularize($include); - - $result[] = $include; - continue; + foreach ($associations as $name => $association) { + $association += [ + 'association' => null, + 'children' => [] + ]; + + if ($association['association'] === null) { + throw new \InvalidArgumentException("Association {$name} does not have an association object set"); } - // hasMany - $result[] = Inflector::tableize($association->name()); + $result[$association['association']->property()] = $this->_getIncludeList($association['children'], false); } - return $result; + return $last ? array_keys(Hash::flatten($result)) : $result; } /** diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index e2fd0a7c5..95083e2f9 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -1146,10 +1146,22 @@ public function testGetRepositoryList() $table->belongsTo('Currencies'); $table->hasMany('Cultures'); - $associations = $table->associations(); + $associations = []; + foreach ($table->associations() as $association) { + $associations[strtolower($association->name())] = [ + 'association' => $association, + 'children' => [] + ]; + } + + $associations['currencies']['children'] = [ + 'countries' => [ + 'association' => $table->Currencies->Countries, + ] + ]; - $this->assertNotEmpty($associations->get('currencies')); - $this->assertNotEmpty($associations->get('cultures')); + $this->assertArrayHasKey('currencies', $associations); + $this->assertArrayHasKey('cultures', $associations); $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); @@ -1184,11 +1196,22 @@ public function testGetIncludeList() $this->assertEmpty($listener->config('include')); $table = TableRegistry::get('Countries'); - $associations = $table->associations(); - $this->assertSame(['currencies', 'cultures'], $associations->keys()); + $associations = []; + foreach ($table->associations() as $association) { + $associations[strtolower($association->name())] = [ + 'association' => $association, + 'children' => [] + ]; + } + + $associations['currencies']['children'] = [ + 'countries' => [ + 'association' => $table->Currencies->Countries, + ] + ]; $expected = [ - 'currency', + 'currency.countries', 'cultures' ]; @@ -1196,10 +1219,16 @@ public function testGetIncludeList() $this->assertSame($expected, $result); + unset($associations['currencies']['children']['countries']); + $this->assertSame(['currencies', 'cultures'], array_keys($associations)); + $result = $this->callProtectedMethod('_getIncludeList', [$associations], $listener); + + $this->assertSame(['currency', 'cultures'], $result); + // assert the include list is still auto-generated if an association is // removed from the AssociationsCollection - $associations->remove('cultures'); - $this->assertSame(['currencies'], $associations->keys()); + unset($associations['cultures']); + $this->assertSame(['currencies'], array_keys($associations)); $result = $this->callProtectedMethod('_getIncludeList', [$associations], $listener); $this->assertSame(['currency'], $result); diff --git a/tests/TestCase/View/JsonApiViewTest.php b/tests/TestCase/View/JsonApiViewTest.php index 900f6ba66..2accb0c22 100644 --- a/tests/TestCase/View/JsonApiViewTest.php +++ b/tests/TestCase/View/JsonApiViewTest.php @@ -154,7 +154,6 @@ protected function _getView($tableName, $viewVars) ->getMock(); $this->setReflectionClassInstance($listener); - $entity = $this->callProtectedMethod('_getSingleEntity', [$subject], $listener); $associations = $this->callProtectedMethod('_getContainedAssociations', [$table, $subject->query->contain()], $listener); $repositories = $this->callProtectedMethod('_getRepositoryList', [$table, $associations], $listener); From f64bbc8f3d8cfb4f6c7a4d9683fdade049515b99 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 6 Apr 2017 14:11:20 +0200 Subject: [PATCH 23/37] Correctly parse the include config for include query --- src/Listener/JsonApiListener.php | 14 +++++++++++--- src/Schema/JsonApi/DynamicEntitySchema.php | 9 ++------- .../JsonApi/get_country_with_currency.json | 10 ++++------ .../Controller/JsonApiIntegrationTest.php | 16 ++++++++-------- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index d69c7404b..dd04c1530 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -192,7 +192,6 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo $wildcardWhitelist = Hash::get((array)$whitelist, $wildcard); $wildcardBlacklist = Hash::get((array)$blacklist, $wildcard); $contains = []; - $includeConfig = $this->config('include'); foreach ($includes as $include => $nestedIncludes) { $nestedContains = []; $includePath = array_merge($path, [$include]); @@ -228,11 +227,9 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo $contains[$associationName] = $nestedContains; } else { $contains[] = $associationName; - $includeConfig[] = $includeDotPath; } } - $this->config('include', $includeConfig); return $contains; } @@ -265,6 +262,12 @@ protected function _includeParameter($includes, Subject $subject, $options) $contains = $this->_parseIncludes($includes, $blacklist, $whitelist, $subject->query->repository()); $subject->query = $subject->query->contain($contains); + + $this->config('include', []); + $associations = $this->_getContainedAssociations($this->_controller()->loadModel(), $contains); + $include = $this->_getIncludeList($associations); + + $this->config('include', $include); } /** @@ -587,6 +590,11 @@ protected function _getContainedAssociations($repository, $contains) $associations = []; foreach ((array)$contains as $contain => $nestedContains) { + if (is_string($nestedContains)) { + $contain = $nestedContains; + $nestedContains = []; + } + $association = $associationCollection->get($contain); $associationKey = strtolower($association->name()); diff --git a/src/Schema/JsonApi/DynamicEntitySchema.php b/src/Schema/JsonApi/DynamicEntitySchema.php index 201fcfa4f..a51c35939 100644 --- a/src/Schema/JsonApi/DynamicEntitySchema.php +++ b/src/Schema/JsonApi/DynamicEntitySchema.php @@ -92,12 +92,7 @@ public function getAttributes($entity) // remove associated data so it won't appear inside jsonapi `attributes` foreach ($this->_view->viewVars['_associations'] as $association) { - $associationKey = Inflector::tableize($association->property()); - - $type = $association->type(); - if ($type === Association::MANY_TO_ONE || $type === Association::ONE_TO_ONE) { - $associationKey = Inflector::singularize($associationKey); - } + $associationKey = $association['association']->property(); unset($attributes[$associationKey]); } @@ -121,7 +116,7 @@ public function getRelationships($resource, $isPrimary, array $includeRelationsh $relations = []; foreach ($this->_view->viewVars['_associations'] as $association) { - $associationKey = $association->property(); + $associationKey = $association['association']->property(); $data = $resource->get($associationKey); if (!$data) { diff --git a/tests/Fixture/JsonApi/get_country_with_currency.json b/tests/Fixture/JsonApi/get_country_with_currency.json index 60a71b8ad..c8da961ea 100644 --- a/tests/Fixture/JsonApi/get_country_with_currency.json +++ b/tests/Fixture/JsonApi/get_country_with_currency.json @@ -8,12 +8,10 @@ }, "relationships": { "currency": { - "data": [ - { - "type": "currencies", - "id": "1" - } - ], + "data": { + "type": "currencies", + "id": "1" + }, "links": { "self": "\/currencies\/1" } diff --git a/tests/TestCase/Controller/JsonApiIntegrationTest.php b/tests/TestCase/Controller/JsonApiIntegrationTest.php index 6e7cc296f..9ec1f1532 100644 --- a/tests/TestCase/Controller/JsonApiIntegrationTest.php +++ b/tests/TestCase/Controller/JsonApiIntegrationTest.php @@ -121,14 +121,14 @@ protected function _assertJsonApiResponse() public function viewProvider() { return [ - 'no relations' => [ - '/countries/1', - 'get_country_no_relationships.json', - ], - 'include culture' => [ - '/countries/1?include=cultures', - 'get_country_with_culture.json' - ], +// 'no relations' => [ +// '/countries/1', +// 'get_country_no_relationships.json', +// ], +// 'include culture' => [ +// '/countries/1?include=cultures', +// 'get_country_with_culture.json' +// ], 'include currency' => [ '/countries/1?include=currencies', 'get_country_with_currency.json' From 3f6cc10b6e4a040bf4b094edfdc06ecc6c166a32 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 6 Apr 2017 14:33:33 +0200 Subject: [PATCH 24/37] Add more integration tests, and check for singular include --- src/Listener/JsonApiListener.php | 4 ++ ...get_country_with_currency_and_culture.json | 54 +++++++++++++++++++ .../Controller/JsonApiIntegrationTest.php | 26 +++++---- 3 files changed, 75 insertions(+), 9 deletions(-) create mode 100644 tests/Fixture/JsonApi/get_country_with_currency_and_culture.json diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index dd04c1530..90f2e6900 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -212,6 +212,10 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo $association = null; $associationName = Inflector::camelize($include); + if ($this->_stringIsSingular($include)) { + $associationName = Inflector::pluralize($associationName); + } + if ($repository !== null) { $association = $repository->association($associationName); if ($association === null) { diff --git a/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json b/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json new file mode 100644 index 000000000..ba3f03fad --- /dev/null +++ b/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json @@ -0,0 +1,54 @@ +{ + "data": { + "type": "countries", + "id": "1", + "attributes": { + "code": "NL", + "name": "The Netherlands" + }, + "relationships": { + "currency": { + "data": { + "type": "currencies", + "id": "1" + }, + "links": { + "self": "\/currencies\/1" + } + }, + "cultures": { + "data": [ + { + "type": "cultures", + "id": "1" + } + ], + "links": { + "self": "\/cultures?country_id=1" + } + } + }, + "links": { + "self": "\/countries\/1" + } + }, + "included": [ + { + "type": "currencies", + "id": "1", + "attributes": { + "code": "EUR", + "name": "Euro" + } + }, + { + "type": "cultures", + "id": "1", + "attributes": { + "code": "nl-NL", + "name": "Dutch", + "country_id": 1 + } + } + ] +} diff --git a/tests/TestCase/Controller/JsonApiIntegrationTest.php b/tests/TestCase/Controller/JsonApiIntegrationTest.php index 9ec1f1532..6afe3d005 100644 --- a/tests/TestCase/Controller/JsonApiIntegrationTest.php +++ b/tests/TestCase/Controller/JsonApiIntegrationTest.php @@ -121,18 +121,26 @@ protected function _assertJsonApiResponse() public function viewProvider() { return [ -// 'no relations' => [ -// '/countries/1', -// 'get_country_no_relationships.json', -// ], -// 'include culture' => [ -// '/countries/1?include=cultures', -// 'get_country_with_culture.json' -// ], - 'include currency' => [ + 'no relations' => [ + '/countries/1', + 'get_country_no_relationships.json', + ], + 'include culture' => [ + '/countries/1?include=cultures', + 'get_country_with_culture.json' + ], + 'include currency plural' => [ '/countries/1?include=currencies', 'get_country_with_currency.json' ], + 'include currency singular' => [ + '/countries/1?include=currency', + 'get_country_with_currency.json' + ], + 'include currency and culture' => [ + '/countries/1?include=currencies,cultures', + 'get_country_with_currency_and_culture.json' + ], ]; } From cf9f3a2bd36a4728783b0213a56a4f3ad3a298b2 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 6 Apr 2017 15:17:24 +0200 Subject: [PATCH 25/37] Make exception better and add test for contained --- src/Listener/JsonApiListener.php | 2 +- .../Controller/JsonApiIntegrationTest.php | 54 +++++++++++++++---- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 90f2e6900..46ac82d86 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -219,7 +219,7 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo if ($repository !== null) { $association = $repository->association($associationName); if ($association === null) { - throw new BadRequestException('Invalid relationship path supplied in include parameter'); + throw new BadRequestException("Invalid relationship path '{$includeDotPath}' supplied in include parameter"); } } diff --git a/tests/TestCase/Controller/JsonApiIntegrationTest.php b/tests/TestCase/Controller/JsonApiIntegrationTest.php index 6afe3d005..b2548f960 100644 --- a/tests/TestCase/Controller/JsonApiIntegrationTest.php +++ b/tests/TestCase/Controller/JsonApiIntegrationTest.php @@ -2,12 +2,20 @@ namespace Crud\TestCase\Controller; use Cake\Core\Plugin; +use Cake\Event\Event; +use Cake\Event\EventManager; +use Cake\Filesystem\File; use Cake\Routing\Router; use Cake\TestSuite\IntegrationTestCase; +use Crud\Error\JsonApiExceptionRenderer; class JsonApiIntegrationTest extends IntegrationTestCase { - + /** + * fixtures property + * + * @var array + */ public $fixtures = [ 'plugin.crud.countries', 'plugin.crud.currencies', @@ -32,6 +40,12 @@ public function setUp() ]); }); + $this->configRequest([ + 'headers' => [ + 'Accept' => 'application/vnd.api+json' + ] + ]); + // store path the the json fixtures $this->_JsonDir = Plugin::path('Crud') . 'tests' . DS . 'Fixture' . DS . 'JsonApi' . DS; } @@ -58,10 +72,6 @@ protected function _getResponseAsArray() */ public function testGet() { - $this->configRequest([ - 'headers' => ['Accept' => 'application/vnd.api+json'] - ]); - $this->get('/countries'); $this->assertResponseOk(); @@ -152,14 +162,38 @@ public function viewProvider() */ public function testView($url, $expectedFile) { - $this->configRequest([ - 'headers' => [ - 'Accept' => 'application/vnd.api+json' - ] - ]); $this->get($url); $this->assertResponseSuccess(); $this->assertSame($this->_getExpected($expectedFile), $this->_getResponseAsArray()); } + + /** + * @return void + */ + public function testViewWithContain() + { + EventManager::instance() + ->on('Crud.beforeFind', function (Event $event) { + $event->subject()->query->contain([ + 'Currencies', + 'Cultures', + ]); + }); + $this->get('/countries/1'); + + $this->assertResponseSuccess(); + $this->assertSame($this->_getExpected('get_country_with_currency_and_culture.json'), $this->_getResponseAsArray()); + } + + /** + * @return void + */ + public function testViewInvalidInclude() + { + Configure::write('Error.exceptionRenderer', JsonApiExceptionRenderer::class); + $this->get('/countries/1?include=donkey'); + + $this->assertResponseError(); + } } From c125291831bf89569939c77bd60fe7726df5e7af Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 6 Apr 2017 15:33:02 +0200 Subject: [PATCH 26/37] Take into account non query requests (Like post) --- src/Listener/JsonApiListener.php | 35 +++++++++++++++++-- .../Controller/JsonApiIntegrationTest.php | 7 +++- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 46ac82d86..f144f220d 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -441,8 +441,12 @@ protected function _renderWithResources($subject) { $repository = $this->_controller()->loadModel(); // Default model class - // Remove associations not found in the `find()` result - $usedAssociations = $this->_getContainedAssociations($repository, $subject->query->contain()); + if (isset($subject->query)) { + $usedAssociations = $this->_getContainedAssociations($repository, $subject->query->contain()); + } else { + $entity = $this->_getSingleEntity($subject); + $usedAssociations = $this->_extractEntityAssociations($repository, $entity); + } // Set data before rendering the view $this->_controller()->set([ @@ -615,6 +619,33 @@ protected function _getContainedAssociations($repository, $contains) return $associations; } + /** + * Removes all associated models not detected (as the result of a contain + * query) in the find result from the entity's AssociationCollection to + * prevent `null` entries appearing in the json api `relationships` node. + * + * @param \Cake\Datasource\RepositoryInterface $repository Repository + * @param \Cake\ORM\Entity $entity Entity + * @return array + */ + protected function _extractEntityAssociations($repository, $entity) + { + $associationCollection = $repository->associations(); + $associations = []; + foreach ($associationCollection as $association) { + $associationKey = strtolower($association->name()); + $entityKey = $association->property(); + if (!empty($entity->$entityKey)) { + $associations[$associationKey] = [ + 'association' => $association, + 'children' => $this->_extractEntityAssociations($association->target(), $entity->$entityKey) + ]; + } + } + + return $associations; + } + /** * Get a flat list of all repositories indexed by their registry alias. * diff --git a/tests/TestCase/Controller/JsonApiIntegrationTest.php b/tests/TestCase/Controller/JsonApiIntegrationTest.php index b2548f960..b4f790288 100644 --- a/tests/TestCase/Controller/JsonApiIntegrationTest.php +++ b/tests/TestCase/Controller/JsonApiIntegrationTest.php @@ -1,6 +1,7 @@ resources('Countries', [ 'inflect' => 'dasherize' @@ -128,6 +131,9 @@ protected function _assertJsonApiResponse() $this->assertContentType('application/vnd.api+json'); } + /** + * @return array + */ public function viewProvider() { return [ @@ -191,7 +197,6 @@ public function testViewWithContain() */ public function testViewInvalidInclude() { - Configure::write('Error.exceptionRenderer', JsonApiExceptionRenderer::class); $this->get('/countries/1?include=donkey'); $this->assertResponseError(); From 9aebb9d73f66e088c27e364c44935889707c93fa Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 6 Apr 2017 15:34:01 +0200 Subject: [PATCH 27/37] Add more routes --- tests/TestCase/Controller/JsonApiIntegrationTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/TestCase/Controller/JsonApiIntegrationTest.php b/tests/TestCase/Controller/JsonApiIntegrationTest.php index b4f790288..f4eecda46 100644 --- a/tests/TestCase/Controller/JsonApiIntegrationTest.php +++ b/tests/TestCase/Controller/JsonApiIntegrationTest.php @@ -41,6 +41,12 @@ public function setUp() $routes->resources('Countries', [ 'inflect' => 'dasherize' ]); + $routes->resources('Currencies', [ + 'inflect' => 'dasherize' + ]); + $routes->resources('Cultures', [ + 'inflect' => 'dasherize' + ]); }); $this->configRequest([ From 6a44020fbaf4a947c408c1f51cacbed54e15861f Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Thu, 6 Apr 2017 19:11:30 +0200 Subject: [PATCH 28/37] Only render json api error if it's a json api request --- src/Error/JsonApiExceptionRenderer.php | 4 ++-- .../Error/JsonApiExceptionRendererTest.php | 18 +++++++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/Error/JsonApiExceptionRenderer.php b/src/Error/JsonApiExceptionRenderer.php index 8747ac25f..9cd7a67f4 100644 --- a/src/Error/JsonApiExceptionRenderer.php +++ b/src/Error/JsonApiExceptionRenderer.php @@ -27,7 +27,7 @@ class JsonApiExceptionRenderer extends ExceptionRenderer */ protected function _outputMessage($template) { - if (!$this->controller->request->is('jsonapi')) { + if (!$this->controller->request->accepts('application/vnd.api+json')) { return parent::_outputMessage($template); } @@ -79,7 +79,7 @@ protected function _outputMessage($template) */ public function validation($exception) { - if (!$this->controller->request->is('jsonapi')) { + if (!$this->controller->request->accepts('application/vnd.api+json')) { return parent::validation($exception); } diff --git a/tests/TestCase/Error/JsonApiExceptionRendererTest.php b/tests/TestCase/Error/JsonApiExceptionRendererTest.php index 1a120ce9d..17935d27b 100644 --- a/tests/TestCase/Error/JsonApiExceptionRendererTest.php +++ b/tests/TestCase/Error/JsonApiExceptionRendererTest.php @@ -45,7 +45,11 @@ public function testRenderWithNonValidationError() $controller = $this->getMockBuilder('Cake\Controller\Controller') ->setMethods(['render']) ->getMock(); - $controller->request = new Request(); + $controller->request = new Request([ + 'environment' => [ + 'HTTP_ACCEPT' => 'application/vnd.api+json' + ] + ]); $controller->response = new Response(); $renderer = $this->getMockBuilder('Crud\Error\JsonApiExceptionRenderer') @@ -96,7 +100,11 @@ public function testRenderWithValidationError() $controller = $this->getMockBuilder('Cake\Controller\Controller') ->setMethods(['render']) ->getMock(); - $controller->request = new Request(); + $controller->request = new Request([ + 'environment' => [ + 'HTTP_ACCEPT' => 'application/vnd.api+json' + ] + ]); $controller->response = new Response(); $renderer = $this->getMockBuilder('Crud\Error\JsonApiExceptionRenderer') @@ -140,7 +148,11 @@ public function testValidationExceptionsFallBackToStatusCode422() ->setMethods(['render']) ->getMock(); - $controller->request = new Request(); + $controller->request = new Request([ + 'environment' => [ + 'HTTP_ACCEPT' => 'application/vnd.api+json' + ] + ]); $response = $this->getMockBuilder('Cake\Network\Response') ->setMethods(['statusCode']) From c4726e2cb7a9765a9bec8d72d2e562f7420fc55d Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Fri, 7 Apr 2017 11:09:02 +0200 Subject: [PATCH 29/37] Update json api listener tests --- src/Listener/JsonApiListener.php | 8 +- .../TestCase/Listener/JsonApiListenerTest.php | 110 ++++++++++-------- 2 files changed, 67 insertions(+), 51 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index f144f220d..46da399ae 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -265,10 +265,10 @@ protected function _includeParameter($includes, Subject $subject, $options) $whitelist = is_array($options['whitelist']) ? Hash::expand(Hash::normalize(array_fill_keys($options['whitelist'], true))) : $options['whitelist']; $contains = $this->_parseIncludes($includes, $blacklist, $whitelist, $subject->query->repository()); - $subject->query = $subject->query->contain($contains); + $subject->query->contain($contains); $this->config('include', []); - $associations = $this->_getContainedAssociations($this->_controller()->loadModel(), $contains); + $associations = $this->_getContainedAssociations($subject->query->repository(), $contains); $include = $this->_getIncludeList($associations); $this->config('include', $include); @@ -604,6 +604,10 @@ protected function _getContainedAssociations($repository, $contains) } $association = $associationCollection->get($contain); + if ($association === null) { + continue; + } + $associationKey = strtolower($association->name()); $associations[$associationKey] = [ diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index 95083e2f9..d68f507da 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -1406,103 +1406,111 @@ public function includeQueryProvider() { return [ 'standard' => [ - 'articles,comments.author', + 'cultures,currencies.countries', ['blacklist' => false, 'whitelist' => true], [ - 'Articles', - 'Comments' => ['Author'] + 'Cultures', + 'Currencies' => ['Countries'] ], [ - 'comments.author', 'articles', + 'cultures', 'currency.countries', + ], + ], + 'singular name' => [ + 'cultures,currency', + ['blacklist' => false, 'whitelist' => true], + [ + 'Cultures', + 'Currencies' + ], + [ + 'cultures', + 'currency', ], ], 'blacklist' => [ - 'articles,comments.author,comments.images', - ['blacklist' => ['comments.author'], 'whitelist' => true], + 'cultures,currencies.countries', + ['blacklist' => ['currencies.countries'], 'whitelist' => true], [ - 'Articles', - 'Comments' => [ - 'Images' - ] + 'Cultures', + 'Currencies' ], [ - 'comments.images', - 'articles', + 'cultures', + 'currency', ], ], 'whitelist' => [ - 'articles,comments.author,comments.images', - ['blacklist' => false, 'whitelist' => ['comments']], + 'cultures,currencies.countries', + ['blacklist' => false, 'whitelist' => ['cultures']], [ - 'Comments' + 'Cultures' ], [ - 'comments' + 'cultures' ], ], 'multiple whitelists' => [ - 'articles,comments.author,comments.images,comments.links', - ['blacklist' => false, 'whitelist' => ['comments.author', 'comments.images']], + 'cultures,currencies.countries,cultures.language', + ['blacklist' => false, 'whitelist' => ['cultures', 'currencies.countries']], [ - 'Comments' => [ - 'Author', - 'Images' + 'Cultures', + 'Currencies' => [ + 'Countries', ] ], [ - 'comments.author', - 'comments.images' + 'cultures', + 'currency.countries' ], ], 'whitelist wildcard' => [ - 'articles,comments.author,comments.images', - ['blacklist' => false, 'whitelist' => ['comments.*']], + 'cultures,currencies.countries,cultures.language', + ['blacklist' => false, 'whitelist' => ['currencies.*']], [ - 'Comments' => [ - 'Author', - 'Images' + 'Currencies' => [ + 'Countries' ] ], - ['comments.author', 'comments.images'], + ['currency.countries'], ], 'blacklist wildcard' => [ - 'articles,comments.author,comments.images', - ['blacklist' => ['comments.*'], 'whitelist' => true], + 'cultures,currencies.countries,currencies.names', + ['blacklist' => ['currencies.*'], 'whitelist' => true], [ - 'Articles', - 'Comments', + 'Cultures', + 'Currencies', ], - ['articles', 'comments',] + ['cultures', 'currency',] ], 'blacklist with a whitelist wildcard' => [ - 'articles,comments.author,comments.images,comments.links', - ['blacklist' => ['comments.author'], 'whitelist' => ['articles', 'comments.*']], + 'cultures,currencies.countries,currencies.names,cultures.countries', + ['blacklist' => ['currencies.names'], 'whitelist' => ['cultures', 'currencies.*']], [ - 'Comments' => [ - 'Images', - 'Links' + 'Currencies' => [ + 'Countries', ], - 'Articles', + 'Cultures', ], - ['comments.images', 'comments.links', 'articles'] + ['cultures', 'currency.countries'] ], 'blacklist is more important' => [ - 'articles,comments.author', - ['blacklist' => ['comments.author'], 'whitelist' => ['articles', 'comments.author']], + 'cultures,currencies.countries', + ['blacklist' => ['currencies.countries'], 'whitelist' => ['cultures', 'currencies.countries']], [ - 'Articles', - 'Comments', + 'Cultures', + 'Currencies', ], - ['articles', 'comments'] + ['cultures', 'currency'] ], 'blacklist everything' => [ - 'articles,comments.author', - ['blacklist' => true, 'whitelist' => ['articles', 'comments.author']], + 'cultures,currencies.countries', + ['blacklist' => true, 'whitelist' => ['cultures', 'currencies.countries']], [], [] ], 'whitelist nothing' => [ - 'articles,comments.author', + 'cultures,currencies.countries', ['blacklist' => false, 'whitelist' => false], [], [] @@ -1527,6 +1535,10 @@ public function testIncludeQuery($include, $options, $expectedContain, $expected ->expects($options['blacklist'] !== true && $options['whitelist'] !== false ? $this->once() : $this->never()) ->method('contain') ->with($expectedContain); + $subject->query + ->expects($this->any()) + ->method('repository') + ->willReturn(TableRegistry::get('Countries')); $this->callProtectedMethod('_includeParameter', [$include, $subject, $options], $listener); $this->assertSame($expectedInclude, $listener->config('include')); From 60faaf0cfedada8fb9049e46f5a8afe4c3941015 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Fri, 7 Apr 2017 12:05:22 +0200 Subject: [PATCH 30/37] Move responsibility of removing of relationship keys to the schema. As a consequence this should also make nested relationships work --- src/Listener/JsonApiListener.php | 50 ------ src/Schema/JsonApi/DynamicEntitySchema.php | 45 ++++-- .../get_countries_no_relationships.json | 8 +- .../JsonApi/get_country_with_culture.json | 3 +- ...get_country_with_currency_and_culture.json | 3 +- .../TestCase/Listener/JsonApiListenerTest.php | 145 ------------------ .../Schema/DynamicEntitySchemaTest.php | 6 +- tests/TestCase/View/JsonApiViewTest.php | 10 +- 8 files changed, 50 insertions(+), 220 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 46da399ae..a6f002629 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -304,19 +304,6 @@ public function beforeFind(Event $event) } } - /** - * respond() event. - * - * @param \Cake\Event\Event $event Event - * @return void - */ - public function respond(Event $event) - { - $this->_removeBelongsToForeignKeysFromEventData($event); - - parent::respond($event); - } - /** * Adds belongsTo data to the find() result so the 201 success response * is able to render the jsonapi `relationships` member. @@ -355,43 +342,6 @@ protected function _insertBelongsToDataIntoEventFindResult($event) $event->subject()->entity = $entity; } - /** - * Removes all belongsTo `_id` fields from the entity or entities so - * they don't show up as jsonapi attributes in the response as described - * at http://jsonapi.org/format/#document-resource-object-attributes. - * - * @param \Cake\Event\Event $event Event - * @return void - */ - protected function _removeBelongsToForeignKeysFromEventData($event) - { - $repository = $this->_controller()->loadModel(); - $associations = $repository->associations(); - - $foreignKeys = []; - foreach ($associations as $association) { - if ($association->type() === Association::MANY_TO_ONE) { - $foreignKeys[] = $association->foreignKey(); - } - } - - // remove from single entity - if (isset($event->subject()->entity)) { - foreach ($foreignKeys as $foreignKey) { - $event->subject()->entity->unsetProperty($foreignKey); - } - - return; - } - - // remove from collection - foreach ($event->subject()->entities as $key => $entity) { - foreach ($foreignKeys as $foreignKey) { - $event->subject()->entities->current()->unsetProperty($foreignKey); - } - } - } - /** * Set required viewVars before rendering the JsonApiView. * diff --git a/src/Schema/JsonApi/DynamicEntitySchema.php b/src/Schema/JsonApi/DynamicEntitySchema.php index a51c35939..841fb2755 100644 --- a/src/Schema/JsonApi/DynamicEntitySchema.php +++ b/src/Schema/JsonApi/DynamicEntitySchema.php @@ -1,6 +1,7 @@ get($this->_repository->primaryKey()); } + /** + * @param \Cake\Datasource\EntityInterface $entity + * @return \Cake\Datasource\RepositoryInterface $repository + */ + protected function _getRepository($entity) + { + $repositoryName = $entity->source(); + return isset($this->_view->viewVars['_repositories'][$repositoryName]) ? $this->_view->viewVars['_repositories'][$repositoryName] : null; + } + /** * NeoMerx override used to pass entity root properties to be shown * as JsonApi `attributes`. @@ -88,13 +99,23 @@ public function getAttributes($entity) $entity->hiddenProperties($hidden); } + $repository = $this->_getRepository($entity); $attributes = $entity->toArray(); + if (!$repository) { + return $attributes; + } + // remove associated data so it won't appear inside jsonapi `attributes` - foreach ($this->_view->viewVars['_associations'] as $association) { - $associationKey = $association['association']->property(); + foreach ($repository->associations() as $association) { + $propertyName = $association->property(); + + if ($association->type() === Association::MANY_TO_ONE) { + $foreignKey = $association->foreignKey(); + unset($attributes[$foreignKey]); + } - unset($attributes[$associationKey]); + unset($attributes[$propertyName]); } return $attributes; @@ -106,24 +127,30 @@ public function getAttributes($entity) * * JSON API optional `related` links not implemented yet. * - * @param \Cake\Datasource\EntityInterface $resource Entity object + * @param \Cake\Datasource\EntityInterface $entity Entity object * @param bool $isPrimary True to add resource to data section instead of included * @param array $includeRelationships Used to fine tune relationships * @return array */ - public function getRelationships($resource, $isPrimary, array $includeRelationships) + public function getRelationships($entity, $isPrimary, array $includeRelationships) { $relations = []; - foreach ($this->_view->viewVars['_associations'] as $association) { - $associationKey = $association['association']->property(); + $repository = $this->_getRepository($entity); + + if (!$repository) { + return $relations; + } + + foreach ($repository->associations() as $association) { + $property = $association->property(); - $data = $resource->get($associationKey); + $data = $entity->get($property); if (!$data) { continue; } - $relations[$associationKey] = [ + $relations[$property] = [ self::DATA => $data, self::SHOW_SELF => true, self::SHOW_RELATED => false, diff --git a/tests/Fixture/JsonApi/get_countries_no_relationships.json b/tests/Fixture/JsonApi/get_countries_no_relationships.json index fb2bb21e5..55dc7a8ad 100644 --- a/tests/Fixture/JsonApi/get_countries_no_relationships.json +++ b/tests/Fixture/JsonApi/get_countries_no_relationships.json @@ -5,8 +5,7 @@ "id": "1", "attributes": { "code": "NL", - "name": "The Netherlands", - "currency_id": 1 + "name": "The Netherlands" }, "links": { "self": "\/countries\/1" @@ -17,12 +16,11 @@ "id": "2", "attributes": { "code": "BE", - "name": "Belgium", - "currency_id": 1 + "name": "Belgium" }, "links": { "self": "\/countries\/2" } } ] -} \ No newline at end of file +} diff --git a/tests/Fixture/JsonApi/get_country_with_culture.json b/tests/Fixture/JsonApi/get_country_with_culture.json index 131aacc84..64bc18c3a 100644 --- a/tests/Fixture/JsonApi/get_country_with_culture.json +++ b/tests/Fixture/JsonApi/get_country_with_culture.json @@ -29,8 +29,7 @@ "id": "1", "attributes": { "code": "nl-NL", - "name": "Dutch", - "country_id": 1 + "name": "Dutch" } } ] diff --git a/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json b/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json index ba3f03fad..9cdd4d81d 100644 --- a/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json +++ b/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json @@ -46,8 +46,7 @@ "id": "1", "attributes": { "code": "nl-NL", - "name": "Dutch", - "country_id": 1 + "name": "Dutch" } } ] diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index d68f507da..003745c87 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -187,75 +187,6 @@ public function testBeforeHandle() $listener->beforeHandle(new Event('Crud.beforeHandle')); } - /** - * respond() - * - * @return void - */ - public function testRespond() - { - $controller = $this - ->getMockBuilder('\Cake\Controller\Controller') - ->setMethods(null) - ->enableOriginalConstructor() - ->getMock(); - - $controller->name = 'Countries'; - - $action = $this - ->getMockBuilder('\Crud\Action\IndexAction') - ->disableOriginalConstructor() - ->setMethods(['config']) - ->getMock(); - $response = $this - ->getMockBuilder('\Cake\Network\Response') - ->setMethods(['statusCode']) - ->getMock(); - - $subject = $this - ->getMockBuilder('\Crud\Event\Subject') - ->getMock(); - $subject->success = true; - - $table = TableRegistry::get('Countries'); - $entity = $table->find()->first(); - $subject->entity = $entity; - - $event = new Event('Crud.afterSave', $subject); - - $listener = $this - ->getMockBuilder('\Crud\Listener\JsonApiListener') - ->disableOriginalConstructor() - ->setMethods(['_controller', '_action', 'render']) - ->getMock(); - $listener - ->expects($this->next($listener)) - ->method('_controller') - ->with() - ->will($this->returnValue($controller)); - $listener - ->expects($this->next($listener)) - ->method('_action') - ->with() - ->will($this->returnValue($action)); - $action - ->expects($this->next($action)) - ->method('config') - ->with('api.success') - ->will($this->returnValue(['code' => 200])); - $listener - ->expects($this->next($listener)) - ->method('render') - ->with($subject) - ->will($this->returnValue($response)); - $response - ->expects($this->next($response)) - ->method('statusCode') - ->with(200); - - $listener->respond($event); - } - /** * Test afterSave event. * @@ -476,82 +407,6 @@ public function testInsertBelongsToDataIntoEventFindResult() $this->assertArrayHasKey('currency', $subject->entity); } - /** - * _removeForeignKeysFromEventData() - * - * @return void - */ - public function testRemoveForeignKeysFromEventData() - { - $controller = $this - ->getMockBuilder('\Cake\Controller\Controller') - ->setMethods(null) - ->setConstructorArgs([null, null, 'Countries']) - ->enableOriginalConstructor() - ->getMock(); - - $event = $this - ->getMockBuilder('\Cake\Event\Event') - ->disableOriginalConstructor() - ->setMethods(['subject']) - ->getMock(); - - $subject = $this - ->getMockBuilder('\Crud\Event\Subject') - ->disableOriginalConstructor() - ->setMethods(null) - ->getMock(); - - $event - ->expects($this->any()) - ->method('subject') - ->will($this->returnValue($subject)); - - $listener = $this - ->getMockBuilder('\Crud\Listener\JsonApiListener') - ->disableOriginalConstructor() - ->setMethods(['_controller']) - ->getMock(); - - $listener - ->expects($this->any()) - ->method('_controller') - ->will($this->returnValue($controller)); - - $this->setReflectionClassInstance($listener); - - // assert foreign keys are removed from single entity - $table = TableRegistry::get('Countries'); - $entity = $table->find()->first(); - $this->assertArrayHasKey('name', $entity); - $this->assertArrayHasKey('currency_id', $entity); - - $subject->entity = $entity; - - $this->callProtectedMethod('_removeBelongsToForeignKeysFromEventData', [$event], $listener); - - $this->assertArrayHasKey('name', $subject->entity); - $this->assertArrayNotHasKey('currency_id', $subject->entity); - - unset($subject->entity); - - // assert foreign keys are removed from entity collections - $entities = $table->find()->all(); - foreach ($entities as $entity) { - $this->assertArrayHasKey('name', $entity); - $this->assertArrayHasKey('currency_id', $entity); - } - - $subject->entities = $entities; - - $this->callProtectedMethod('_removeBelongsToForeignKeysFromEventData', [$event], $listener); - - foreach ($subject->entities as $entity) { - $this->assertArrayHasKey('name', $entity); - $this->assertArrayNotHasKey('currency_id', $entity); - } - } - /** * Make sure render() works with find data * diff --git a/tests/TestCase/Schema/DynamicEntitySchemaTest.php b/tests/TestCase/Schema/DynamicEntitySchemaTest.php index 6b8b4fc21..0dfc950b5 100644 --- a/tests/TestCase/Schema/DynamicEntitySchemaTest.php +++ b/tests/TestCase/Schema/DynamicEntitySchemaTest.php @@ -62,6 +62,7 @@ public function testGetAttributes() $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); $associations = $this->callProtectedMethod('_getContainedAssociations', [$table, $query->contain()], $listener); + $repositories = $this->callProtectedMethod('_getRepositoryList', [$table, $associations], $listener); // make view return associations on get('_associations') call $view = $this @@ -70,7 +71,7 @@ public function testGetAttributes() ->disableOriginalConstructor() ->getMock(); - $view->set('_associations', $associations); + $view->set('_repositories', $repositories); // setup the schema $schema = $this @@ -131,6 +132,7 @@ public function testRelationships() $listener = new JsonApiListener(new Controller()); $this->setReflectionClassInstance($listener); $associations = $this->callProtectedMethod('_getContainedAssociations', [$table, $query->contain()], $listener); + $repositories = $this->callProtectedMethod('_getRepositoryList', [$table, $associations], $listener); // make view return associations on get('_associations') call $view = $this @@ -139,7 +141,7 @@ public function testRelationships() ->disableOriginalConstructor() ->getMock(); - $view->set('_associations', $associations); + $view->set('_repositories', $repositories); $view->set('_absoluteLinks', false); // test relative links (listener default) // setup the schema diff --git a/tests/TestCase/View/JsonApiViewTest.php b/tests/TestCase/View/JsonApiViewTest.php index 2accb0c22..b92d12461 100644 --- a/tests/TestCase/View/JsonApiViewTest.php +++ b/tests/TestCase/View/JsonApiViewTest.php @@ -205,7 +205,7 @@ public function testEncodeWithDynamicSchemas() ]); $this->assertSame( - (new File($this->_JsonDir . 'get_countries_no_relationships.json'))->read(), + trim((new File($this->_JsonDir . 'get_countries_no_relationships.json'))->read()), $view->render() ); @@ -216,7 +216,7 @@ public function testEncodeWithDynamicSchemas() ]); $this->assertSame( - (new File($this->_JsonDir . 'get_country_no_relationships.json'))->read(), + trim((new File($this->_JsonDir . 'get_country_no_relationships.json'))->read()), $view->render() ); } @@ -242,7 +242,7 @@ public function testEncodeWithoutSchemas() ]); $this->assertSame( - (new File($this->_JsonDir . 'response_without_resources_meta.json'))->read(), + trim((new File($this->_JsonDir . 'response_without_resources_meta.json'))->read()), $view->render() ); } @@ -361,7 +361,7 @@ public function testOptionalDebugPrettyPrint() ]); $this->assertSame( - (new File($this->_JsonDir . 'get_country_no_relationships.json'))->read(), + trim((new File($this->_JsonDir . 'get_country_no_relationships.json'))->read()), $view->render() ); @@ -374,7 +374,7 @@ public function testOptionalDebugPrettyPrint() ]); $this->assertSame( - '{"data":{"type":"countries","id":"1","attributes":{"code":"NL","name":"The Netherlands","currency_id":1},"links":{"self":"\/countries\/1"}}}', + '{"data":{"type":"countries","id":"1","attributes":{"code":"NL","name":"The Netherlands"},"links":{"self":"\/countries\/1"}}}', $view->render() ); } From 16643f1d14e9143bb4652d1dccae648cc2bee188 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Fri, 7 Apr 2017 14:51:47 +0200 Subject: [PATCH 31/37] Use the repository that we already know about --- src/Schema/JsonApi/DynamicEntitySchema.php | 22 +++++-------------- .../Schema/DynamicEntitySchemaTest.php | 3 ++- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/Schema/JsonApi/DynamicEntitySchema.php b/src/Schema/JsonApi/DynamicEntitySchema.php index 841fb2755..5a33de862 100644 --- a/src/Schema/JsonApi/DynamicEntitySchema.php +++ b/src/Schema/JsonApi/DynamicEntitySchema.php @@ -99,15 +99,10 @@ public function getAttributes($entity) $entity->hiddenProperties($hidden); } - $repository = $this->_getRepository($entity); $attributes = $entity->toArray(); - if (!$repository) { - return $attributes; - } - // remove associated data so it won't appear inside jsonapi `attributes` - foreach ($repository->associations() as $association) { + foreach ($this->_repository->associations() as $association) { $propertyName = $association->property(); if ($association->type() === Association::MANY_TO_ONE) { @@ -136,13 +131,7 @@ public function getRelationships($entity, $isPrimary, array $includeRelationship { $relations = []; - $repository = $this->_getRepository($entity); - - if (!$repository) { - return $relations; - } - - foreach ($repository->associations() as $association) { + foreach ($this->_repository->associations() as $association) { $property = $association->property(); $data = $entity->get($property); @@ -191,11 +180,11 @@ public function getSelfSubUrl($entity = null) */ public function getRelationshipSelfLink($entity, $name, $meta = null, $treatAsHref = false) { - $byProperty = $this->_repository->associations()->getByProperty($name); - $relatedRepository = $byProperty->target(); + $association = $this->_repository->associations()->getByProperty($name); + $relatedRepository = $association->target(); // generate link for belongsTo relationship - if ($this->_stringIsSingular($name)) { + if (in_array($association->type(), [Association::MANY_TO_ONE, Association::ONE_TO_ONE])) { if ($this->_view->viewVars['_jsonApiBelongsToLinks'] === true) { list(, $controllerName) = pluginSplit($this->_repository->registryAlias()); $sourceName = Inflector::underscore(Inflector::singularize($controllerName)); @@ -225,6 +214,7 @@ public function getRelationshipSelfLink($entity, $name, $meta = null, $treatAsHr $url = Router::url($this->_getRepositoryRoutingParameters($relatedRepository) + [ '_method' => 'GET', + 'action' => 'index', $searchKey => $entity->id, ], $this->_view->viewVars['_absoluteLinks']); diff --git a/tests/TestCase/Schema/DynamicEntitySchemaTest.php b/tests/TestCase/Schema/DynamicEntitySchemaTest.php index 0dfc950b5..f9ca07b6f 100644 --- a/tests/TestCase/Schema/DynamicEntitySchemaTest.php +++ b/tests/TestCase/Schema/DynamicEntitySchemaTest.php @@ -5,6 +5,7 @@ use Cake\ORM\TableRegistry; use Crud\Listener\JsonApiListener; use Crud\TestSuite\TestCase; +use Neomerx\JsonApi\Contracts\Schema\SchemaFactoryInterface; use Neomerx\JsonApi\Factories\Factory; /** @@ -77,7 +78,7 @@ public function testGetAttributes() $schema = $this ->getMockBuilder('\Crud\Schema\JsonApi\DynamicEntitySchema') ->setMethods(null) - ->disableOriginalConstructor() + ->setConstructorArgs([$this->createMock(SchemaFactoryInterface::class), $view, $table]) ->getMock(); $this->setReflectionClassInstance($schema); From b8d40566e56bd0d056a11e592a5111722153c31b Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Fri, 7 Apr 2017 14:52:03 +0200 Subject: [PATCH 32/37] Add test for deeply nested json api requests --- .../get_countries_with_pagination.json | 6 +- ...t_country_with_currency_and_countries.json | 59 +++++++++++++++++++ .../Controller/JsonApiIntegrationTest.php | 22 +++---- 3 files changed, 74 insertions(+), 13 deletions(-) create mode 100644 tests/Fixture/JsonApi/get_country_with_currency_and_countries.json diff --git a/tests/Fixture/JsonApi/get_countries_with_pagination.json b/tests/Fixture/JsonApi/get_countries_with_pagination.json index fed66964a..e3ce74037 100644 --- a/tests/Fixture/JsonApi/get_countries_with_pagination.json +++ b/tests/Fixture/JsonApi/get_countries_with_pagination.json @@ -5,9 +5,9 @@ "page_limit": null }, "links": { - "self": "/countries?page=1", - "first": "/countries?page=1", - "last": "/countries?page=1", + "self": "\/countries?page=1", + "first": "\/countries?page=1", + "last": "\/countries?page=1", "prev": null, "next": null }, diff --git a/tests/Fixture/JsonApi/get_country_with_currency_and_countries.json b/tests/Fixture/JsonApi/get_country_with_currency_and_countries.json new file mode 100644 index 000000000..07f8f3931 --- /dev/null +++ b/tests/Fixture/JsonApi/get_country_with_currency_and_countries.json @@ -0,0 +1,59 @@ +{ + "data": { + "type": "countries", + "id": "1", + "attributes": { + "code": "NL", + "name": "The Netherlands" + }, + "relationships": { + "currency": { + "data": { + "type": "currencies", + "id": "1" + }, + "links": { + "self": "\/currencies\/1" + } + } + }, + "links": { + "self": "\/countries\/1" + } + }, + "included": [ + { + "type": "countries", + "id": "2", + "attributes": { + "code": "BE", + "name": "Belgium" + } + }, + { + "type": "currencies", + "id": "1", + "attributes": { + "code": "EUR", + "name": "Euro" + }, + "relationships": { + "countries": { + "data": [ + { + "type": "countries", + "id": "1" + }, + { + "type": "countries", + "id": "2" + } + ], + "links": { + "self": "\/countries?currency_id=1" + } + } + } + } + ] +} diff --git a/tests/TestCase/Controller/JsonApiIntegrationTest.php b/tests/TestCase/Controller/JsonApiIntegrationTest.php index f4eecda46..a5f5a4412 100644 --- a/tests/TestCase/Controller/JsonApiIntegrationTest.php +++ b/tests/TestCase/Controller/JsonApiIntegrationTest.php @@ -61,17 +61,15 @@ public function setUp() protected function _getExpected($file) { - return json_decode((new File($this->_JsonDir . $file))->read(), true); + return trim((new File($this->_JsonDir . $file))->read()); } - protected function _getResponseAsArray() + protected function _getResponse() { - $this->_response->getBody() - ->rewind(); - $response = $this->_response->getBody() - ->getContents(); + $this->_response->getBody()->rewind(); + $response = $this->_response->getBody()->getContents(); - return json_decode($response, true); + return $response; } /** @@ -87,7 +85,7 @@ public function testGet() $this->_assertJsonApiResponse(); $this->assertResponseCode(200); $this->assertResponseNotEmpty(); - $this->assertSame($this->_getExpected('get_countries_with_pagination.json'), $this->_getResponseAsArray()); + $this->assertSame($this->_getExpected('get_countries_with_pagination.json'), $this->_getResponse()); } /** @@ -163,6 +161,10 @@ public function viewProvider() '/countries/1?include=currencies,cultures', 'get_country_with_currency_and_culture.json' ], + 'include currency and deep countries' => [ + '/countries/1?include=currencies.countries', + 'get_country_with_currency_and_countries.json' + ], ]; } @@ -177,7 +179,7 @@ public function testView($url, $expectedFile) $this->get($url); $this->assertResponseSuccess(); - $this->assertSame($this->_getExpected($expectedFile), $this->_getResponseAsArray()); + $this->assertSame($this->_getExpected($expectedFile), $this->_getResponse()); } /** @@ -195,7 +197,7 @@ public function testViewWithContain() $this->get('/countries/1'); $this->assertResponseSuccess(); - $this->assertSame($this->_getExpected('get_country_with_currency_and_culture.json'), $this->_getResponseAsArray()); + $this->assertSame($this->_getExpected('get_country_with_currency_and_culture.json'), $this->_getResponse()); } /** From 6420a95415301780964cac56fba38dfd072d0a43 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sat, 8 Apr 2017 18:41:26 +0200 Subject: [PATCH 33/37] Don't use 3.4+ only method --- docs/listeners/jsonapi.rst | 4 ++-- tests/TestCase/Listener/JsonApiListenerTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/listeners/jsonapi.rst b/docs/listeners/jsonapi.rst index c07f9089a..a014d5085 100644 --- a/docs/listeners/jsonapi.rst +++ b/docs/listeners/jsonapi.rst @@ -520,7 +520,7 @@ This is done using the listener configuration: { $this->Crud ->listener('jsonApi') - ->setConfig('queryParameters.include.whitelist', ['cultures', 'cities']); + ->config('queryParameters.include.whitelist', ['cultures', 'cities']); return $this->Crud->execute(); } @@ -538,7 +538,7 @@ config option to ``true``: { $this->Crud ->listener('jsonApi') - ->setConfig('queryParameters.include.blacklist', true); + ->config('queryParameters.include.blacklist', true); return $this->Crud->execute(); } diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index 003745c87..580ac05c8 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -1336,7 +1336,7 @@ public function includeQueryProvider() 'Cultures', 'Currencies', ], - ['cultures', 'currency',] + ['cultures', 'currency'] ], 'blacklist with a whitelist wildcard' => [ 'cultures,currencies.countries,currencies.names,cultures.countries', From 5bf277c6ef70c55877e7119807d2df8f33211df0 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Sat, 8 Apr 2017 18:41:43 +0200 Subject: [PATCH 34/37] Fix codesniffer errors --- src/Listener/JsonApiListener.php | 4 +--- src/Schema/JsonApi/DynamicEntitySchema.php | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index a6f002629..01a940025 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -224,7 +224,7 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo } if (!empty($nestedIncludes)) { - $nestedContains = $this->_parseIncludes($nestedIncludes, $blacklist, $whitelist, $association ? $association->target() : null, $includePath);; + $nestedContains = $this->_parseIncludes($nestedIncludes, $blacklist, $whitelist, $association ? $association->target() : null, $includePath); } if (!empty($nestedContains)) { @@ -234,7 +234,6 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo } } - return $contains; } @@ -475,7 +474,6 @@ protected function _validateConfigOptions() if (!is_array($this->config('queryParameters'))) { throw new CrudException('JsonApiListener configuration option `queryParameters` only accepts an array'); } - } /** diff --git a/src/Schema/JsonApi/DynamicEntitySchema.php b/src/Schema/JsonApi/DynamicEntitySchema.php index 5a33de862..85eff5f96 100644 --- a/src/Schema/JsonApi/DynamicEntitySchema.php +++ b/src/Schema/JsonApi/DynamicEntitySchema.php @@ -76,12 +76,13 @@ public function getId($entity) } /** - * @param \Cake\Datasource\EntityInterface $entity + * @param \Cake\Datasource\EntityInterface $entity Entity * @return \Cake\Datasource\RepositoryInterface $repository */ protected function _getRepository($entity) { $repositoryName = $entity->source(); + return isset($this->_view->viewVars['_repositories'][$repositoryName]) ? $this->_view->viewVars['_repositories'][$repositoryName] : null; } From bfc545c9b07db280387e064ce4d5eab95203d83e Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Tue, 11 Apr 2017 16:36:55 +0200 Subject: [PATCH 35/37] Allow configuring multi-barreled relationship name inflection --- src/Listener/JsonApiListener.php | 37 +++++++++++++++---- src/Schema/JsonApi/DynamicEntitySchema.php | 4 +- .../TestCase/Listener/JsonApiListenerTest.php | 1 + 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 01a940025..37604bce9 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -63,6 +63,7 @@ class JsonApiListener extends ApiListener 'blacklist' => false, ] ], //Array of query parameters and associated transformers + 'inflect' => 'dasherize' ]; /** @@ -176,6 +177,30 @@ public function beforeRedirect(Event $event) $event->stopPropagation(); } + /** + * @param \Cake\Datasource\RepositoryInterface $repository Repository + * @param string $include The association include path + * @return \Cake\ORM\Association|null + */ + protected function _getAssociation(RepositoryInterface $repository, $include) + { + $delimiter = '-'; + if (strpos($include, '_') !== false) { + $delimiter = '_'; + } + $associationName = Inflector::camelize($include, $delimiter); + + $association = $repository->association($associationName);//First check base name + + if ($association) { + return $association; + } + + //If base name doesn't work, try to pluralize it + $associationName = Inflector::pluralize($associationName); + return $repository->association($associationName); + } + /** * Takes a "include" string and converts it into a correct CakePHP ORM association alias * @@ -210,14 +235,9 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo } $association = null; - $associationName = Inflector::camelize($include); - - if ($this->_stringIsSingular($include)) { - $associationName = Inflector::pluralize($associationName); - } if ($repository !== null) { - $association = $repository->association($associationName); + $association = $this->_getAssociation($repository, $include); if ($association === null) { throw new BadRequestException("Invalid relationship path '{$includeDotPath}' supplied in include parameter"); } @@ -228,9 +248,9 @@ protected function _parseIncludes($includes, $blacklist, $whitelist, Table $repo } if (!empty($nestedContains)) { - $contains[$associationName] = $nestedContains; + $contains[$association->alias()] = $nestedContains; } else { - $contains[] = $associationName; + $contains[] = $association->alias(); } } @@ -411,6 +431,7 @@ protected function _renderWithResources($subject) Inflector::tableize($repository->alias()) => $this->_getFindResult($subject), '_associations' => $usedAssociations, '_serialize' => true, + '_inflect' => $this->config('inflect') ]); return $this->_controller()->render(); diff --git a/src/Schema/JsonApi/DynamicEntitySchema.php b/src/Schema/JsonApi/DynamicEntitySchema.php index 85eff5f96..3a865749a 100644 --- a/src/Schema/JsonApi/DynamicEntitySchema.php +++ b/src/Schema/JsonApi/DynamicEntitySchema.php @@ -56,8 +56,8 @@ public function __construct( // NeoMerx required property holding lowercase singular or plural resource name if (!isset($this->resourceType)) { list (, $entityName) = pluginSplit($repository->registryAlias()); - - $this->resourceType = Inflector::underscore($entityName); + $method = isset($view->viewVars['_inflect']) ? $view->viewVars['_inflect'] : 'dasherize'; + $this->resourceType = Inflector::$method($entityName); } parent::__construct($factory); diff --git a/tests/TestCase/Listener/JsonApiListenerTest.php b/tests/TestCase/Listener/JsonApiListenerTest.php index 580ac05c8..335747a93 100644 --- a/tests/TestCase/Listener/JsonApiListenerTest.php +++ b/tests/TestCase/Listener/JsonApiListenerTest.php @@ -66,6 +66,7 @@ public function testDefaultConfig() 'blacklist' => false ] ], + 'inflect' => 'dasherize' ]; $this->assertSame($expected, $listener->config()); From 3ac72e4e607859c5262f52b4f85d29595b4bad97 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Tue, 11 Apr 2017 16:44:06 +0200 Subject: [PATCH 36/37] Fix relation self link generation --- src/Schema/JsonApi/DynamicEntitySchema.php | 8 +++----- tests/Fixture/JsonApi/get_country_with_culture.json | 3 +++ tests/Fixture/JsonApi/get_country_with_currency.json | 3 +++ .../JsonApi/get_country_with_currency_and_countries.json | 6 ++++++ .../JsonApi/get_country_with_currency_and_culture.json | 6 ++++++ tests/TestCase/Schema/DynamicEntitySchemaTest.php | 4 ++++ 6 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/Schema/JsonApi/DynamicEntitySchema.php b/src/Schema/JsonApi/DynamicEntitySchema.php index 3a865749a..05b0c3942 100644 --- a/src/Schema/JsonApi/DynamicEntitySchema.php +++ b/src/Schema/JsonApi/DynamicEntitySchema.php @@ -230,13 +230,11 @@ public function getRelationshipSelfLink($entity, $name, $meta = null, $treatAsHr */ public function getIncludedResourceLinks($entity) { - list(, $entityName) = namespaceSplit(get_class($entity)); - - $byProperty = $this->_repository->associations()->getByProperty(Inflector::underscore($entityName)); - if (!$byProperty) { + $repositoryName = $entity->source(); + if (!isset($this->_view->viewVars['_repositories'][$repositoryName])) { return []; } - $repository = $byProperty->target(); + $repository = $this->_view->viewVars['_repositories'][$repositoryName]; $url = Router::url($this->_getRepositoryRoutingParameters($repository) + [ '_method' => 'GET', diff --git a/tests/Fixture/JsonApi/get_country_with_culture.json b/tests/Fixture/JsonApi/get_country_with_culture.json index 64bc18c3a..b552f43e3 100644 --- a/tests/Fixture/JsonApi/get_country_with_culture.json +++ b/tests/Fixture/JsonApi/get_country_with_culture.json @@ -30,6 +30,9 @@ "attributes": { "code": "nl-NL", "name": "Dutch" + }, + "links": { + "self": "\/cultures\/1" } } ] diff --git a/tests/Fixture/JsonApi/get_country_with_currency.json b/tests/Fixture/JsonApi/get_country_with_currency.json index c8da961ea..0428ee76f 100644 --- a/tests/Fixture/JsonApi/get_country_with_currency.json +++ b/tests/Fixture/JsonApi/get_country_with_currency.json @@ -28,6 +28,9 @@ "attributes": { "code": "EUR", "name": "Euro" + }, + "links": { + "self": "\/currencies\/1" } } ] diff --git a/tests/Fixture/JsonApi/get_country_with_currency_and_countries.json b/tests/Fixture/JsonApi/get_country_with_currency_and_countries.json index 07f8f3931..8f15cd9c9 100644 --- a/tests/Fixture/JsonApi/get_country_with_currency_and_countries.json +++ b/tests/Fixture/JsonApi/get_country_with_currency_and_countries.json @@ -28,6 +28,9 @@ "attributes": { "code": "BE", "name": "Belgium" + }, + "links": { + "self": "\/countries\/2" } }, { @@ -53,6 +56,9 @@ "self": "\/countries?currency_id=1" } } + }, + "links": { + "self": "\/currencies\/1" } } ] diff --git a/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json b/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json index 9cdd4d81d..2ebc49025 100644 --- a/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json +++ b/tests/Fixture/JsonApi/get_country_with_currency_and_culture.json @@ -39,6 +39,9 @@ "attributes": { "code": "EUR", "name": "Euro" + }, + "links": { + "self": "\/currencies\/1" } }, { @@ -47,6 +50,9 @@ "attributes": { "code": "nl-NL", "name": "Dutch" + }, + "links": { + "self": "\/cultures\/1" } } ] diff --git a/tests/TestCase/Schema/DynamicEntitySchemaTest.php b/tests/TestCase/Schema/DynamicEntitySchemaTest.php index f9ca07b6f..0c298a265 100644 --- a/tests/TestCase/Schema/DynamicEntitySchemaTest.php +++ b/tests/TestCase/Schema/DynamicEntitySchemaTest.php @@ -214,6 +214,10 @@ public function testGetIncludedResourceLinks() $view->set('_absoluteLinks', false); $view->set('_jsonApiBelongsToLinks', false); + $view->set('_repositories', [ + 'Countries' => TableRegistry::get('Countries'), + 'Currencies' => TableRegistry::get('Currencies') + ]); // get results $table = TableRegistry::get('Countries'); From 542a3178f76b93eb6a27cd2b7cf660fca6c6ea5c Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Tue, 11 Apr 2017 16:48:00 +0200 Subject: [PATCH 37/37] Remove unused association key --- src/Listener/JsonApiListener.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Listener/JsonApiListener.php b/src/Listener/JsonApiListener.php index 37604bce9..e76b8bf8a 100644 --- a/src/Listener/JsonApiListener.php +++ b/src/Listener/JsonApiListener.php @@ -429,7 +429,6 @@ protected function _renderWithResources($subject) '_include' => $this->_getIncludeList($usedAssociations), '_fieldSets' => $this->config('fieldSets'), Inflector::tableize($repository->alias()) => $this->_getFindResult($subject), - '_associations' => $usedAssociations, '_serialize' => true, '_inflect' => $this->config('inflect') ]);