[Refactor] Properly extract existing values for update requests

Closes #2

Author: lindyhopchris

CHANGELOG.md

@@ -3,6 +3,19 @@
 All notable changes to this project will be documented in this file. This project adheres to
 [Semantic Versioning](http://semver.org/) and [this changelog format](http://keepachangelog.com/).
 
+## Unreleased
+
+### Changed
+- [#2](https://github.com/laravel-json-api/laravel/issues/2) **BREAKING** Improved the extraction of existing resource
+  field values when constructing validation data for update requests:
+    - The `existingAttributes()` and `existingRelationships()` methods on the resource request class has been removed.
+      If you need to modify the existing values before the client values are merged, implement the `withExisting()`
+      method instead. This receives the model its JSON representation (as an array).
+    - The `mustValidate()` method must now be called on a schema relationship field. (Previously this was on the
+      resource relation.) By default, belongs-to and morph-to relations will be included when extracting existing
+      values; all other relations will not. Use the `mustValidate()` or `notValidated()` method on the schema relation
+      to alter whether a relation is included in the extracted values.
+
 ## [1.0.0-alpha.2] - 2021-02-02
 
 ### Added

composer.json

@@ -25,9 +25,9 @@
     "require": {
         "php": "^7.4|^8.0",
         "ext-json": "*",
-        "laravel-json-api/core": "^1.0.0-alpha.2",
-        "laravel-json-api/eloquent": "^1.0.0-alpha.2",
-        "laravel-json-api/encoder-neomerx": "^1.0.0-alpha.2",
+        "laravel-json-api/core": "^1.0.0-alpha.3",
+        "laravel-json-api/eloquent": "^1.0.0-alpha.3",
+        "laravel-json-api/encoder-neomerx": "^1.0.0-alpha.3",
         "laravel-json-api/exceptions": "^1.0.0-alpha.1",
         "laravel-json-api/spec": "^1.0.0-alpha.2",
         "laravel-json-api/validation": "^1.0.0-alpha.2",

src/Http/Requests/ResourceRequest.php

@@ -24,11 +24,10 @@
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Http\Response;
 use LaravelJsonApi\Contracts\Auth\Authorizer;
-use LaravelJsonApi\Contracts\Resources\Container as ResourceContainer;
+use LaravelJsonApi\Contracts\Schema\Relation;
 use LaravelJsonApi\Core\Document\ResourceObject;
 use LaravelJsonApi\Core\Exceptions\JsonApiException;
-use LaravelJsonApi\Core\Resources\JsonApiResource;
-use LaravelJsonApi\Core\Resources\Relation;
+use LaravelJsonApi\Core\Query\IncludePaths;
 use LaravelJsonApi\Core\Support\Str;
 use LaravelJsonApi\Spec\RelationBuilder;
 use LaravelJsonApi\Spec\ResourceBuilder;
@@ -41,11 +40,6 @@
 class ResourceRequest extends FormRequest
 {
 
-    /**
-     * @var bool
-     */
-    protected bool $validateExisting = true;
-
     /**
      * @var callable|null
      */
@@ -353,43 +347,37 @@ protected function dataForCreate(array $document): array
      * @param Model|object $model
      *      the model being updated.
      * @param array $document
-     *      the JSON API document to validate.
+     *      the JSON:API document to validate.
      * @return array
      */
     protected function dataForUpdate(object $model, array $document): array
     {
-        $data = $document['data'] ?? [];
-
-        if ($this->mustValidateExisting($model, $document)) {
-            $data['attributes'] = $this->extractAttributes(
-                $model,
-                $data['attributes'] ?? []
-            );
+        $existing = $this->extractForUpdate($model);
 
-            $data['relationships'] = $this->extractRelationships(
-                $model,
-                $data['relationships'] ?? []
-            );
+        if (method_exists($this, 'withExisting')) {
+            $existing = $this->withExisting($model, $existing) ?? $existing;
         }
 
-        return $data;
+        return ResourceObject::fromArray($existing)->merge(
+            $document['data']
+        )->jsonSerialize();
     }
 
     /**
      * Get validation data for modifying a relationship.
      *
-     * @param Model|object $record
+     * @param Model|object $model
      * @param string $fieldName
      * @param array $document
      * @return array
      */
-    protected function dataForRelationship(object $record, string $fieldName, array $document): array
+    protected function dataForRelationship(object $model, string $fieldName, array $document): array
     {
-        $resource = $this->resources()->create($record);
+        $route = $this->jsonApi()->route();
 
         return [
-            'type' => $resource->type(),
-            'id' => $resource->id(),
+            'type' => $route->resourceType(),
+            'id' => $route->resourceId(),
             'relationships' => [
                 $fieldName => [
                     'data' => $document['data'],
@@ -406,14 +394,7 @@ protected function dataForRelationship(object $record, string $fieldName, array
      */
     protected function dataForDelete(object $record): array
     {
-        $route = $this->jsonApi()->route();
-
-        $data = $this->dataForUpdate($record, [
-            'data' => [
-                'type' => $route->resourceType(),
-                'id' => $route->resourceId(),
-            ],
-        ]);
+        $data = $this->extractForUpdate($record);
 
         if (method_exists($this, 'metaForDelete')) {
             $data['meta'] = (array) $this->metaForDelete($record);
@@ -423,80 +404,36 @@ protected function dataForDelete(object $record): array
     }
 
     /**
-     * Should existing resource values be provided to the validator for an update request?
+     * Extract the existing values for the provided model.
      *
-     * Child classes can overload this method if they need to programmatically work out
-     * if existing values must be provided to the validator instance for an update request.
-     *
-     * @param Model|object $model
-     *      the model being updated
-     * @param array $document
-     *      the JSON API document provided by the client.
-     * @return bool
-     */
-    protected function mustValidateExisting(object $model, array $document): bool
-    {
-        return false !== $this->validateExisting;
-    }
-
-    /**
-     * Extract existing attributes for the provided model.
-     *
-     * @param Model|object $model
-     * @return iterable
+     * @param object $model
+     * @return array
      */
-    protected function existingAttributes(object $model): iterable
+    private function extractForUpdate(object $model): array
     {
-        $resource = $this->resources()->create($model);
+        $encoder = $this->jsonApi()->server()->encoder();
 
-        return $resource->attributes($this);
+        return $encoder
+            ->withRequest($this)
+            ->withIncludePaths($this->includePathsToExtract($model))
+            ->withResource($model)
+            ->toArray()['data'];
     }
 
     /**
-     * Extract existing relationships for the provided model.
+     * Get the relationship paths that must be included when extracting the current field values.
      *
-     * @param Model|object $model
-     * @return iterable
+     * @param object $model
+     * @return IncludePaths
      */
-    protected function existingRelationships(object $model): iterable
+    private function includePathsToExtract(object $model): IncludePaths
     {
-        $resource = $this->resources()->create($model);
-
-        /** @var Relation $relationship */
-        foreach ($resource->relationships($this) as $relationship) {
-            if ($relationship->isValidated()) {
-                yield $relationship->fieldName() => $relationship->data();
-            }
-        }
-    }
+        $paths = collect($this->schema()->relationships())
+            ->filter(fn (Relation $relation) => $relation->isValidated())
+            ->map(fn (Relation $relation) => $relation->name())
+            ->values();
 
-    /**
-     * Extract attributes for a resource update.
-     *
-     * @param Model|object $model
-     * @param array $new
-     * @return array
-     */
-    private function extractAttributes(object $model, array $new): array
-    {
-        return collect($this->existingAttributes($model))
-            ->merge($new)
-            ->all();
-    }
-
-    /**
-     * Extract relationships for a resource update.
-     *
-     * @param Model|object $model
-     * @param array $new
-     * @return array
-     */
-    private function extractRelationships(object $model, array $new): array
-    {
-        return collect($this->existingRelationships($model))
-            ->map(fn($value) => $this->convertExistingRelationships($value))
-            ->merge($new)
-            ->all();
+        return IncludePaths::fromArray($paths);
     }
 
     /**
@@ -514,64 +451,6 @@ private function relationshipRules(): array
             ->all();
     }
 
-    /**
-     * Convert relationships returned by the `existingRelationships()` method.
-     *
-     * We support the method returning JSON API formatted relationships, e.g.:
-     *
-     * ```
-     * return [
-     *          'author' => [
-     *            'data' => [
-     *              'type' => 'users',
-     *              'id' => (string) $record->author->getRouteKey(),
-     *          ],
-     *      ],
-     * ];
-     * ```
-     *
-     * Or this shorthand:
-     *
-     * ```php
-     * return [
-     *      'author' => $record->author,
-     * ];
-     * ```
-     *
-     * This method converts the shorthand into the JSON API formatted relationships.
-     *
-     * @param $value
-     * @return array
-     */
-    private function convertExistingRelationships($value)
-    {
-        if (is_array($value) && array_key_exists('data', $value)) {
-            return $value;
-        }
-
-        if (is_null($value)) {
-            return ['data' => null];
-        }
-
-        $value = $this->resources()->resolve($value);
-
-        if ($value instanceof JsonApiResource) {
-            return [
-                'data' => [
-                    'type' => $value->type(),
-                    'id' => $value->id(),
-                ],
-            ];
-        }
-
-        return [
-            'data' => collect($value)->map(fn(JsonApiResource $resource) => [
-                'type' => $resource->type(),
-                'id' => $resource->id()
-            ])->all(),
-        ];
-    }
-
     /**
      * Validate the JSON API document for a resource request.
      *
@@ -618,12 +497,4 @@ private function validateRelationshipDocument(): void
         }
     }
 
-    /**
-     * @return ResourceContainer
-     */
-    final protected function resources(): ResourceContainer
-    {
-        return $this->jsonApi()->server()->resources();
-    }
-
 }

tests/dummy/app/JsonApi/V1/Posts/PostResource.php

@@ -55,7 +55,7 @@ public function relationships($request): iterable
         return [
             $this->relation('author')->showDataIfLoaded(),
             $this->relation('comments'),
-            $this->relation('tags')->showDataIfLoaded()->mustValidate(),
+            $this->relation('tags')->showDataIfLoaded(),
         ];
     }
 

tests/dummy/tests/Api/V1/Posts/DeleteTest.php

@@ -44,6 +44,7 @@ protected function setUp(): void
     public function test(): void
     {
         $response = $this
+            ->withoutExceptionHandling()
             ->actingAs($this->post->author)
             ->jsonApi()
             ->delete(url('api/v1/posts', $this->post));

tests/dummy/tests/Api/V1/Posts/IndexTest.php

@@ -101,13 +101,13 @@ public function testIncludeAuthor(): void
     {
         $posts = Post::factory()->count(2)->create();
 
-        $expected1 = $this->serializer->post($posts[0])->toArray();
+        $expected1 = $this->serializer->post($posts[0])->jsonSerialize();
         $expected1['relationships']['author']['data'] = $user1 = [
             'type' => 'users',
             'id' => (string) $posts[0]->author->getRouteKey(),
         ];
 
-        $expected2 = $this->serializer->post($posts[1])->toArray();
+        $expected2 = $this->serializer->post($posts[1])->jsonSerialize();
         $expected2['relationships']['author']['data'] = $user2 = [
             'type' => 'users',
             'id' => (string) $posts[1]->author->getRouteKey(),
@@ -143,7 +143,7 @@ public function testIdFilter(): void
     public function testSlugFilter(): void
     {
         $posts = Post::factory()->count(2)->create();
-        $expected = $this->serializer->post($posts[1])->toArray();
+        $expected = $this->serializer->post($posts[1])->jsonSerialize();
 
         $response = $this
             ->withoutExceptionHandling()

tests/dummy/tests/Api/V1/Posts/UpdateTest.php

@@ -97,6 +97,7 @@ public function testIndividualField(string $fieldName): void
             ->replace($fieldName, $data[$fieldName]);
 
         $response = $this
+            ->withoutExceptionHandling()
             ->actingAs($this->post->author)
             ->jsonApi('posts')
             ->withData($data)
@@ -165,8 +166,7 @@ public function testNotAcceptableMediaType(): void
 
         $response = $this
             ->actingAs($this->post->author)
-            ->jsonApi()
-            ->expects('posts')
+            ->jsonApi('posts')
             ->accept('text/html')
             ->withData($data)
             ->patch(url('/api/v1/posts', $this->post));