Problem/Motivation

The work done on #3048348: Denormalizing NULL for an optional @FieldType=address or @FieldType=geolocation field fails due to either no main property name or computed read-only main property result on a failure on automated test where the db storage is PostgreSQL.

See test failures against 8.8.x and 8.7.x (patch to be commited yet).

Failure message is:

1) Drupal\Tests\jsonapi\Functional\JsonApiRegressionTest::testMapFieldTypeNormalizationFromIssue3040590
Failed asserting that null is identical to Array &0 (
'foo' => 'bar'
).

/var/www/html/core/modules/jsonapi/tests/src/Functional/JsonApiRegressionTest.php:913

Proposed resolution

Fix the problem.

Remaining tasks

  1. Investigate why this is happening
  2. Write a fix for the issue
  3. Provide tests

User interface changes

No.

API changes

Not expected.

Data model changes

Not expected.

Release notes snippet

Not expected.

CommentFileSizeAuthor
#3 3078639-3.patch762 bytesgabesullice
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gambry created an issue. See original summary.

gambry’s picture

gambry
Primary language Italian
Location Milan
CreditAttribution: gambry at manifesto commented

This is critical as being a bug which Cause tests to fail in HEAD on the automated testing platform for a supported environment (policy).

gabesullice’s picture

gabesullice
Primary language English
CreditAttribution: gabesullice at Acquia commented
Issue tags: +API-First Initiative
FileSize
3078639-3.patch762 bytes

Validating a hunch...

I don't yet have Postgres set up for testing, so let's let the testbot do it.

gabesullice’s picture

gabesullice
Primary language English
CreditAttribution: gabesullice at Acquia commented
Status: Active » Needs review

The tests haven't finished, but looking at the console output I can see that JsonApiRegressionTest passed. I think the hunch was right: Postgres is probably sorting by "last updated" while MySQL is sorting by ID when no sort is explicitly provided. By adding a sort query parameter to the request URL we can side-step the implementation difference.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Entity Field API

Makes sense! I would've expected the Entity Field Query API would have handled this distinction for us automatically, but it's understandable that it does not.

The work-around is very reasonable. Thanks for getting to the bottom of this, @gabesullice! 👏

gambry’s picture

gambry
Primary language Italian
Location Milan
CreditAttribution: gambry at manifesto commented

Amazing found out. I honestly thought the whole segment $data['data'] of $data['data'][0]['attributes']['data'] was missing, presenting a more catastrophic scenario... then just a sorting issue.
Makes a lot of sense. Thanks @gabesullice . Would you update patch on #3048348: Denormalizing NULL for an optional @FieldType=address or @FieldType=geolocation field fails due to either no main property name or computed read-only main property as well? I believe there won't be any more release on 8.7.x, so it's probably pointless to update the patch in there. 8.8.x will happen in December, so there is plenty of time.

  • catch committed 5cd4900 on 8.8.x 
    Issue #3078639 by gabesullice, gambry: PostgreSQL automated test failing...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 5cd4900 and pushed to 8.8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.