[core] Encode definition names to be compliant with JSON Pointer Specification #3669

stchr · 2020-08-11T09:09:23Z

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	yes
Deprecations?	no
Tickets	fixes #1969
License	MIT
Doc PR	api-platform/docs#...

~~This PR currently only urlencodes the colon : separator in schema names as this currently seems the only illegal char besides the allowed a-zA-Z0-9 _ . - in OpenAPI 3.0.~~

This PR urlencodes the whole name and additionally replacing / and ~ according to JSON Pointer Specification.

@rkopera

api-platform#3616) * Changed depth and maxDepth handling for selfreferencing subresourceOperations. Addresses: api-platform#2533. * Use single name for subpath * fix: address @rkopera comments Co-authored-by: Clemens Pflaum <clemens.pflaum@yahoo.de>

src/JsonSchema/SchemaFactory.php

soyuka · 2020-08-12T07:49:53Z

behat tests failing

…cification

stchr · 2020-08-12T12:53:14Z

behat tests failing

@soyuka Done.

soyuka · 2020-08-12T13:03:07Z

looks good, @dunglas could you check ?

stchr · 2020-08-12T13:20:09Z

@soyuka @dunglas As it comes out only the refs are affected (and not the schema definition names in general) there should be no more BC break and we could merge this in the 2.5 branch.
What do you think?

commit b5fcf46 Author: soyuka <soyuka@users.noreply.github.com> Date: Wed Aug 12 16:02:51 2020 +0200 fix phpstan commit 0cbc7e2 Author: Arnaud J <jabin@cikaba.com> Date: Sat Aug 8 16:35:09 2020 +0200 :white_check_mark: Test API subresources with custom identifiers commit d741950 Author: mahmood bazdar <mahmood.bazdar@gmail.com> Date: Mon Apr 27 15:29:01 2020 +0430 Fixing error for none database identifiers Api ids

…esource has pagination disabled

dszwcz · 2020-08-19T13:51:03Z

@soyuka @dunglas it would be great to merge it :)

* 2.5: fix: Order filter log when the property is invalid (api-platform#3673) Merge api-platform#3529 and api-platform#3671 (api-platform#3675) Check change tracking deferred for ODM (api-platform#3629) Changed depth and maxDepth handling for selfreferencing subresourceOp… (api-platform#3616) [GraphQL] No BC for types (api-platform#3654)

…ints

…atform#3684)

Following: - api-platform#3673 - api-platform#3675

dunglas · 2020-08-26T19:30:14Z

src/JsonSchema/SchemaFactory.php

+    private function encodeDefinitionName(string $name): string
+    {
+        $entities = ['%2F', '%7E'];
+        $replacements = ['~1', '~0'];


Why is it necessary? IIRC only ~ and / must be encoded, not there URL encoded versions. Or am I missing something?

Please have a look at #1969 (comment)

The characters / and ~ in node names must be encoded as ~1 and ~0 when used in a $ref, and other special characters must be percent-encoded.

dunglas · 2020-08-26T19:31:18Z

features/openapi/docs.feature

@@ -95,7 +95,7 @@ Feature: Documentation support
    And the JSON node "paths./related_dummies/{id}/related_to_dummy_friends.get.parameters" should have 6 elements

    # Subcollection - check schema
-    And the JSON node "paths./related_dummies/{id}/related_to_dummy_friends.get.responses.200.content.application/ld+json.schema.properties.hydra:member.items.$ref" should be equal to "#/components/schemas/RelatedToDummyFriend:jsonld-fakemanytomany"
+    And the JSON node "paths./related_dummies/{id}/related_to_dummy_friends.get.responses.200.content.application/ld+json.schema.properties.hydra:member.items.$ref" should be equal to "#/components/schemas/RelatedToDummyFriend%3Ajsonld-fakemanytomany"


If we introduce a BC break, shouldn't we also change to a URL-safe character as separator to make the names more human-readable?

Currently there should be no BC break as only the refs are affected, not the definition names itself.
And you prefered urlencode in #1969 (comment)

But sure we can switch to safe chars, if you like. Then we would have a BC break ;-) Which do you prefer? I guess . would be a good alternative to :?

dunglas · 2020-08-26T19:37:55Z

Actually, as pointed out by @hkosova (#1969 (comment)), the generated identifiers are still invalid according to the OpenAPI spec: All the fixed fields declared above are objects that MUST use keys that match the regular expression: ^[a-zA-Z0-9\.\-_]+$.. We must use safe characters according to this regex instead of escaping.

stchr · 2020-08-26T20:03:32Z

Actually, as pointed out by @hkosova (#1969 (comment)), the generated identifiers are still invalid according to the OpenAPI spec: All the fixed fields declared above are objects that MUST use keys that match the regular expression: ^[a-zA-Z0-9\.\-_]+$.. We must use safe characters according to this regex instead of escaping.

@dunglas Good point, missed out OpenAPI 3.0 spec. Which character do you prefer from this regex? Would go with .?

dszwcz · 2020-08-27T08:04:01Z

Actually, as pointed out by @hkosova (#1969 (comment)), the generated identifiers are still invalid according to the OpenAPI spec: All the fixed fields declared above are objects that MUST use keys that match the regular expression: ^[a-zA-Z0-9\.\-_]+$.. We must use safe characters according to this regex instead of escaping.

@dunglas Good point, missed out OpenAPI 3.0 spec. Which character do you prefer from this regex? Would go with .?

dot is ok for me

…ints-in-swagger [Swagger] Fix that subresource endpoints do not override custom endpoints

api-platform#3676 - fixed subresource pagination support if primary resource has pagination disabled

dunglas · 2020-08-27T13:14:20Z

. looks good to me too.

* fix: fix tests * Remove phpspec/prophecy-phpunit * Fix tests * Fix PHPStan * wip * WIP * Add PhpUnitPolyfillTrait * Switch to simple-phpunit in Github Actions * Use PHPUnit 9 * Try to use the good version of PHPUnit * Use the master version of PHPUnit Bridge * Remove PHPUnit from deps * Use the files from Simple PHPUnit * wip * test * test * test * improve phpstan config * improve phpstan config * Fix Behat * Fix CI * Fix some deprecations * Fix build for Symfony 5.2 * Fix Mongo deprecations * Fix AppVeyor build

* Add changelog for 2.5.7 * Update CHANGELOG.md Co-authored-by: Alan Poulain <contact@alanpoulain.eu> Co-authored-by: Alan Poulain <contact@alanpoulain.eu>

feat: bump JS deps

* 2.5: feat: bump JS deps Add changelog for 2.5.7 (api-platform#3691) fix: fix tests (api-platform#3688) Fix missing use statement (api-platform#3670) [MongoDB] Fix Behat tests (api-platform#3687) Fix missing Doctrine\Common\Persistence\ManagerRegistry class (api-platform#3684) [Swagger] Fix that subresource endpoints do not override custom endpoints api-platform#3676 - fixed subresource pagination support if primary resource has pagination disabled

* Use symfony/serializer >=4.4.9-5.0.9 to fix issues symfony/symfony#34455 symfony/symfony#36601 * Lowest + legacy test suite missing git * fix Symfony 5 router generate with reference type

…cification

# Conflicts: # CHANGELOG.md

…finition names to be compliant with OpenAPI 3.0

stchr · 2020-09-02T20:21:47Z

Messed up this PR 😞 , new and clean one is #3705
FYI @dunglas @soyuka

alanpoulain and others added 3 commits July 23, 2020 15:09

[GraphQL] No BC for types (api-platform#3654)

0a0c7c5

Check change tracking deferred for ODM (api-platform#3629)

1d45d79

stchr changed the title ~~Encode definition names to be compliant with JSON Pointer Specification~~ [core] Encode definition names to be compliant with JSON Pointer Specification Aug 11, 2020

stchr force-pushed the master branch 2 times, most recently from 1dbe47d to bd2f071 Compare August 11, 2020 10:32

stchr marked this pull request as ready for review August 11, 2020 19:39

stchr mentioned this pull request Aug 11, 2020

Swagger documentation: Could not resolve reference #1969

Closed

stchr force-pushed the master branch from bd2f071 to 23da16a Compare August 11, 2020 20:44

soyuka reviewed Aug 12, 2020

View reviewed changes

src/JsonSchema/SchemaFactory.php Show resolved Hide resolved

encode definition names in refs to be compliant with JSON Pointer Spe…

843ceea

…cification

stchr force-pushed the master branch from 23da16a to 843ceea Compare August 12, 2020 12:32

soyuka added 2 commits August 12, 2020 16:53

fix: Order filter log when the property is invalid (api-platform#3673)

26523ed

stchr mentioned this pull request Aug 17, 2020

Schema names special symbols api-platform/api-platform#1493

Open

api-platform#3676 - fixed subresource pagination support if primary r…

f9463ac

…esource has pagination disabled

dunglas and others added 4 commits August 22, 2020 19:21

[Swagger] Fix that subresource endpoints do not override custom endpo…

548c1bb

…ints

Fix missing Doctrine\Common\Persistence\ManagerRegistry class (api-pl…

19de800

…atform#3684)

[MongoDB] Fix Behat tests (api-platform#3687)

8eac1cc

Following: - api-platform#3673 - api-platform#3675

dunglas reviewed Aug 26, 2020

View reviewed changes

Fix missing use statement (api-platform#3670)

99ecd5b

dunglas added 2 commits August 27, 2020 11:54

Merge pull request api-platform#3685 from GSadee/fix-overriding-endpo…

84f8f7e

…ints-in-swagger [Swagger] Fix that subresource endpoints do not override custom endpoints

Merge pull request api-platform#3678 from nightio/fix/3676

ae350a9

api-platform#3676 - fixed subresource pagination support if primary resource has pagination disabled

dunglas and others added 9 commits August 28, 2020 14:00

Add changelog for 2.5.7 (api-platform#3691)

39651ec

* Add changelog for 2.5.7 * Update CHANGELOG.md Co-authored-by: Alan Poulain <contact@alanpoulain.eu> Co-authored-by: Alan Poulain <contact@alanpoulain.eu>

feat: bump JS deps

32d5e8c

Merge pull request api-platform#3693 from dunglas/feat/upgrade-js-deps

37b5edf

feat: bump JS deps

Fix tests (api-platform#3698)

2c87089

* Use symfony/serializer >=4.4.9-5.0.9 to fix issues symfony/symfony#34455 symfony/symfony#36601 * Lowest + legacy test suite missing git * fix Symfony 5 router generate with reference type

encode definition names in refs to be compliant with JSON Pointer Spe…

aba9894

…cification

Merge remote-tracking branch 'origin/master'

7b89e7c

# Conflicts: # CHANGELOG.md

Replace all characters other than ^[a-zA-Z0-9\.\-_]+$. to . in de…

0f95763

…finition names to be compliant with OpenAPI 3.0

stchr mentioned this pull request Sep 2, 2020

Fix definition names to be compliant with OpenAPI 3.0 and JSON Point Spec #3705

Merged

stchr closed this Sep 2, 2020

stchr deleted the master branch February 5, 2021 21:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[core] Encode definition names to be compliant with JSON Pointer Specification #3669

[core] Encode definition names to be compliant with JSON Pointer Specification #3669

Uh oh!

stchr commented Aug 11, 2020 •

edited

Loading

Uh oh!

Uh oh!

soyuka commented Aug 12, 2020

Uh oh!

stchr commented Aug 12, 2020

Uh oh!

soyuka commented Aug 12, 2020

Uh oh!

stchr commented Aug 12, 2020

Uh oh!

dszwcz commented Aug 19, 2020

Uh oh!

dunglas Aug 26, 2020

Uh oh!

stchr Aug 26, 2020 •

edited

Loading

Uh oh!

dunglas Aug 26, 2020

Uh oh!

stchr Aug 26, 2020 •

edited

Loading

Uh oh!

dunglas commented Aug 26, 2020

Uh oh!

stchr commented Aug 26, 2020

Uh oh!

dszwcz commented Aug 27, 2020

Uh oh!

dunglas commented Aug 27, 2020

Uh oh!

stchr commented Sep 2, 2020 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

[core] Encode definition names to be compliant with JSON Pointer Specification #3669

[core] Encode definition names to be compliant with JSON Pointer Specification #3669

Uh oh!

Conversation

stchr commented Aug 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

soyuka commented Aug 12, 2020

Uh oh!

stchr commented Aug 12, 2020

Uh oh!

soyuka commented Aug 12, 2020

Uh oh!

stchr commented Aug 12, 2020

Uh oh!

dszwcz commented Aug 19, 2020

Uh oh!

dunglas Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

stchr Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dunglas Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

stchr Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dunglas commented Aug 26, 2020

Uh oh!

stchr commented Aug 26, 2020

Uh oh!

dszwcz commented Aug 27, 2020

Uh oh!

dunglas commented Aug 27, 2020

Uh oh!

stchr commented Sep 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

stchr commented Aug 11, 2020 •

edited

Loading

stchr Aug 26, 2020 •

edited

Loading

stchr Aug 26, 2020 •

edited

Loading

stchr commented Sep 2, 2020 •

edited

Loading