Problem/Motivation

While using default_content, it seems that information on certain entity reference fields is not being normalized and denormalized. The example in question is a file reference field which is missing the "display" and "description" keys. On reimporting of the content, a SQL error is thrown at the DB level due to the missing information.

Proposed resolution

Fix EntityReferenceItemNormalizer to include additional data about reference fields, not just the link to the entity itself.

Remaining tasks

Write a patch.

User interface changes

None.

API changes

REST output would change. It would be the addition of keys from what I can gather, not changing anything else.

CommentFileSizeAuthor
#11 2394063-hal-issue-11.patch2.47 KBWim Leers
#6 2394063-hal-issue-5.patch2.2 KBSam152
#4 2394063-hal-issue-4.patch2.18 KBSam152
#2 2394063-hal-issue.patch2.74 KBSam152
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

I think we're making exactly this change in #1927648: Allow creation of file entities from binary data via REST requests.

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
FileSize
2394063-hal-issue.patch2.74 KB

Attached is a proof of concept patch to fix the issue for file entities specifically. I'll post the known issues next comment.

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
Status: Active » Closed (duplicate)

Oh, did not manage to pick that issue up in the search. Won't bother going into what is broken with the patch.

Thanks :)

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
Status: Closed (duplicate) » Needs review
FileSize
2394063-hal-issue-4.patch2.18 KB

After looking into that issue, this issue aims to do a lot less. I have attached a different approach, which still seems pretty smelly, but it ensures that required keys are present on fields such as images or files during import/export.

This is not about encoding the actual file in the serialized output, but just making sure the keys required for a valid reference are present.

Status: Needs review » Needs work

The last submitted patch, 4: 2394063-hal-issue-4.patch, failed testing.

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
Status: Needs work » Needs review
FileSize
2394063-hal-issue-5.patch2.2 KB
Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
Title: Include field values in EntityReferenceItemNormalizer, not just link to the entity. » Field properties are not normalized or denormalized on entity reference items.
Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
Issue tags: +Needs tests

mgifford queued 6: 2394063-hal-issue-5.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 6: 2394063-hal-issue-5.patch, failed testing.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs work » Needs review
FileSize
2394063-hal-issue-11.patch2.47 KB

Free reroll.

Status: Needs review » Needs work

The last submitted patch, 11: 2394063-hal-issue-11.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Grayside’s picture

Grayside CreditAttribution: Grayside at Phase2 commented

I have found this problem to also obfuscate the title, alt, width, and height attributes on the ImageItem field type. This creates a blocker for management of image accessibility for headless use cases.

Grayside’s picture

Grayside CreditAttribution: Grayside at Phase2 commented
Status: Needs work » Needs review

Let's retry those tests.

The last submitted patch, 2: 2394063-hal-issue.patch, failed testing.

Grayside’s picture

Grayside CreditAttribution: Grayside at Phase2 commented 
+++ b/core/modules/hal/src/Normalizer/EntityReferenceItemNormalizer.php
@@ -103,9 +120,15 @@ protected function constructValue($data, $context) {
+      $constrcuted = array('target_id' => $id);
...
+          $constrcuted[$property_name] = $data[$property_name];
...
+      return $constrcuted;

Consistent typo. => constructed

Grayside’s picture

Grayside CreditAttribution: Grayside at Phase2 commented

Note that this solution sidesteps Typed Data, so any modules dynamically generating API payload descriptions (Schemata) will not be aware of these properties.

Furthermore, the current implementation is adding the alt text and so on as properties, rather than fields, which makes for a "top-level" inconsistency in the JSON, since every other element is an array.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Closed (duplicate)
Issue tags: -Needs tests
Related issues: +#2825487: Fix normalization of File entities: file entities should expose the file URL as a computed property on the 'uri' base field

Note that this solution sidesteps Typed Data, so any modules dynamically generating API payload descriptions (Schemata) will not be aware of these properties.

That makes this approach a dead end IMO.

The example in question is a file reference field which is missing the "display" and "description" keys.

The solution in that case is to provide a FileItemNormalizer, because class FileItem extends EntityReferenceItem.

This patch/issue is proposing to modify EntityReferenceItemNormalizer, which is the wrong place to do this. We have #2825487: Fix normalization of File entities: file entities should expose the file URL as a computed property on the 'uri' base field for this.