[PP-2] POST/PATCH config entities via REST for config entity types that support validation

Hard-blocked on 📌 Split validation-related methods from FieldableEntityInterface into a separate ValidatableInterface Needs work and soft-blocked on 🌱 [meta] Add constraints to all config entity types Active . At least one of the children of #2869792 needs to be finished before we can do this.

At least one of the children of #2869792 needs to be finished before we can do this.

Great news: two of those children are done: system.menu.* (Menu config entities) and shortcut.set.* (ShortcutSet config entities) are fully validatable!

Even though we should land 📌 Split validation-related methods from FieldableEntityInterface into a separate ValidatableInterface Needs work before this, that does not mean it should stop this issue from creating a PoC for how JSON:API would selectively expose config entities: only those config entities that are actually validatable would be allowed to be modified.

I'd love to sprint on this during DrupalCon Lille 🤓

This is older and has more issues.

This likely would be impacted by/simplified by 📌 Follow-up for #3361534: config validation errors in contrib modules should not cause deprecation notices, unless they opt in Active .

See #3395099-33: [meta] Allow config types to opt in to config validation, use "FullyValidatable" constraint at root as signal to opt in to "keys/values are required by default" → … this is now unblocked!

Wim Leers → changed the visibility of the branch 11.x to hidden.

Yay!

• Only 5 functional tests fail: the testPostIndividual() ones for the 5 currently fully validatable config entity types (see 🌱 [meta] Add constraints to all config entity types Active ): https://git.drupalcode.org/issue/drupal-2300677/-/pipelines/99655/test_r...
• One of those 5 now has a valid JSON:API document specified to be POSTed: MenuTest. It's failing tests because it successfully created a menu config entity when it shouldn't have (it should've failed because a value was specified that does not exist on the fieldable entity). Next step: debugging this. But … we're getting somewhere 😄

POST is working, with only 2 @todos remaining.

Next up: PATCH support. Then fix one of the @todos — the other one I just proposed a solution for in a comment, but requires an API addition in ConfigManager AFAICT. Marking in case somebody has an idea/insight for that one 🤞

That failing test in the second bullet of #266 hits an interesting edge case:

• \Drupal\jsonapi\Normalizer\ConfigEntityDenormalizer::prepareInput() does not strip field_rest_test
• nor does EntityDenormalizerBase::denormalize(), because
  

      return $this->entityTypeManager>getStorage($entity_type_id)->create($this->prepareInput($data, $resource_type, $format, $context));
  

  does not transform anything

• it is \Drupal\Core\Entity\EntityBase::__construct() that causes this to happen, because it does
  

      foreach ($values as $key => $value) {
        $this->$key = $value;
      }
  

  … which results in Menu::$field_rest_test being set, but that's simply not validated!

How does this compare to content entities? Well … turns out it's actually similar: it's \Drupal\jsonapi\Normalizer\ContentEntityDenormalizer::prepareInput() that does the detection and does

        throw new UnprocessableEntityHttpException(sprintf(
          'The attribute %s does not exist on the %s resource type.',

So we just need to match that. Did that in https://git.drupalcode.org/project/drupal/-/merge_requests/6704/diffs?co....

Wim Leers → changed the visibility of the branch 2300677-postpatch-config-entities to hidden.

1. Tests are green!!! 🚀🥳 For all 5 fully validatable config entity types currently in core, the POST and PATCH test coverage is passing.
2. Repeating all the validation test coverage from the ConfigEntityValidationTestBase subclasses is pointless. Still, a spot-check makes sense. For that reason, DateFormatTest::testPostValidationErrors() exists. Compare with DateFormatValidationTest and the JSON:API NodeTest::testPostNonExistingAuthor() to verify that this is indeed a consistent DX.
3. As soon as this lands, every additional config entity type that is marked fully validatable will have its JSON:API functional tests (guaranteed to exist thanks to \Drupal\Tests\jsonapi\Kernel\TestCoverageTest) start failing (guaranteed thanks to ResourceTestBase::setUp() skipping test methods for config entities if and only if the config entity type is NOT fully validatable). That means we will be able to guarantee that everything works consistently 👍
4. Follow-up for DELETE created: ✨ [PP-4] JSON:API DELETE support for config entities Active
5. Follow-up for "relationships" and "related" created: 📌 [PP-2] JSON:API relationship/related support for config entities Active
6. Change record updated: https://www.drupal.org/node/2906029 →
7. Issue summary updated.

Next: add test coverage to verify that for fully validatable config entity types that 1) use plugins (such as Editor, FilterFormat, FieldStorageConfig, etc.), 2) those plugins have settings, 3) and creating/modifying such a config entity to use a plugin whose settings are NOT fully validatable, should result in either a 422 (like any other validation error), with a message such as The value at property path foo.plugin_settings.baz cannot be validated. Please contact the developer of the "bar" foo plugin to make this validatable.

This is blocked on either of the following child issues of 🌱 [meta] Add constraints to all config entity types Active landing:

• 📌 [PP-1] Convert field_storage_config and field_config's form validation logic to validation constraints Postponed (FieldConfig, FieldStorageConfig, BaseFieldOverride)
• ✨ [PP-1] Mark parts of CKEditor 5 and Editor config schema as fully validatable Postponed (Editor)
• 📌 [PP-2] Make FilterFormat config entities fully validatable Postponed (FilterFormat
• 📌 Add config validation for workflow entities. Active (Workflow
• … or a yet-to-be-created issues for other qualifying config entity types that are not yet fully validatable

There are a few @todos outstanding, but each of those are for clean-up or contain proposed solutions, so this is ready for review despite still being blocked — that blocker is only for making this have watertight security 👍

Document which API changes/additions were needed to make this a reality.

Hid all patches. Last one was from >5 years ago. MRs were a distant dream back then 😄

(document.querySelectorAll('#edit-field-issue-files-und-table input[type=checkbox]').forEach(el => el.checked = false);)

Removed 2 related issues:

• #1916302: RFC 7807: "Problem Details for HTTP APIs" — serve REST error responses as application/problem+json → — only relevant for REST, not JSON:API.
• 📌 Prevent the deletion of a bundle entity if there are existing content entities of that bundle Needs work — only relevant for a follow-up now: ✨ [PP-4] JSON:API DELETE support for config entities Active

Furthermore, #2907163 is a just nice-to-have rather than a hard blocker now. See #2907163-50: Split validation-related methods from FieldableEntityInterface into a separate ValidatableInterface → . That was proposed before ConfigEntityAdapter landed a year after #198 (in #1818574: Support config entities in typed data EntityAdapter → ).

Needs further investigation: #2905594: Missing entity validation constraint: don't allow new entities when there is an existing one with the same ID → by @dawehner in #181. This might be a bit of missing test coverage. Updated IS.

🏁 Finished triaging all related issues.

This is ready for review now! 🚀 Issue is fully cleaned up just before it became a teenager 😅

📌 Allow vocabularies to be validated via the API, not just during form submissions RTBC landed. Merged origin/11.x; VocabularyTest should fail now 🤓

Failed as expected!

✨ [PP-1] Mark parts of CKEditor 5 and Editor config schema as fully validatable Postponed also landed, so now I'm definitely unblocked on next steps here 👍

Now a 422 response is generated when a subtree of the config is not actually fully validatable. That is one of the nastiest/trickiest aspects we need to handle here. It's proven to be working now, but the exact messaging is still TBD.

But … that means this is now definitely ready for review 🥳

Insanity. In a good way. ;)

I've gone through the code and did some comments. It was hard to poke real holes into this. There are a few areas we do need work, but those are mostly marked with the todo's.

Addressed one thing @bbrala spotted, answered some questions and … asked some questions that hopefully a config system maintainer will respond to 😄

Gone through all responses and think a few more can be closed @Wim Leers :) Thanks for the awsers.

Note: Wim asking for feedback from someone with config experience (or maintainer there). Quoting for visibility:

https://git.drupalcode.org/project/drupal/-/merge_requests/6704#note_305097

    // @todo this is inaccurate — it won't work for e.g. `field.field.*.*.*`. We
    //   need the inverse of \Drupal\Core\Config\ConfigManager::getEntityTypeIdByName()
    //   To build that, we would need to expand the `@ConfigEntityType`
    //   annotation with a "id_parts" key-value pair, which defaults to 1, and
    //   FieldConfig, FieldStorageConfig, EntityViewMode etc. would specify.
    $config_schema_type_name = $entity_type->getConfigPrefix() . '.*';
    $config_schema_type_definition = \Drupal::service('config.typed')->getDefinition($config_schema_type_name);

📌 Improve JSON:API test failure messages to include errors when data is expected Fixed landed today and conflicted. Resolved.

I will do a live demo of this in my DrupalCon Portland talk next Monday. 🤓 So here's a backport to 10.3.x (against 0da9964174175a013f8d517cdee1ce08e590f440).

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Since this was added to the IS, there was a complete rewrite of the patch, is it still valid? Do we have to postpone this on that?

Needs further investigation: #2905594: Missing entity validation constraint: don't allow new entities when there is an existing one with the same ID by @dawehner in #181. This might be a bit of missing test coverage?

I looked at the entire MR again, since it came up in one of Wim's XB weeks: https://wimleers.com/xb-week-24
I think the merge request looks great, and I don't have any big open questions, it needs a rebase and Wim needs to close some of the open questions (that's still only possible to do for the mr author).

We also need to find an answer to the question highlighted in #282.

I'll update this MR and the comments and such.

Rebased, did need to change a few lines to adjust to return types and the changes to the base class (doTestPatchIndividual). Lets see what it tells us.

bbrala → credited casey → .

Crediting casey for helping with the config type definition puzzle.

I've updated the changerecord, went through credits. Also went through open credits and it seems all is addressed.

Now i need to investigate testing failures ;x

Test failure is unrealated. I feel this might actually be ready? o_O

Lets recap current state.

1. #2905594: Missing entity validation constraint: don't allow new entities when there is an existing one with the same ID → is still mentioned in the IS as something to investigate.
2. EntityValidationTrait.php:111: @todo Add recursion … or add a helper to TypedConfigManager?
4. ResourceTestBase:2086: 2086: @todo Config schema assumes at least high-level type compliance, which is violated here. Harden it to provide equivalently helpful error responses.

Hmm, also see some other thing that need work. Not sure the removal of the SKIP_TESTS is doing what it should.

I'll get back to this.

Ok, tests are now properly failing on ConfigurationEntitites that are either missing a postDocument or those who are validatiable but have plugins that are not (see failing ActionTest)

#2905594: Missing entity validation constraint: don't allow new entities when there is an existing one with the same ID → is no longer needed, added a check for existing config entities by id (and fallback to uuid) to make sure they don't exist.