Skip to content

Fix JSON:API collection included schema#8190

Open
Will-thom wants to merge 1 commit into
api-platform:4.3from
Will-thom:fix/jsonapi-openapi-included
Open

Fix JSON:API collection included schema#8190
Will-thom wants to merge 1 commit into
api-platform:4.3from
Will-thom:fix/jsonapi-openapi-included

Conversation

@Will-thom
Copy link
Copy Markdown

Fixes #7956.

Preserves the JSON:API included schema when building OpenAPI schemas for collection responses and adds a functional regression test.

Tests:

  • php -l src/JsonApi/JsonSchema/SchemaFactory.php
  • php -l tests/Functional/OpenApiTest.php
  • vendor/bin/phpunit tests/Functional/OpenApiTest.php --filter testJsonApiCollectionSchemaDocumentsIncludedResources
  • vendor/bin/phpunit tests/Functional/OpenApiTest.php --filter testRetrieveTheOpenApiDocumentation
  • vendor/bin/phpunit tests/Functional/OpenApiTest.php --filter testHasSchemasForMultipleFormats
  • vendor/bin/phpunit tests/Functional/PaginationDisabledTest.php --filter testSchemaCollectionJsonApi

Comment on lines +262 to +276
$collectionProperties = [
'data' => [
'type' => 'array',
'items' => $properties['data'],
],
];

if (isset($properties['included'])) {
$collectionProperties['included'] = $properties['included'];
}

$schema['description'] = "$definitionName collection.";
$schema['allOf'] = [
['$ref' => $prefix.(false === $operation->getPaginationEnabled() ? self::COLLECTION_BASE_SCHEMA_NAME_NO_PAGINATION : self::COLLECTION_BASE_SCHEMA_NAME)],
['type' => 'object', 'properties' => [
'data' => [
'type' => 'array',
'items' => $properties['data'],
],
]],
['type' => 'object', 'properties' => $collectionProperties],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be simpler by mutating $properties['data'] in place instead of introducing a $collectionProperties intermediate. That way included (and any future top-level key returned by buildDefinitionPropertiesSchema) flows through automatically:

$properties['data'] = [
    'type' => 'array',
    'items' => $properties['data'],
];

$schema['description'] = "$definitionName collection.";
$schema['allOf'] = [
    ['$ref' => $prefix.(false === $operation->getPaginationEnabled() ? self::COLLECTION_BASE_SCHEMA_NAME_NO_PAGINATION : self::COLLECTION_BASE_SCHEMA_NAME)],
    ['type' => 'object', 'properties' => $properties],
];

Removes the explicit isset($properties['included']) branch and keeps the wrapper shape consistent with the item schema.

],
];

if (isset($properties['included'])) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: buildDefinitionPropertiesSchema returns 'included' => [...] only when relationships exist (see L357-376), so it can never be null here — isset() is safe. If the suggestion above is applied this branch goes away anyway.

@Will-thom
Copy link
Copy Markdown
Author

Addressed the review suggestion in the latest commit (f37dde8b8): the collection schema now wraps $properties['data'] in place and passes $properties through directly, so included and any future top-level keys flow through without a separate branch.

Validation was run in an ephemeral Composer/PHP container:

  • php -l src/JsonApi/JsonSchema/SchemaFactory.php
  • php -l tests/Functional/OpenApiTest.php
  • vendor/bin/phpunit tests/Functional/OpenApiTest.php --filter testJsonApiCollectionSchemaDocumentsIncludedResources — OK (1 test, 6 assertions)
  • vendor/bin/phpunit tests/Functional/OpenApiTest.php --filter testRetrieveTheOpenApiDocumentation — OK (7 tests, 119 assertions)
  • vendor/bin/phpunit tests/Functional/OpenApiTest.php --filter testHasSchemasForMultipleFormats — OK (1 test, 2 assertions)
  • vendor/bin/phpunit tests/Functional/PaginationDisabledTest.php --filter testSchemaCollectionJsonApi — OK (1 test, 8 assertions)
  • git diff --check

@soyuka soyuka changed the base branch from main to 4.3 May 23, 2026 07:05
@soyuka soyuka force-pushed the fix/jsonapi-openapi-included branch from f37dde8 to 55a4e41 Compare May 23, 2026 07:07
@Will-thom Will-thom force-pushed the fix/jsonapi-openapi-included branch from 55a4e41 to 96f8b80 Compare May 23, 2026 07:11
@Will-thom
Copy link
Copy Markdown
Author

I rebased the branch on the current main and squashed the PR changes into a conventional commit ( ix(jsonapi): include collection schema data properties) so commitlint should pass now.

I also re-ran the relevant functional coverage locally in a Composer/PHP container:

  • �endor/bin/phpunit tests/Functional/OpenApiTest.php --filter testJsonApiCollectionSchemaDocumentsIncludedResources -> OK (1 test, 6 assertions)
  • �endor/bin/phpunit tests/Functional/OpenApiTest.php -> OK (20 tests, 210 assertions)

The previous Doctrine/Laravel failures were coming from new main changes (
epositoryMethod / withCredentials) not present on the older branch base, so the rebase should address those as well.

@Will-thom Will-thom force-pushed the fix/jsonapi-openapi-included branch from 96f8b80 to 3d988bb Compare May 23, 2026 08:42
@Will-thom
Copy link
Copy Markdown
Author

Reworked the branch onto the correct 4.3 base and force-pushed the same scoped JSON:API/OpenAPI patch. The previous doctrine-orm / doctrine-odm minimal-changes failures came from accidentally rebasing on main, which pulled newer internal APIs that are not compatible with the 4.3 dependency matrix.

Local validation passed in an isolated container:

vendor/bin/phpunit tests/Functional/OpenApiTest.php --filter testJsonApiCollectionSchemaDocumentsIncludedResources
vendor/bin/phpunit tests/Functional/OpenApiTest.php

Results: focused test OK with 1 test / 6 assertions; full OpenApiTest OK with 20 tests / 210 assertions. The refreshed CI run is now green as well.

@Will-thom
Copy link
Copy Markdown
Author

The branch is now on the correct 4.3 base and the refreshed CI run is green. Please let me know if anything else is needed from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSON:API included compound documents no longer represented in OpenAPI output in 4.3.x

2 participants