Allow FieldInstance yml files to refer to the Field by field name rather than by field_uuid

Having a crack at this

Let's see what breaks.

4: field_uuid_in_instance_yml-2143519-3.patch queued for re-testing.

Yes, deleted instances have an additional "field_uuid" property, but I don't want to break if the "field_name" property is still there (it is not "wrong" per se, it's just not the one we want to use for determining which field to use).
So the logic is:
- use the field_uuid if present
- else use the field_name, excluding deleted fields

Making this work only on deleted fields: probably a good idea. Lets us get rid of a field_read_fields() call, so attached patch goes with that.

Hah, so, you can be a deleted instance of a non-deleted field.
In that case, you have a field_uuid property, but your field is not deleted.

I feel a bit uncomfortable about enforcing that this only gets used by deleted instances actually. I'd rather punt on that for now.

Also, until #1503314: Remove the concept of active / inactive (field types, storage) from Field API gets in (patch should be ready, needs committers feedback on comment #25 ;-)), we still need to account for the case of "inactive, deleted" fields.

So updated patch basically just moves the existing "if field_uuid specified" case on top of the "if entity_type / field_name specified" case, so that it takes precedence.

Reroll after #2020895: Move save() / delete() logic in Field / FieldInstance to [pre|post]Save(), [pre|post]Delete()

Forgot a chunk in the reroll.

@sun:
Yup, the purpose is 2.2. As per #2121751: [META] Making configuration synchronisation work, we're moving away from "hardcoded UUIDs in default config", in favor of #2133325: Create a site UUID on install and only allow config sync between sites with the same UUID, and a prerequisite is that field instances shipped in default config stop referring to their field by its UUID.

Only non-deleted fields and instances are stored in config, and for those an [entity type, bundle, field_name] is unambiguous. Deleted fields and instances, for which we need a field_uuid in the instance, are stored in state, and there we make sure the field_uuid is present. So in regular runtime operations, field_uuid is not needed in config.

Now that you mention it though - hm, I can indeed think of race conditions during config sync where that would be an issue :-/ So yeah, writing back the field_uuid on save might be a good thing...

Working on it.

Let's see this.
Interdiff attached, but this is mostly reverts, so looking at the patch probably makes more sense.

Yup, fixed.

18: field_uuid_in_instance_yml-2143519-18.patch queued for re-testing.

18: field_uuid_in_instance_yml-2143519-18.patch queued for re-testing.

More accurate title

Indeed, sorry, should have thought of this one :-/
Thanks for the reroll, @Wim. #30 should be good.

Green, back to RTBC

Those field_uuid entries we left in field.instance.*.yml are going to be problematic for default config.

As discussed in #2133325: Create a site UUID on install and only allow config sync between sites with the same UUID, the most common real-life workflow is going to be "I populate my module's config/ folder by picking and copying real yml files from my active config".
So we will find leftover 'uuid' and (in the specific case of field.instance.*.yml) 'field_uuid' entries in the default config/ we import.

The direction over there is that 'uuid' entries will just be silently ignored / overwritten by a fresh UUID by the "import default config" process.

But this doesn't apply to our specific field.instance.* 'field_uuid' here, the config system doesn't know about it and won't clean it up. So if present, it will be taken into account in FieldInstance::__construct() and die with a bang because there's no actual field with that UUID...

Attached patch only considers an incoming 'field_uuid' if there's also a 'uuid' present too - i.e it's a yml file from a real "active config" folder. That would work *if* the "import default config" process actually *removes* the leftover 'uuid' entries it finds in default config before doing the corresponding entity_create().

35: instance_yml-field_uuid-2143519-35.patch queued for re-testing.

The real-life test case would be to test the import of a field.instance with both 'field_uuid' & 'uuid' leftover entries, since this is what files copied from an active store will look like.

The fix in this patch assumes that leftover 'uuid' entries left in config/ folders will be discarded by the "config import" mechanism before they reach the entity_create() step. Is that the case already ?

If not, one way to test this would be to put in some_test_module/config a field.instance file with a leftover 'field_uuid' but no 'uuid'. Would fail on HEAD and pass with the patch.

But ultimately it would be better to test the real life case.

@alexpott: you mean changing the config installer to discard incoming uuids here in this issue ? Why not, but we're already a followup of a committed patch here.

(#35 is only a one line change and is tightly related to the 1st patch that went in, so it seemed easier to keep it tied to the earlier discussion)

Your call :-)