src/Controller/EntityResource.php | 52 ++++++++++++++++++++-- tests/src/Functional/JsonApiFunctionalTest.php | 4 +- tests/src/Functional/JsonApiRegressionTest.php | 2 +- tests/src/Functional/ResourceTestBase.php | 41 ++++------------- tests/src/Kernel/Controller/EntityResourceTest.php | 6 +-- 5 files changed, 64 insertions(+), 41 deletions(-) diff --git a/src/Controller/EntityResource.php b/src/Controller/EntityResource.php index 82db627..bd1324b 100644 --- a/src/Controller/EntityResource.php +++ b/src/Controller/EntityResource.php @@ -543,13 +543,59 @@ class EntityResource { $field_name = $field_list->getName(); throw new EntityAccessDeniedHttpException($entity, $field_access, '/data/relationships/' . $field_name, sprintf('The current user is not allowed to PATCH the selected field (%s).', $field_name)); } + $original_field_list = clone $field_list; // Time to save the relationship. foreach ($parsed_field_list as $field_item) { $field_list->appendItem($field_item->getValue()); } $this->validate($entity); $entity->save(); - return $this->getRelationship($entity, $related_field, $request, 201); + $status = static::relationshipArityIsAffected($original_field_list, $field_list) + ? 200 + : 204; + return $this->getRelationship($entity, $related_field, $request, $status); + } + + /** + * Checks whether relationship arity is affected. + * + * @param \Drupal\Core\Field\EntityReferenceFieldItemListInterface $old + * The old (stored) entity references. + * @param \Drupal\Core\Field\EntityReferenceFieldItemListInterface $new + * The new (udpated) entity references. + * + * @return bool + * Whether entities already being referenced now have additional references. + * + * @see \Drupal\jsonapi\Normalizer\Value\RelationshipNormalizerValue::ensureUniqueResourceIdentifierObjects() + */ + protected static function relationshipArityIsAffected(EntityReferenceFieldItemListInterface $old, EntityReferenceFieldItemListInterface $new) { + $old_targets = static::toTargets($old); + $new_targets = static::toTargets($new); + $relationship_count_changed = count($old_targets) !== count($new_targets); + $existing_relationships_updated = !empty(array_unique(array_intersect($old_targets, $new_targets))); + return $relationship_count_changed && $existing_relationships_updated; + } + + /** + * Maps a list of entity reference field objects to a list of targets. + * + * @param \Drupal\Core\Field\EntityReferenceFieldItemListInterface $relationship_list + * A list of entity reference field objects. + * + * @return string[]|int[] + * A list of targets. + */ + protected static function toTargets(EntityReferenceFieldItemListInterface $relationship_list) { + $main_property_name = $relationship_list->getFieldDefinition() + ->getFieldStorageDefinition() + ->getMainPropertyName(); + + $values = []; + foreach ($relationship_list->getIterator() as $relationship) { + $values[] = $relationship->getValue()[$main_property_name]; + } + return $values; } /** @@ -588,7 +634,7 @@ class EntityResource { $this->{$method}($entity, $parsed_field_list); $this->validate($entity); $entity->save(); - return $this->getRelationship($entity, $related_field, $request); + return $this->getRelationship($entity, $related_field, $request, 204); } /** @@ -681,7 +727,7 @@ class EntityResource { // Save the entity and return the response object. $this->validate($entity); $entity->save(); - return $this->getRelationship($entity, $related_field, $request, 201); + return $this->getRelationship($entity, $related_field, $request, 204); } /** diff --git a/tests/src/Functional/JsonApiFunctionalTest.php b/tests/src/Functional/JsonApiFunctionalTest.php index 9e44660..33dd079 100644 --- a/tests/src/Functional/JsonApiFunctionalTest.php +++ b/tests/src/Functional/JsonApiFunctionalTest.php @@ -744,7 +744,7 @@ class JsonApiFunctionalTest extends JsonApiFunctionalTestBase { 'headers' => ['Content-Type' => 'application/vnd.api+json'], ]); $updated_response = Json::decode($response->getBody()->__toString()); - $this->assertEquals(201, $response->getStatusCode()); + $this->assertEquals(200, $response->getStatusCode()); $this->assertEquals(3, count($updated_response['data'])); $this->assertEquals('taxonomy_term--tags', $updated_response['data'][2]['type']); $this->assertEquals($this->tags[2]->uuid(), $updated_response['data'][2]['id']); @@ -763,7 +763,7 @@ class JsonApiFunctionalTest extends JsonApiFunctionalTestBase { 'headers' => ['Content-Type' => 'application/vnd.api+json'], ]); $updated_response = Json::decode($response->getBody()->__toString()); - $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals(204, $response->getStatusCode()); $this->assertCount(1, $updated_response['data']); $this->assertEquals('taxonomy_term--tags', $updated_response['data'][0]['type']); $this->assertEquals($this->tags[1]->uuid(), $updated_response['data'][0]['id']); diff --git a/tests/src/Functional/JsonApiRegressionTest.php b/tests/src/Functional/JsonApiRegressionTest.php index f7e9804..2a65397 100644 --- a/tests/src/Functional/JsonApiRegressionTest.php +++ b/tests/src/Functional/JsonApiRegressionTest.php @@ -171,7 +171,7 @@ class JsonApiRegressionTest extends JsonApiFunctionalTestBase { ], ]; $response = $this->request('POST', $url, $request_options); - $this->assertSame(201, $response->getStatusCode(), (string) $response->getBody()); + $this->assertSame(204, $response->getStatusCode(), (string) $response->getBody()); } /** diff --git a/tests/src/Functional/ResourceTestBase.php b/tests/src/Functional/ResourceTestBase.php index e0e541f..b1e0d1c 100644 --- a/tests/src/Functional/ResourceTestBase.php +++ b/tests/src/Functional/ResourceTestBase.php @@ -1392,16 +1392,11 @@ abstract class ResourceTestBase extends BrowserTestBase { // Test POST: empty data. $request_options[RequestOptions::BODY] = Json::encode(['data' => []]); $response = $this->request('POST', $url, $request_options); - // @todo Remove line below in favor of commented line in https://www.drupal.org/project/jsonapi/issues/2977653. - $this->assertResourceResponse(201, FALSE, $response); - // $this->assertResourceResponse(204, NULL, $response); + $this->assertResourceResponse(204, NULL, $response); // Test PATCH: empty data. $request_options[RequestOptions::BODY] = Json::encode(['data' => []]); $response = $this->request('PATCH', $url, $request_options); - // @todo Remove line below in favor of commented line in https://www.drupal.org/project/jsonapi/issues/2977653. - $expected_document = $this->getExpectedGetRelationshipDocument($relationship_field_name, $resource); - $this->assertResourceResponse(200, $expected_document, $response); - /* $this->assertResourceResponse(204, NULL, $response); */ + $this->assertResourceResponse(204, NULL, $response); // Test POST: data as resource identifier, not array of identifiers. $request_options[RequestOptions::BODY] = Json::encode(['data' => $target_identifier]); @@ -1439,10 +1434,7 @@ abstract class ResourceTestBase extends BrowserTestBase { $request_options[RequestOptions::BODY] = Json::encode(['data' => [$target_identifier]]); $response = $this->request('POST', $url, $request_options); $resource->set($relationship_field_name, [$target_resource]); - $expected_document = $this->getExpectedGetRelationshipDocument($relationship_field_name, $resource); - // @todo Remove line below in favor of commented line in https://www.drupal.org/project/jsonapi/issues/2977653. - $this->assertResourceResponse(201, $expected_document, $response); - /* $this->assertResourceResponse(204, NULL, $response); */ + $this->assertResourceResponse(204, NULL, $response); // @todo: Uncomment the following two assertions in https://www.drupal.org/project/jsonapi/issues/2977659. // Test POST: success, relationship already exists, no arity. @@ -1457,10 +1449,7 @@ abstract class ResourceTestBase extends BrowserTestBase { $request_options[RequestOptions::BODY] = Json::encode(['data' => [$target_identifier]]); $response = $this->request('PATCH', $url, $request_options); $resource->set($relationship_field_name, [$target_resource]); - $expected_document = $this->getExpectedGetRelationshipDocument($relationship_field_name, $resource); - // @todo Remove line below in favor of commented line in https://www.drupal.org/project/jsonapi/issues/2977653. - $this->assertResourceResponse(200, $expected_document, $response); - /* $this->assertResourceResponse(204, NULL, $response); */ + $this->assertResourceResponse(204, NULL, $response); // Test POST: success, relationship already exists, with unique arity. $request_options[RequestOptions::BODY] = Json::encode([ @@ -1475,9 +1464,7 @@ abstract class ResourceTestBase extends BrowserTestBase { $expected_document['data'][1] += ['meta' => ['arity' => 1]]; // 200 with response body because the request did not include the // existing relationship resource identifier object. - // @todo Remove line below in favor of commented assertion in https://www.drupal.org/project/jsonapi/issues/2977653. - $this->assertResourceResponse(201, $expected_document, $response); - /* $this->assertResourceResponse(200, $expected_document, $response); */ + $this->assertResourceResponse(200, $expected_document, $response); // @todo: Uncomment the following block in https://www.drupal.org/project/jsonapi/issues/2977659. // @codingStandardsIgnoreStart @@ -1501,10 +1488,7 @@ abstract class ResourceTestBase extends BrowserTestBase { $request_options[RequestOptions::BODY] = Json::encode(['data' => [$target_identifier]]); $response = $this->request('DELETE', $url, $request_options); $resource->set($relationship_field_name, []); - $expected_document = $this->getExpectedGetRelationshipDocument($relationship_field_name, $resource); - // @todo Remove line below in favor of commented line in https://www.drupal.org/project/jsonapi/issues/2977653. - $this->assertResourceResponse(201, $expected_document, $response); - /* $this->assertResourceResponse(204, NULL, $response); */ + $this->assertResourceResponse(204, NULL, $response); $expected_document = $this->getExpectedGetRelationshipDocument($relationship_field_name, $resource); $response = $this->request('GET', $url, $request_options); $this->assertSameDocument($expected_document, Json::decode((string) $response->getBody())); @@ -1512,9 +1496,7 @@ abstract class ResourceTestBase extends BrowserTestBase { // Test DELETE: no existing relationships, no op, success. $request_options[RequestOptions::BODY] = Json::encode(['data' => [$target_identifier]]); $response = $this->request('DELETE', $url, $request_options); - // @todo Remove line below in favor of commented line in https://www.drupal.org/project/jsonapi/issues/2977653. - $this->assertResourceResponse(201, $expected_document, $response); - /* $this->assertResourceResponse(204, NULL, $response); */ + $this->assertResourceResponse(204, NULL, $response); $expected_document = $this->getExpectedGetRelationshipDocument($relationship_field_name, $resource); $response = $this->request('GET', $url, $request_options); $this->assertSameDocument($expected_document, Json::decode((string) $response->getBody())); @@ -1526,19 +1508,14 @@ abstract class ResourceTestBase extends BrowserTestBase { $expected_document = $this->getExpectedGetRelationshipDocument($relationship_field_name, $resource); $expected_document['data'][0] += ['meta' => ['arity' => 0]]; $expected_document['data'][1] += ['meta' => ['arity' => 1]]; - // @todo Remove line below in favor of commented line in https://www.drupal.org/project/jsonapi/issues/2977653. - $this->assertResourceResponse(200, $expected_document, $response); - /* $this->assertResourceResponse(204, NULL, $response); */ + $this->assertResourceResponse(204, NULL, $response); // Test DELETE: two existing relationships, both removed because no arity // was specified. $request_options[RequestOptions::BODY] = Json::encode(['data' => [$target_identifier]]); $response = $this->request('DELETE', $url, $request_options); $resource->set($relationship_field_name, []); - $expected_document = $this->getExpectedGetRelationshipDocument($relationship_field_name, $resource); - // @todo Remove line below in favor of commented line in https://www.drupal.org/project/jsonapi/issues/2977653. - $this->assertResourceResponse(201, $expected_document, $response); - /* $this->assertResourceResponse(204, NULL, $response); */ + $this->assertResourceResponse(204, NULL, $response); $resource->set($relationship_field_name, []); $expected_document = $this->getExpectedGetRelationshipDocument($relationship_field_name, $resource); $response = $this->request('GET', $url, $request_options); diff --git a/tests/src/Kernel/Controller/EntityResourceTest.php b/tests/src/Kernel/Controller/EntityResourceTest.php index e8268f7..5a7124c 100644 --- a/tests/src/Kernel/Controller/EntityResourceTest.php +++ b/tests/src/Kernel/Controller/EntityResourceTest.php @@ -691,7 +691,7 @@ class EntityResourceTest extends JsonapiKernelTestBase { $this->assertInstanceOf(EntityReferenceFieldItemListInterface::class, $field_list); $this->assertSame('field_relationships', $field_list->getName()); $this->assertEquals([['target_id' => 1]], $field_list->getValue()); - $this->assertEquals(201, $response->getStatusCode()); + $this->assertEquals(204, $response->getStatusCode()); } /** @@ -720,7 +720,7 @@ class EntityResourceTest extends JsonapiKernelTestBase { $this->assertInstanceOf(EntityReferenceFieldItemListInterface::class, $field_list); $this->assertSame('field_relationships', $field_list->getName()); $this->assertEquals($relationships, $field_list->getValue()); - $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals(204, $response->getStatusCode()); } /** @@ -764,7 +764,7 @@ class EntityResourceTest extends JsonapiKernelTestBase { $this->assertInstanceOf(EntityReferenceFieldItemListInterface::class, $field_list); $this->assertSame('field_relationships', $field_list->getName()); $this->assertEquals($kept_rels, $field_list->getValue()); - $this->assertEquals(201, $response->getStatusCode()); + $this->assertEquals(204, $response->getStatusCode()); } /**