Config validation: config entities should get the same validation errors when validated as plain config vs ConfigEntityAdapter

Tests pushed.

This blocks 📌 Deprecate \Drupal\ckeditor5\Plugin\Editor\CKEditor5::validatePair() and `type: ckeditor5_valid_pair__format_and_editor` Active as well as many other issues, such as ✨ [PP-4] JSON:API DELETE support for config entities Active . And #3401925: After a recipe is successfully applied, validate all of the config that was imported or modified by config actions → in the Recipes initiative.

AFAICT

1. Ensure validation using either approach yields the same results

is actually impossible, because ImmutablePropertiesConstraintValidator requires a config object rather than an array to work: it is otherwise impossible to know the original ID (to verify no disallowed property mutation is occurring). And config entities cannot be loaded by UUID; otherwise that'd be a viable alternative.

So … switching to approach #2: 😞

Actually, approach #2 is not a viable approach, because it prevents the validation that is executed by ConfigSchemaChecker during ConfigInstaller. So we MUST have both. Fortunately, I found a way forward (ironically, thanks to reading the code in ConfigInstaller 😄🥳👍).

There's two things to still iron out:

1. The one @todo
2. Figure out how/why the ContentTranslationSynchronizedFields constraint is being added to config entity types 🤯

99% if not 100% of the failing kernel tests will be fixed by the todo other than the one I just added.

One of the two @todos is solved. The MR is now 99.9% green.

Some comments, but I think I see the nature of the problem and why this fix is so important.

What gives me pause is the fact that these two "sides" of validation -- config schema, and ConfigEntityAdapter -- have to stay actively in sync with each other -- means we haven't necessarily fixed this problem, just mitigated it, or moved it to where it's less likely to cause trouble (not to mention added test coverage). That's probably good enough to get past the critical problem, but to me it still smacks of the "more than one way of doing things, even important things" problem.

Not sure if I have any ideas on how to resolve that once and for all...but that's not necessarily the goal of this issue anyway. Just thought I'd call it out and be transparent about my thinking.

just mitigated it

Interesting take!

to me it still smacks of the "more than one way of doing things, even important things" problem.

This is definitely true.

Overall, I think @phenaproxima is saying that he is not fully satisfied with the solution in that The Proper Solution would be to not have two distinct mechanisms. I echo that desire but … the fact/reality is that all this wasn't fully thought through when config schema was added to Drupal core. The only way to eventually get to such a point where there's only one way, is to first make it all validatable, and then introduce a deprecation that disallows one or the other.

Meanwhile, before we get to such a HUGE refactoring, let's get things that we have today working :)

Nope, we're in full agreement. You know me...I like pragmatism.

I agree with this being a good stop gap solution, we can improve the situation further down the line but this is already a good improvement and we have the test coverage to prove that this is an improvement.
I found one extra nitpick.

I processed the feedback at a high level, but I won't have time to push this forward. Would be great if one of you could push it forward 🙏

Wrote a change record. It's a bit punchier than my usual style, but hopefully it's clear.

It's a bit punchier than my usual style

🤣

Clarified the CR.

Reviewed and removed one obsolete @todo. I +1'd your request for a clarifying comment in one place (could you add it? 🙏) and added remarks on the trickier bits of the MR.

I cannot RTBC this since I wrote ~99% of this, but I think that once you agree this makes sense, that you can still RTBC it, @phenaproxima: you've just implemented your own review remarks, but I still wrote ~99%, so should be fine :)

Looks great to me.

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.

Merged in upstream changes.

This conflicted with 📌 Use cache collector for state Needs review (for updating expectations in StandardPerformanceTest) and with 🐛 Install profile is disabled for lots of different reasons and core doesn't allow for that Fixed (for updating ModuleInstaller).

This seemed like a trivial conflict resolution, but let's await test results first.

A single kernel test and a single functional test failed — looks like I resolved the conflict with 🐛 Install profile is disabled for lots of different reasons and core doesn't allow for that Fixed incorrectly after all.

https://www.drupal.org/project/experience_builder → is why I haven't had time for this 😬