#Configuration schema

FYI, picking this up because it became more important since 📌 Decouple image shape matching from the `image` MediaType Active , and now soon also 📌 Support image shape matching from the `image` MediaSource: support both the `image` + `oembed` MediaSources simultaneously Active — we need to be able to be as confident as possible that PropExpressions stored in XB's config are valid.

@thoward216 is out today, I'd like to land something nice & simple for once before I go on PTO 😇

Oops, during the rebase on top of 1.x I apparently somehow erased the authorship by @libbna! 😅 Crediting them already to ensure they are recognized for what they contributed :)

Asked @larowlan to double-check my security concerns, and per https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... seems like he's +1 😅

After rebasing and uncommenting the schema changes as mentioned in #13, there are a number of failing tests where the validation messages are not as expected.

An example: 'versioned_properties' => 'This value should satisfy at least one of the following constraints: [1] This value should be of the correct primitive type. [2] This value should be of the correct primitive type.'

To me this doesn't look right and the message is also duplicated. I spent some time investigating a bit further with @f.mazeikis we're not sure that the constraint AtLeastOneOf can be used with sequences? Looking at the core test AtLeastOneOfConstraintValidatorTest it only ever tests with strings or integers.

I'm closing the ticket as we are cleaning up the backlog to start fresh, and after a quick review of the code, it no longer seems relevant.
Feel free to reopen if this was a mistake on my part.

Finished reviewing the MR. Epic work! 🤩 Confirmed that it meets all 5 goals outlined in the issue summary! 👍

My commits/self-addressed:

• Comment nits.
• A fix for using root-relative URLs for the example images rather than absolute ones

Remaining feedback:

• Spotted a few bits of missing test coverage.
• Everywhere this MR uses "default", it should use "example", because of 🐛 DX & authoring experience: for SDC+code components, the example is treated as the default, may not be desirable Active . (Yes, I know this is how we've been talking about it for a while, but that doesn't mean we shouldn't realize our mistake and get better. Not your fault, but this MR understandably introduces a lot more uses of this terminology! So: time to rectify it now.)
• Concern: manually creating a File entity in a test — what am I missing?
• Major concern: the way this paves the path for ✨ [PP-1] Add support for example images for Code Components' image props Postponed is nice but AFAICT insecure 😅 — I think it actually might be better to REMOVE this bit from the MR, because of the security aspects, but also because it's not in scope for this issue (for that very reason).
• In fact, a similar concern exists when just using encoded_files itself; a config export+import could become an attack vector. See https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... + https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... + https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

Thanks, @larowlan for creating 📌 [PP-1] Consider adding ramsey/uuid for hash-based UUID (v5) generation for default files Postponed: needs info — linked to it 👍

Thanks for the excellent docs addition, that made it 10x easier to review this! Still in the process of doing so.

Created ✨ [PP-1] Add support for example images for Code Components' image props Postponed for the actual feature that this is intended to unblock.

FYI: just landed 📌 Disallow component trees with `component_version: active` Active ; the @todo was corrected there — the AtLeastOneOf didn't belong on active_version, but on versioned_properties.

OR the original constraint that was commented to add later maybe stale?

Bingo! Turns out this was independently of this (low-priority) issue discovered while investigating a bug — see 📌 Disallow component trees with `component_version: active` Active , which is about to remove that @todo 🥳.

📌 Tighten validation of `experience_builder.generated_field_explicit_input_ux: prop_field_definitions` Active is in! 🎉

Thanks, both of you, looks great! 🤩

Thanks @jonathandealmeida!

I think this means we would need to completely change how the test would work. Based on the machine name, the whole point is that the role does not exist. However, if we're conditionally creating it, that's no longer the casein the situation under test. In other words, fixing it in this way would basically remove the existing test coverage for non-existent roles.

This test coverage was added originally in #2737719-71: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method → . Quoting that comment:

Apparently another fail sneaked in there. In all Block config entities. The root cause is that same problem that was discovered before (see #41):

    // All blocks can be viewed by the anonymous user by default. An interesting
    // side effect of this is that any anonymous user is also able to read the
    // corresponding block config entity via REST, even if an authentication
    // provider is configured for the block config entity REST resource! In
    // other words: Block entities do not distinguish between 'view' as in
    // "render on a page" and 'view' as in "read the configuration".

But #65 introduced additional assertions to ensure a 403 is returned before the necessary authorization is set up. The work-around I had in place didn't account for that. So, now I have changed the work-around to disallow access by any user until the necessary permission is granted. We still need to fix the root cause though.

Created an issue for that now: #2820315: Blocks' 'view' operation is interpreted as "render on page" instead of "read this config entity" → .

Adding those two issues to the related issues.

Based on the above, I don't think we can go ahead with the workaround of conditionally creating the role. Or, if we did, we would need to test the 4-3s for non-existent roles in some other way.

This is also definitely not novice anymore, so untagging.

To comply with the new ConfigExists constraint added in the user.schema.yml schema for the roles field of the user_role condition, it was necessary to ensure that the roles referenced in the tests actually exist as valid configuration. In the createEntity() method of the base test class BlockResourceTestBase, I added the conditional creation of the test role. This guarantees that the role exists in the configuration during test execution, satisfying the ConfigExists constraint.