Problem/Motivation

Right now we are treating the configuration entities as string properties outputting only their ID. We want to have full GET support for configuration entities.

Proposed resolution

Modify the normalizers to give support for the configuration entities so we can:

  • GET individual configuration entities.
  • GET collections of configuration entities.
  • Include config entities in the returned payload.
  • Set sparse fieldsets to select the output.

Remaining tasks

Identify the bigger tasks and put them in their own follow up issue so they can be tackled separately.

Identified tasks so far:

  • Filter collections of configuration entities.
CommentFileSizeAuthor
#5 2733097--interdiff--5-3.txt1.97 KBe0ipso
#5 2733097--config-entities--5.patch17.21 KBe0ipso
#3 2733097--config-entities--3.patch16.79 KBe0ipso

Comments

e0ipso created an issue. See original summary.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Active » Needs work

Starting to work on this.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs work » Needs review
StatusFileSize
new 2733097--config-entities--3.patch16.79 KB

Sparse fieldsets and filters will be added separately in follow ups.

gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs review » Needs work

This is super exciting! Great work!

  1. +++ b/src/Normalizer/ConfigEntityNormalizer.php
@@ -0,0 +1,59 @@
+ * Converts the Drupal entity object structure to a HAL array structure.

    Is this correct? 

  2. +++ b/src/Normalizer/ConfigEntityNormalizer.php
@@ -0,0 +1,59 @@
+  protected $supportedInterfaceOrClass = ConfigEntityInterface::class;

    Usually I see this as ::CLASS 

  3. +++ b/src/Normalizer/ConfigEntityNormalizer.php
@@ -0,0 +1,59 @@
+    /* @var \Drupal\Core\Config\Entity\ConfigEntityInterface $entity */

    Is there a reason we're not hinting this in the main docblock? 

  4. +++ b/src/Normalizer/ContentEntityNormalizer.php
@@ -74,30 +65,20 @@ class ContentEntityNormalizer extends NormalizerBase implements ContentEntityNor
+      $fields_names = $this->getFieldNames($entity);

    Nit: field_names 

  5. +++ b/src/Normalizer/ContentEntityNormalizer.php
@@ -128,4 +104,59 @@ class ContentEntityNormalizer extends NormalizerBase implements ContentEntityNor
+    /* @var \Drupal\Core\Entity\ContentEntityInterface $entity */

    Same as above. 

  6. +++ b/src/Normalizer/ContentEntityNormalizer.php
@@ -128,4 +104,59 @@ class ContentEntityNormalizer extends NormalizerBase implements ContentEntityNor
+    /* @var \Drupal\Core\Entity\ContentEntityInterface $entity */

    ditto. I'm not saying it's wrong, I'm really just curious what the reason is ;)

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
StatusFileSize
new 2733097--config-entities--5.patch17.21 KB
new 2733097--interdiff--5-3.txt1.97 KB

2. Doing a quick grep on core's code I found hundreds of examples of ::class but only 2 of ::CLASS.
3., 5. and 6. Yes there is a reason. It is because of polymorphism. We are inheriting from a common class with ContentEntityNormalizer and the method gets different objects there.

The feedback is implemented now.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs work » Needs review
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

Merging, as this is a blocker for #2734755: [FEATURE] Include support for sparse fieldsets in configuration entities. As usual if you have any other concern, let's create a follow up.

  • e0ipso committed b86f097 on 8.x-1.x 
    Issue #2733097 by e0ipso: [FEATURE] Add GET support for configuration...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Closed (fixed) » Active

FYI: #2724823: EntityResource: read-only (GET) support for configuration entities just landed. e0ipso wrote in there that he would follow up on this issue once that landed. So, reopening this issue to create the appropriate follow-up.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2724823: EntityResource: read-only (GET) support for configuration entities
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

Many thanks @Wim Leers! I'll make sure to add a follow up to have a spike to see what can be leveraged from REST core.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Active » Fixed

cleanup

Status: Fixed » Closed (fixed)

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