Automated Drupal 11 compatibility fixes for jsonapi_schema

This is an automated patch generated using Upgrade Status and Drupal Rector. Please see the issue summary for more details. A merge request is also openend and updated.

It is important that any automated tests available are run and that you manually test the changes.

Drupal 11 Compatibility

According to the Upgrade Status module → these changes make this module compatible with Drupal 11! 🎉
Therefore these changes update the info.yml file for Drupal 11 compatibility.

Leaving this issue open, even after committing the current patch, will allow the Project Update Bot → to post additional Drupal 11 compatibility fixes as they become available in Drupal Rector.

Debug info

This patch was created using these packages:

1. drupal/upgrade_status: 4.1.0
2. mglaman/phpstan-drupal: 1.2.7
3. palantirnet/drupal-rector: 0.20.1

Patch worked for me on a Drupal 10.3.6 site. Setting to RTBC.

I wonder if we can get this merged, and tag a "stable" 1.0? The last release was made 16 month ago. I know there's a lot of interesting things happening in core for Drupal 11 (see ✨ Introduce "schematic" normalizers Active ) but it seems that there is still some followup work necessary, and this module is still needed right now for users moving to Drupal 11. It is currently a blocker for farmOS: 🌱 Update Drupal core to 11.x in farmOS Active

Patch seems to be working for me on a Drupal 10.3.14 site.

Thanks for testing @onfire84 and @alexander-tallqvist.

Let's get confirmation that this works on Drupal 11. Setting back to Needs Review.

I tested installing on Drupal 11.1.5 and it failed with the following error:

$ drush en jsonapi_schema

Fatal error: Declaration of Drupal\jsonapi_schema\Normalizer\DataDefinitionNormalizer::normalize($entity, $format = null, array $context = []) must be compatible with Symfony\Component\Serializer\Normalizer\NormalizerInterface::normalize(mixed $data, ?string $format = null, array $context = []): ArrayObject|array|string|int|float|bool|null in /opt/repos/jsonapi_schema/src/Normalizer/DataDefinitionNormalizer.php on line 53
 [warning] Drush command terminated abnormally.

I had to fix this in farmOS as well, so I'm familiar with the solution. I'll work on this...

@m.stena Probably related to this: https://www.drupal.org/project/jsonapi_schema/issues/3504440 🐛 PHP8.3 compatibility issues Active

Fatal error: Declaration of Drupal\jsonapi_schema\Normalizer\DataDefinitionNormalizer::normalize($entity, $format = null, array $context = []) must be compatible with Symfony\Component\Serializer\Normalizer\NormalizerInterface::normalize(mixed $data, ?string $format = null, array $context = [])

I think fixing this is going to necessitate dropping support for Drupal 10.

@bradjones1: I see that you have started a 2.x (and 2.0.x) branch that makes some of these changes already. Should I base a MR off of one of those? I can't find any mention of them in the issue queue, and there are a number of other changes. Any guidance you can provide on your intentions would be appreciated! I want to help push this forward, but don't want to step on toes or duplicate efforts.

@m.stena Probably related to this: https://www.drupal.org/project/jsonapi_schema/issues/3504440 🐛 PHP8.3 compatibility issues Active

Thanks @alexander-tallqvist!

This error is actually a result of changes in the upstream Symfony class that we extend from. So I don't think it's specific to PHP 8.3.

We can probably close 🐛 PHP8.3 compatibility issues Active as a duplicate of this issue, and incorporate the MR changes into here. I'll start a new MR and cherry-pick them over.

Opened a merge request with @volha_si's commit from 🐛 PHP8.3 compatibility issues Active (I updated the commit message to be more descriptive of its purpose, but kept the author credit).

I also added a commit that drops support for Drupal 8, 9, and 10. So this will need to be a new major release (not on the 8.x-1.x branch) - pending @bradjones1's response to my question above (#10).

I will do more testing locally and report back if I find any other issues.

Not working yet...

I tried to view a schema endpoint (/api/[entity-type]/[bundle]/resource/schema) in my browser and the following error occurs:

Uncaught PHP Exception Symfony\\Component\\Serializer\\Exception\\NotNormalizableValueException: "Could not normalize object of type "Drupal\\Core\\Field\\BaseFieldDefinition", no supporting normalizer found." at /opt/drupal/vendor/symfony/serializer/Serializer.php line 181

The root /api/schema endpoint and collection endpoints (/api/[entity-type]/[bundle]/collection/schema) seem to be working.

Pushed a commit that fixes the /api/[entity-type]/[bundle]/resource/schema routes.

This Drupal core change record describes the change that was necessary: https://www.drupal.org/node/3359695 →

Back to Needs Review!

Tested MR!13 on a fresh Drupal site running 11.1.5. The schemas at /api/[entity-type]/[bundle]/resource/schema seem to be working fine, and the implemented changes are in line with change records at https://www.drupal.org/node/3359695 → . Everything looks good to me! Setting to RTBTC.

Hi all,

Can we merge this MR?

Thanks

@dmitry.korhov See my comment #10 above.

Drupal 11 upgrades Symfony from version 7, which changes the method signatures in Symfony's NormalizerInterface. I think this means we need to drop support for Drupal 10.

@m.stenta
Other modules with the same issue addressed are compatible for both 10.3 and 11 versions of Drupal.
So I see no reasons of dropping support of 10.3 for this module as well. See https://git.drupalcode.org/project/rest_views/-/merge_requests/19/diffs#... ( https://www.drupal.org/project/rest_views/issues/3451283 📌 Automated Drupal 11 compatibility fixes for rest_views Active )

@dmitry.korhov It isn't the getSupportedTypes() method that's at issue, it is the normalize() method.

See this commit in the MR: https://git.drupalcode.org/issue/jsonapi_schema-3431487/-/commit/7f424f1...

(Originally cherry-picked from 🐛 PHP8.3 compatibility issues Active )

This is the fatal error that it fixes:

Fatal error: Declaration of Drupal\jsonapi_schema\Normalizer\DataDefinitionNormalizer::normalize($entity, $format = null, array $context = []) must be compatible with Symfony\Component\Serializer\Normalizer\NormalizerInterface::normalize(mixed $data, ?string $format = null, array $context = [])

Is there a way to support that in Drupal 10 and 11? I would be happy to adjust the MR if there is, but if not then I think this is the best we can do.

Hi m.stenta,

Will it work with `#[\ReturnTypeWillChange]` attribute (https://php.watch/versions/8.1/ReturnTypeWillChange)?

@dmitry.korhov: I haven't tried it, but it sounds like that is for silencing deprecation notices. In this case it's a fatal error, not a deprecation notice. So I don't think it will help.

I'm inclined to just keep things simple and make a clean break between 1.x and 2.x of this module. Is there any reason not to do that? The upgrade from D10 to D11 will be seamless either way. It just means that sites will need to update their version constraint in composer.json from ^1.0 to ^2.0.

I don't see a problem with @m.stenta's suggestion in #24. +1 for this approach.

Hi @m.stenta,

When a project is large and complex, the upgrade process goes beyond simply updating composer.json.

What we’ve observed is that 99% of projects are currently supported on both Drupal 10.3 and 11. According to the Drupal 10.3.0 release notes, there’s little reason not to make modules compatible with both versions:

Drupal 10.3 provides almost the same public API as Drupal 11.0, which will be released in August. The main differences between Drupal 10.3 and 11.0 are dependency changes and the removal of deprecated code. For more information, refer to the Drupal 11.0.0-beta1 release notes.

Ensuring that the module is compatible with both 10.3 and 11 allows for a smoother, step-by-step upgrade process:
1. Update all contributed modules to support both 10.3 and 11.
2. Run regression tests on the 10.3-compatible versions (installed on a Drupal 10.3 site).
3. Upgrade the core to Drupal 11.
4. Run regression tests again, this time focused only on the core.

So, if silencing a few deprecations enables broader compatibility and allows other installations to use the same module version across both Drupal 10.3 and 11, it would be greatly appreciated to make that small change. You might even test it with the deprecations silenced to confirm.

I’m inclined to just keep things simple and make a clean break between 1.x and 2.x of this module. Is there any reason not to do that? The upgrade from D10 to D11 will be seamless either way.

Of course, if this is a personal or project-specific preference, that’s totally understandable. That said, it’s worth noting that the vast majority of other modules (around 99%) support both 10.3 and 11 in their 2.x branches, which seems to be a more practical approach for most use cases.

Thanks for considering!

@dmitry.korhov: I looked closer and I think you're right that it's possible to support both Drupal 10 and 11. I mistakenly assumed that the method signature changes were a breaking change and that it wouldn't be possible to support both.

Relatedly, I've started writing automated tests for this module in a merge request in 📌 Add basic test coverage Active (which leverages 📌 Add .gitlab-ci.yml Active ). These will be very helpful for continued development and bug fixing, and will also allow us to demonstrate that it works in both Drupal 10 and 11.

There is still more work to be done to finish the first pass at automated tests, but I will rebase this issue's MR onto that one so that we can test this in parallel in the meantime.

Rebased on top of the commits from 📌 Add basic test coverage Active so that tests can run. I restored the declaration of Drupal 10 support in jsonapi_schema.info.yml. I also added a commit to ensure tests run on the latest major Drupal version (11). This was disabled in 📌 Add basic test coverage Active because they would fail.

I think we still need to declare a minimum minor version of Drupal 10 (eg: ^10.1 or ^10.2).

Tests are passing on both Drupal 10 and 11! 🎉

I think we still need to declare a minimum minor version of Drupal 10 (eg: ^10.1 or ^10.2).

As far as I can tell, the only change that requires this is the one related to https://www.drupal.org/node/3359695 → , which was introduced in Drupal 10.1. I pushed an amended commit that updates the version constraint to ^10.1.

This is ready for final review! (But of course must wait for 📌 Add basic test coverage Active to be merged.) If anyone has other concerns/considerations, now is the time to voice them! If I don't hear anything by next week then I will assume the previous RTBC applies, along with the newly restored Drupal 10 support per @dmitry.korhov's feedback.

Thanks all!

Setting this back to Needs Work because the automated tests I'm working on in 📌 Add basic test coverage Active revealed a bug that will affect Drupal 11 compatibility.

I have rebased on top of the commit in 📌 Add basic test coverage Active that causes the issue to demonstrate the failing test (notably it passes on 8.x-1.x).

There was 1 error:
1) Drupal\Tests\jsonapi_schema\Kernel\JsonApiSchemaTest::testJsonApiSchema
PHPUnit\Framework\Exception: Uncaught PHP Exception TypeError: "Drupal\jsonapi_schema\Normalizer\ComplexDataDefinitionNormalizer::normalize(): Return value must be of type ArrayObject|array|string|int|float|bool|null, stdClass returned" at /builds/project/jsonapi_schema/src/Normalizer/ComplexDataDefinitionNormalizer.php line 51
ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

The declaration of normalize() return types revealed a bug with our current code. When the field module is enabled, there are cases where normalize() attempts to return a stdClass object, which is not allowed. I don't know the exact cause of this yet, but we'll have to figure it out before this can be merged.

This existing issue seems to be closely related: 🐛 Decimal field causes error message on custom entity Active

This is where the stdClass object is coming from: https://git.drupalcode.org/project/jsonapi_schema/-/blob/ed083e4274feec8...

It appears that the (object) cast was originally added in this commit (which created the normalizers): 08a4b2f2 (from #3060299: Remove TypeMapper plugins in favor of normalizers → ).

... despite the fact that the parent method's docblock declares a return type of array: https://git.drupalcode.org/project/jsonapi_schema/-/blob/08a4b2f2b930a6a...

It appears @ tried to remove this in cd73c2f2, but then reverted that in d30041d3

So we have some things to detangle here... :-/

I rebased this branch on top of latest 8.x-1.x (with merged 📌 Add basic test coverage Active ), and added a commit that changes DataDefinitionUndefinedNormalizer::extractPropertyData() so that it returns an array instead of an object. The declared return type is array, so it was wrong for it to return an object, and now that causes a fatal error with Symfony 7+.

Tests are all green (except PHPStan, which will be fixed separately: 📌 Fix PHPStan errors Active ). Setting this back to "Needs Review". Please review this most recent commit and set to RTBC so we can get this merged! :-)

Looks good