Add JSON API basic support #1036

hectorh30 · 2017-04-07T08:29:51Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR	TODO

Add support for the JSON API format: http://jsonapi.org/format/

TODOs:

Other desirable features:
Add suport for sparse fieldsets (similar to #1062)
Add basic include support
Improve POST support by adding the Location header according to spec
Improve linking support

Looking to contribute to #785.
Also related to #147 and #759 because of the PATCH HTTP method support.

dunglas · 2017-04-07T08:49:03Z

Great!

hectorh30 · 2017-04-08T05:12:11Z

What is api_sub_level flag used for? I'm worndering if it is to be considered in the JSON API normalizers, but can't find anything on the docs or the related PR (#616) about it

dunglas · 2017-04-08T06:43:52Z

It's an internal marker allowing a normalizer to build a context for serializing a subresource itself instead of relying on the trait.

hectorh30 · 2017-04-08T21:00:56Z

Is it ok to make PATCH universally available or should it be available only if using JSON API format? Right now I went for the former. In order to get tests working (and without fully knowing about the Hydra spec) right now I added a PATCH operation with @type: hydra:Operation. Hope thats not too bad.

Simperfit · 2017-04-13T09:55:59Z

Thanks for contributing to this ! didn't have much time to go forward.

Ill try to review it when it will be ready.

I think it's ok to have patch support, @dunglas ?

dunglas · 2017-04-20T16:23:22Z

src/JsonApi/EventListener/FlattenPaginationParametersListener.php

+ * Flattens possible 'page' array query parameter into dot-separated values to avoid
+ * conflicts with Doctrine\Orm\Extension\PaginationExtension.
+ *
+ * See: http://jsonapi.org/format/#fetching-pagination


You can use @see.

dunglas · 2017-04-20T16:24:04Z

src/JsonApi/EventListener/TransformFilteringParametersListener.php

+ * Flattens possible 'filter' array query parameter into first-level query parameters
+ * to be processed by api-platform.
+ *
+ * See: http://jsonapi.org/format/#fetching-filtering and http://jsonapi.org/recommendations/#filtering


dunglas · 2017-04-20T16:26:03Z

src/JsonApi/EventListener/TransformFilteringParametersListener.php

+        }
+
+        // If page query parameter is not defined or is not an array, never mind
+        if (!$request->query->get('filter') || !is_array($request->query->get('filter'))) {


This one will be faster: (less function call, the better):

$filter = $request->query->get('filter'); if (null === $filter || !is_array($filter)) { return; }

dunglas · 2017-04-20T16:26:32Z

src/JsonApi/EventListener/TransformFilteringParametersListener.php

+        }
+
+        // Otherwise, flatten into dot-separated values
+        $pageParameters = $request->query->get('filter');


And you'll be able to reuse the $filter variable here.

dunglas · 2017-04-20T16:27:22Z

src/JsonApi/EventListener/TransformSortingParametersListener.php

+final class TransformSortingParametersListener
+{
+    /**
+     * @var string Keyword used to retrieve the value


You can remove this docblock (it is useless)

dunglas · 2017-04-20T16:46:13Z

src/Serializer/AbstractItemNormalizer.php

@@ -417,7 +489,7 @@ protected function getAttributeValue($object, $attribute, $format = null, array
     *
     * @return string|array
     */
-    private function normalizeRelation(PropertyMetadata $propertyMetadata, $relatedObject, string $resourceClass, string $format = null, array $context)
+    protected function normalizeRelation(PropertyMetadata $propertyMetadata, $relatedObject, string $resourceClass, string $format = null, array $context)


I would prefer to don't propagate this data structure outside of the JSONAPI component. Can't you transform this array in IRI before passing it to the abstract normalizer?

dunglas · 2017-04-20T16:46:27Z

src/Serializer/NameConverter/CamelCaseToDashedCaseNameConverter.php

+class CamelCaseToDashedCaseNameConverter implements NameConverterInterface
+{
+    /**
+     * @var array|null


dunglas · 2017-04-20T16:46:34Z

src/Serializer/NameConverter/CamelCaseToDashedCaseNameConverter.php

+    private $attributes;
+
+    /**
+     * @var bool


dunglas · 2017-04-20T16:46:49Z

src/Serializer/NameConverter/CamelCaseToDashedCaseNameConverter.php

+     * @param null|array $attributes     The list of attributes to rename or null for all attributes
+     * @param bool       $lowerCamelCase Use lowerCamelCase style
+     */
+    public function __construct(array $attributes = null, $lowerCamelCase = true)


bool $lowerCamelCase

dunglas · 2017-04-20T16:47:47Z

src/Serializer/NameConverter/CamelCaseToDashedCaseNameConverter.php

+/**
+ * CamelCase to dashed name converter.
+ *
+ * Based on Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter.


Can't we use it directly instead of copying the class?

No because that one does not allow to use underscores instead of dashes.

hectorh30

Thanks a lot. I fixed most of the stuff.

Still pending: enable PATCH only if using JSON API.

hectorh30 · 2017-04-25T17:46:03Z

src/Serializer/AbstractItemNormalizer.php

@@ -417,7 +489,7 @@ protected function getAttributeValue($object, $attribute, $format = null, array
     *
     * @return string|array
     */
-    private function normalizeRelation(PropertyMetadata $propertyMetadata, $relatedObject, string $resourceClass, string $format = null, array $context)
+    protected function normalizeRelation(PropertyMetadata $propertyMetadata, $relatedObject, string $resourceClass, string $format = null, array $context)


Ok. Approaching another way.

hectorh30 · 2017-04-25T17:55:55Z

features/bootstrap/JsonApiContext.php

+        $isValidJsonapi = 'response.json is valid JSON API.' === $validationResponse;
+
+        if (!$isValidJsonapi) {
+            unlink($fileName);


Yep, good catch. Thanks.

hectorh30 · 2017-04-25T18:16:44Z

src/JsonApi/Serializer/CollectionNormalizer.php

+                );
+            }
+
+            $normalizedItem = $normalizedItem['data'];


Sure. I agree. WDYT about $normalizedItemData = $normalizedItem['data'];?

hectorh30 · 2017-04-25T18:20:39Z

src/JsonApi/Serializer/CollectionNormalizer.php

+
+            $relationships = [];
+
+            if (isset($normalizedItem['relationships'])) {


Yeah, no point. Sorry about that.

hectorh30 · 2017-04-25T18:25:25Z

src/JsonApi/Serializer/ConstraintViolationListNormalizer.php

+                $fieldName
+            );
+
+        if ($this->nameConverter) {


Sure, I took Symfony code this as a reference, but see no problem.

hectorh30 · 2017-04-25T18:41:09Z

src/JsonApi/Serializer/ItemNormalizer.php

+            foreach ($attributeValue as $attributeValueElement) {
+                if (!isset($attributeValueElement['data'])) {
+                    throw new RuntimeException(sprintf(
+                        'Expected \'data\' attribute in collection for attribute \'%s\'',


Would it be ok to specify the attribute? Something like 'The JSON API attribute \'%s\' must contain a "data" key.'

hectorh30 · 2017-04-25T18:48:00Z

src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php

@@ -46,7 +46,7 @@ public function create(string $resourceClass): ResourceMetadata

        if (null === $resourceMetadata->getItemOperations()) {
            $resourceMetadata = $resourceMetadata->withItemOperations($this->createOperations(
-                $isAbstract ? ['GET', 'DELETE'] : ['GET', 'PUT', 'DELETE']
+                $isAbstract ? ['GET', 'DELETE'] : ['GET', 'PUT', 'DELETE', 'PATCH']


Sure. I added it to the PR description.

hectorh30 · 2017-04-25T18:51:21Z

src/Serializer/NameConverter/CamelCaseToDashedCaseNameConverter.php

+/**
+ * CamelCase to dashed name converter.
+ *
+ * Based on Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter.


No because that one does not allow to use underscores instead of dashes.

hectorh30 · 2017-04-25T19:10:52Z

src/Serializer/AbstractItemNormalizer.php

@@ -417,7 +489,7 @@ protected function getAttributeValue($object, $attribute, $format = null, array
     *
     * @return string|array
     */
-    private function normalizeRelation(PropertyMetadata $propertyMetadata, $relatedObject, string $resourceClass, string $format = null, array $context)
+    protected function normalizeRelation(PropertyMetadata $propertyMetadata, $relatedObject, string $resourceClass, string $format = null, array $context)


Rolled back the changes on the visilibity of both methods, but got to copy some code to the JsonApi ItemNormalizer, hope thats ok.

meyerbaptiste · 2017-04-25T21:25:09Z

features/bootstrap/JsonApiContext.php

+    {
+        $fileName = $this->iSaveTheResponse();
+
+        $validationResponse = exec(sprintf('cd %s && jsonapi-validator -f response.json', __DIR__));


I repost my comment from #785 (comment) here, I'm really not a fan of this.

I agree. It should be done in Travis directly. No need to run the command from PHP.

@dunglas, my suggestion was to use \Sanpi\Behatch\Json\JsonInspector::validate() and not to run any command in Travis or PHP! 😛

Yep I agree with @meyerbaptiste didn't had the time to do it but we should do that

meyerbaptiste · 2017-04-25T21:45:51Z

src/JsonApi/Serializer/ErrorNormalizer.php

+            $trace = $object->getTrace();
+        }
+
+        $message = $object->getErrorMessage($object, $context, $this->debug);


It's $this instead of $object here. BTW, this var is useless.

meyerbaptiste · 2017-04-25T21:49:38Z

src/JsonApi/Serializer/ErrorNormalizer.php

+
+        $data = [
+            'title' => $context['title'] ?? 'An error occurred',
+            'description' => $message ?? (string) $object,


(string) $object is not correct here because if $object var is a FlattenException instance then you will get a fatal error because this class cannot be converted to string.

BTW, ErrorNormalizerTrait::getErrorMessage() always returns a string, so the null coalescing operator is useless here:

'description' => $this->getErrorMessage($object, $context, $this->debug),

You're totally right. Thanks a lot. I just pushed the fix.

meyerbaptiste · 2017-04-26T23:03:50Z

features/integration/nelmio_api_doc.feature

@@ -5,11 +5,12 @@ Feature: NelmioApiDoc integration

  Scenario: Create a user
    When I send a "GET" request to "/nelmioapidoc"
-    Then the response status code should be 200
+    And the response status code should be 200


Why this change?

meyerbaptiste · 2017-04-26T23:24:46Z

features/jsonapi/collections.feature

+
+  @createSchema
+  @dropSchema
+  Scenario: Correctly serialize a collection


Your scenario isn't correct. You have to divide it into several scenarios, with one for each operation. Each scenario consist of a list of steps: Givens, Whens, Thens. In your scenario here, there is only a list of Whens, no Thens, it's a little insane.
If you want to know more about this, I invite you to follow this link: http://docs.behat.org/en/latest/user_guide.html
You can also take inspiration from what has been done for Hydra: https://github.com/api-platform/core/tree/master/features/hydra

StephenOTT · 2017-07-13T21:27:14Z

How close is this work?

Is there major hurdles to overcome?

hectorh30 · 2017-07-14T14:32:45Z

Work on this was followed up in #1175. Please have a look there.

I'm closing this in favor of that PR.

* GraphQL Query support (api-platform#1358) * Fix missing service when no Varnish URL is defined * [PropertyFilter] Fix whitelist comparison (api-platform#1379) * Remove wrong doc * Swagger subcollection documentation issue (api-platform#1395) * Make the requirements configurable Closes api-platform#1401 * Provide a better exception message and type Getting "Not found" on a route where you are getting an object can get really confusing. * Bump branch alias to 2.2.x-dev 2.1.x-dev is already taken by the 2.1 branch, and this should represent the next minor version anyway. * Fix tests * Reuse PriorityTaggedServiceTrait from symfony * Improve payload support and remove duplicate code in ConstraintViolationListNormalizer (api-platform#1416) * Filter Annotation implementation Parent class filters (needs test) Support for nested properties Tests Fix some comments and remove id=>id in compiler pass * Throw on abstract data providers / filters * Remove an unused var * Remove useless badges * Enable the coverage * Fix some quality issues * Add job to test upstream libs deprecations If api-platform uses upstream libs in a deprecated way, we should be aware of it. * Add job to test upstream libs deprecations If api-platform uses upstream libs in a deprecated way, we should be aware of it. * Fix missing cache tag on empty collections * Allow plain IDs with `allow_plain_identifiers` * Fix indentation for GraphQL features. * Add JSON API basic support (api-platform#785, api-platform#1036, api-platform#1175) * Clean Behat tests * Fix tags addition with an empty value * Document swagger-specific description options … and where to find them should there be newer ones. * Fix PHPUnit tests * Fix missing return statement * Support & compatibility for PHP7.2 * Add feature to update swagger context for properties * Generator compat improvements (api-platform#1429) * Add support for resource names without namespace * [SF 4.0] Make actions explicitly public * Allow phpdocumentor/reflection-docblock 4 * fix hydra documentation normalizer with subresources * Create a base collection normalizer * Fix request auto-runner * Update the changelog * Update changelog

…platform#1175)

Amrouche Hamza and others added 6 commits April 7, 2017 01:46

feat: add jsonapi format

58bcfb4

add phpunit test

7b8bd74

wip: response.json is valid JSON API.

fc8d0a8

feat: add behat test

b9da3aa

WIP: Update tests and improve normalize/denormalize

ddc38d1

Apply php-cs-fixer changes

1b04cda

Get tests back to green

5d9846a

hectorh30 changed the title ~~Feature/add jsonapi update~~ Add JSON API basic support Apr 8, 2017

hectorh30 added 9 commits April 8, 2017 00:56

Test node specific version in travis

3bae015

Add php-cs-fixer style changes

7e2582a

WIP: Enable PATCH method support

6b21d46

Get POST of a RelatedDummy with an inhertied set field working

e1d467b

Fix tests

1421971

Add PATCH test to jsonapi.feature

27a133b

Get unit tests back to green

e2eaa3b

Get behat suite back to green

b1c05d8

Fix CS

e3e0cee

hectorh30 added 6 commits April 8, 2017 16:01

Add PATCH support to the swagger documentation normalizer

f2cc39c

Fix CS

ca3f390

Merge branch 'master' into feature/add-jsonapi-update

6051c5c

Add jsonapi-validator to appveyor CI

d9e4f8f

Add basic error serialization support

42eeb19

Add basic pagination support

cc12778

soyuka added this to the 2.1.0 milestone Apr 12, 2017

WIP: Add basic pagination support

43754f3

Simperfit requested a review from teohhanhui April 20, 2017 16:27

Simperfit added [WFC] Waiting for Change enhancement labels Apr 20, 2017

Simperfit mentioned this pull request Apr 20, 2017

[RFR] feat: add jsonapi format #785

Closed

2 tasks

dunglas requested changes Apr 20, 2017

View reviewed changes

hectorh30 added 3 commits April 25, 2017 12:54

Apply review changes

a4fd58c

Merge branch 'master' into feature/add-jsonapi-update

77d34e6

Fix syntax error

59e0716

hectorh30 commented Apr 25, 2017

View reviewed changes

Fix CS

33b6eb0

meyerbaptiste requested changes Apr 25, 2017

View reviewed changes

meyerbaptiste mentioned this pull request Apr 26, 2017

Remove useless code for error normalizers #1084

Merged

Fix ErrorNormalizer and variable names

858f4ac

meyerbaptiste requested changes Apr 26, 2017

View reviewed changes

Add support for POSTing and PATCHing many-to-many relationships

39955cb

meyerbaptiste mentioned this pull request Jun 13, 2017

Add JSON API basic support (continuation) #1175

Merged

hectorh30 closed this Jul 14, 2017

meyerbaptiste added a commit to meyerbaptiste/core that referenced this pull request Sep 18, 2017

Add support for jsonapi format (api-platform#785, api-platform#1036)

77ce702

meyerbaptiste added a commit to meyerbaptiste/core that referenced this pull request Sep 18, 2017

Add support for jsonapi format (api-platform#785, api-platform#1036)

1cd41cd

meyerbaptiste added a commit to meyerbaptiste/core that referenced this pull request Oct 6, 2017

Add support for jsonapi format (api-platform#785, api-platform#1036)

a195a1e

meyerbaptiste added a commit to meyerbaptiste/core that referenced this pull request Oct 12, 2017

Add support for jsonapi format (api-platform#785, api-platform#1036)

30a96c8

meyerbaptiste added a commit to meyerbaptiste/core that referenced this pull request Oct 12, 2017

Add support for jsonapi format (api-platform#785, api-platform#1036)

7270737

meyerbaptiste added a commit to meyerbaptiste/core that referenced this pull request Oct 13, 2017

Add support for jsonapi format (api-platform#785, api-platform#1036)

646c303

meyerbaptiste added a commit to meyerbaptiste/core that referenced this pull request Oct 13, 2017

Add support for jsonapi format (api-platform#785, api-platform#1036)

9100c35

meyerbaptiste added a commit that referenced this pull request Oct 13, 2017

Add JSON API basic support (#785, #1036, #1175)

4b3d1ab

hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018

Add JSON API basic support (api-platform#785, api-platform#1036, api-…

913f80a

…platform#1175)


		$relationships = [];

		if (isset($normalizedItem['relationships'])) {

Add JSON API basic support #1036

Add JSON API basic support #1036

Conversation

hectorh30 commented Apr 7, 2017 • edited Loading

dunglas commented Apr 7, 2017

hectorh30 commented Apr 8, 2017 • edited Loading

dunglas commented Apr 8, 2017

hectorh30 commented Apr 8, 2017

Simperfit commented Apr 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hectorh30 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meyerbaptiste Apr 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StephenOTT commented Jul 13, 2017

hectorh30 commented Jul 14, 2017

hectorh30 commented Apr 7, 2017 •

edited

Loading

hectorh30 commented Apr 8, 2017 •

edited

Loading

meyerbaptiste Apr 25, 2017 •

edited

Loading