diff --git a/CHANGELOG.md b/CHANGELOG.md index a74a565..f895ef1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -117,6 +117,9 @@ and this project adheres to ### Fixed +- Fix nested includes being dropped for a resource that an earlier include + path already reached with a shallower include + ([#125](https://github.com/tobyzerner/json-api-server/issues/125)) - `Context::filter()` and `filters()` now normalize the request's filters for the primary collection, so related resources no longer reject valid filters - Laravel: Reject cursors that can't be decoded instead of ignoring them and diff --git a/src/Serializer.php b/src/Serializer.php index 3c4bd88..3f04143 100644 --- a/src/Serializer.php +++ b/src/Serializer.php @@ -156,14 +156,13 @@ private function initializeResource( private function serializeFields(string $key, Context $context): void { foreach ($context->sparseFields($context->resource) as $field) { - if ($this->fieldProcessed($key, $field)) { + $include = $context->include[$field->name] ?? null; + + if ($this->fieldProcessed($key, $field, $include)) { continue; } - $fieldContext = $context->forSerializedField( - $field, - $context->include[$field->name] ?? null, - ); + $fieldContext = $context->forSerializedField($field, $include); if (!$field->isVisible($fieldContext)) { continue; @@ -214,17 +213,54 @@ private function serializeLinks(string $key, Context $context): void } } - private function fieldProcessed(string $key, Field $field): bool + /** + * A resource can be reached through several include paths. A field that + * was already processed is processed again only if this path includes + * something the earlier paths did not, so its related resources get the + * deeper include. + */ + private function fieldProcessed(string $key, Field $field, ?array $include): bool { - if (in_array($field, $this->processedFields[$key] ?? [], true)) { - return true; + // false marks a field processed without an include + $processed = $this->processedFields[$key][$field->name] ?? null; + + if ($processed !== null) { + if ( + $include === null + || $include === $processed + || $processed !== false && $this->includeCovers($processed, $include) + ) { + return true; + } + + $include = $this->mergeInclude($processed ?: [], $include); } - $this->processedFields[$key][] = $field; + $this->processedFields[$key][$field->name] = $include ?? false; return false; } + private function includeCovers(array $processed, array $include): bool + { + foreach ($include as $name => $nested) { + if (!isset($processed[$name]) || !$this->includeCovers($processed[$name], $nested)) { + return false; + } + } + + return true; + } + + private function mergeInclude(array $processed, array $include): array + { + foreach ($include as $name => $nested) { + $processed[$name] = $this->mergeInclude($processed[$name] ?? [], $nested); + } + + return $processed; + } + private function resolveDeferred(): void { $i = 0; diff --git a/tests/specification/InclusionOfRelatedResourcesTest.php b/tests/specification/InclusionOfRelatedResourcesTest.php index eb59e17..88f22bb 100644 --- a/tests/specification/InclusionOfRelatedResourcesTest.php +++ b/tests/specification/InclusionOfRelatedResourcesTest.php @@ -378,4 +378,99 @@ public function test_relationship_inclusion_for_polymorphic_relationship() $response->getBody(), ); } + + public function test_deeper_include_on_resource_already_reached_through_shallower_path() + { + $image = (object) ['id' => '1']; + $user = (object) ['id' => '1', 'avatar' => $image]; + + $this->api->resource(new MockResource('images', models: [$image])); + + $this->api->resource( + new MockResource( + 'people', + models: [$user], + fields: [ToOne::make('avatar')->type('images')->includable()], + ), + ); + + $this->api->resource( + new MockResource( + 'posts', + models: [(object) ['id' => '1', 'author' => $user, 'editor' => $user]], + endpoints: [Show::make()], + fields: [ + ToOne::make('author')->type('people')->includable(), + ToOne::make('editor')->type('people')->includable(), + ], + ), + ); + + $response = $this->api->handle( + $this->buildRequest('GET', '/posts/1?include=author,editor.avatar'), + ); + + $this->assertEqualsCanonicalizing( + ['people:1', 'images:1'], + $this->includedKeys($response->getBody()), + ); + } + + public function test_sibling_includes_on_resource_reached_through_two_paths() + { + $image = (object) ['id' => '1']; + $banner = (object) ['id' => '2']; + $team = (object) ['id' => '1', 'logo' => $image, 'banner' => $banner]; + $user = (object) ['id' => '1', 'team' => $team]; + + $this->api->resource(new MockResource('images', models: [$image, $banner])); + + $this->api->resource( + new MockResource( + 'teams', + models: [$team], + fields: [ + ToOne::make('logo')->type('images')->includable(), + ToOne::make('banner')->type('images')->includable(), + ], + ), + ); + + $this->api->resource( + new MockResource( + 'people', + models: [$user], + fields: [ToOne::make('team')->type('teams')->includable()], + ), + ); + + $this->api->resource( + new MockResource( + 'posts', + models: [(object) ['id' => '1', 'author' => $user, 'editor' => $user]], + endpoints: [Show::make()], + fields: [ + ToOne::make('author')->type('people')->includable(), + ToOne::make('editor')->type('people')->includable(), + ], + ), + ); + + $response = $this->api->handle( + $this->buildRequest('GET', '/posts/1?include=author.team.logo,editor.team.banner'), + ); + + $this->assertEqualsCanonicalizing( + ['people:1', 'teams:1', 'images:1', 'images:2'], + $this->includedKeys($response->getBody()), + ); + } + + private function includedKeys(string $body): array + { + return array_map( + fn($resource) => $resource['type'] . ':' . $resource['id'], + json_decode($body, true)['included'] ?? [], + ); + } }