Allow config entities to specify the config key <-> entity property mapping

1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
   @@ -91,7 +91,20 @@
   +    foreach ($values as $key => $value) {
   ...
   +    foreach ($values as $key => $value) {
   +      $this->$key = $value;
   

   Why not just skip the manipulation of $values?
   Something like this:

   foreach ($values as $key => $value) {
     $mapped_key = isset($config_mapping[$key]) ? $config_mapping[$key] : $key;
     $this->$mapped_key = $value;
   }
   

2. +++ b/core/modules/node/src/Entity/NodeType.php
   @@ -29,6 +29,16 @@
   + *     "name" = "name",
   + *     "type" = "type"
   + *     "description" = "description"
   + *     "help" = "help"
   

   You could leave these out

1. +++ b/core/modules/node/src/Entity/NodeType.php
   @@ -29,6 +29,16 @@
   + *     "name" = "name",
   + *     "type" = "type"
   + *     "description" = "description"
   + *     "help" = "help"
   + *     "new_revision" = "newRevision"
   + *     "preview_mode" = "previewMode"
   + *     "display_submitted" = "displaySubmitted"
   + *     "third_party_settings" = "third_party_settings"
   

   This failed first because of missing commas here.

2. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
   @@ -244,12 +257,25 @@ public static function sort(ConfigEntityInterface $a, ConfigEntityInterface $b)
   +      if (!isset($config_mapping[$name])) {
   +        continue;
   +      }
   

   Then it failed because not every exported value will be in $config_mapping.

I included my suggestions from #3.

I also included unit tests (which required a bit of rejiggering of ConfigEntityBaseUnitTest).

Finally, I removed the dual typing of config_mapping in EntityTypeInterface. We'll be served just as well by an empty array.

-------

One last thought I had, which I didn't touch: this isn't really config_mapping, but config_key_mapping. Thoughts on renaming it?

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
@@ -274,6 +277,21 @@ protected function getTypedConfig() {
+  public static function preCreate(EntityStorageInterface $storage, array &$values) {

Since this isn't implemented on the base classes, none of the entity classes overriding it call parent::, so this won't run. Not many config entities override it, but View certainly does.

ConfigEntityForm::copyFormValuesToEntity() : $entity or $this->entity ?

Although : if the snake_case properties are only for config records, while runtime objects have camelCase properties, then the forms should manipulate the camelCase properties ?

I.e you'd have
'#default_value' => $entity->myProperty
$form['myProperty']
$form_state['values']['myProperty']
rather than 'my_property' ?

Meaning, no mapping logic in the form, only on "save to a config record" ?

More accurately, #2016679: Expand Entity Type interfaces to provide methods, protect the properties is about moving all properties to protected with getters, and the generic get() / set(), so my snippet in #14 would rather be something like :

// on form build :
$form['myProperty'] = array(
  '#default_value' => $entity->get('myProperty'),
);
// on form submit
foreach ($form_values as $property => $value) {
  $entity->set($property, $value);
}

Doesn't change much - either the form uses the camelCase properties and you can do the generic "assign form values into the object" as above, or the form uses snake_case properties and we need to use the mapping.

Yeah, camelCase entries in $form would look a bit uncommon, and not sure what kind of HTML attributes they would produce ('id', 'name'...), maybe that's not ideal.

But with the mapping (current patch) we'd have :
- entity load: map snake to camel (config record to $entity properties)
- form build : map back camel to snake ($entity properties to $form structure) - but that step will more likely be a series of explicit "$form['snake_case'] = array(...$entity->get('camelCase')...)", that needs to comply to the mapping without actually using the mapping.
- form submit : map snake to camel ($form_values to $entity properties)
- entity save : map back camel to snake ($entity properties to config record)
That's a hell of a dance... :-/

It also makes the mapping be not a "config mapping", but a "mapping for places where we'd prefer to have camelCase (object properties) and places where we'd prefer to have snake_case (config records, form structures)".

On a different topic, it looks weird that mapping is done in toArray() but mapping back is done in preCreate() :
when loading an existing entity from config, preCreate() doesn't run , only __consruct() does, right ?
So I'm not sure how loading could work with the current patch ?

There's currently a (ok, unwritten :-)) contract that toArray() produces values that are valid for passing back to __construct().
That's what config entity loading from CMI records relies on.
That's also the base for the serialization logic implementend in EntityDisplayBase (and that #1977206: Default serialization of ConfigEntities tries to generalize to all config entities) - basically: serialize to toArray(), unserialize by passing those values back into __construct($values_from_to_array).
If toArray() maps camelCase -> snake_case, but mapping back is done in preCreate(), then it looks like this contract is broken ?

More generally: toArray() advertizes itself as a generic "dump to an array" mechanism, that happens to be used, among other things, for config record generation. By baking a "camelCase to snake_case" mapping in there, we confiscate it a bit for CMI business logic.

I guess it's not completely unreasonable to state that "the official array dump shape of a ConfigEntity" is one with camel_case keys. Not sure of the exact consequences outside of CMI, though.

@bojanz: Anything left to do here after #2432791: Skip Config::save schema validation of config data for trusted data?

Fix merge error in last patch.

Alright, here are two patches: One adds a test to ConfigEntityStorageTest which tests the camelCase to snake_case conversion in core (which is working), the second one does the same for flag, the tests will break because camelCase/snake_case import is not yet supported for contrib modules.

I don't know if the flags tests will even be run/if the patch "applies", but we can do manual testing I guess?

Patch was not applying.

I think that is not the test you are looking for (*jedi hand gesture*). The critical part is that the value is read from protected_property in the DB, and then set to $entity->protected_property but it should be set to $entity->protectedProperty. So something like assertEqual($entity->protectedProperty, ‘TEST’) would make more sense imho.

Here is a new patch, which fails for the new getProtectedProperty() method. It is NULL instead of TEST. So something is really wrong there.

I left the toArray stuff in (the test method could use a renaming) and added a line which tests the camelCasing for toArray, which we seem to want according to #18:

So, the latest approach means:
- Config entities declare camelCase properties and snake_case config keys.

That seems to work.

- entity_create() expects camelCase keys, and ConfigEntityBase::toArray() returns camelCase keys.

entity_create still uses snake_case, when I switch to camelCase, all three tests fail.

- The config entity storage maps the snake_case keys to camelCase keys (and back) when the config entities are being loaded/saved.

That doesn't seem to work. I added two tests: one which returns the property as defined in code. That one fails, strangely. And another one which returns the property witch snake_casing. That actually works, even though there is no property in the code, the import messes it up.

Forgot the interdiff, sorry!

@LKS90's patch in #35 no longer applied to 8.4.x. A reroll with no further modifications is attached in case this issue still needs to go forward.

Reroll of #42 for 10.1.x

Fixed Failed Commands on #54
Attached patch against Drupal 10.1.x
Attached interdiff

This is a proposal implementing an idea in another issue, #1832630: Decide on coding standard for configuration keys of ConfigEntities. I've just closed that as outdated, so it makes sense to do that to this one as well.