Preserve the 'field_type' within FieldInstanceConfig

FWIW, not sure if we should keep the current title or name this "Avoid loading FieldStorage in the cache hit critical paths" ?

Ran a quick check by placing a breakpoint in FIC::getFieldStorageDefinition(), and checking the occurrences where FIC::getType() is *not* the caller.

On node/[nid] (with entity and render cache hits) :

- EntityViewController::view() (page controller for "full entity" pages) does an explicit $entity->title->view('full'), to extract the entity title as page title, this creates the FieldItem/FieldItemList objects and required the FieldStorage for various stuff
- The "add new comment" form creates a fresh comment entity and generates a form for it, that requires full loading as well.

Not too worried on those two, no big impact, + not much we can do anyway.

On /node (list of node teasers, with entity and render cache hits) :

- comment_node_links_alter() does a $node->get($comment_field)->status, full load

So we might be good ?

Well, of course, the above might vary if some formatters have #post_render_cache steps that do additional work with field values, but similarly, not sure we can do much about that case.

Let's see with this.

Contrary to what I wrote in the summary, fetching the FieldStorage in FieldinstanceConfig::__construct() is painful, because $this->getFieldStorageDefinition() doesn't work at this point (the $values haven't been assigned to the object properties yet), and we'd have to basically duplicate it.

Instead, getType() lazy loads the field type if not set, and toArray() makes sure it's set and will be present in the config record. Not a huge fan, but it's what I have for now. Suggestions welcome.

From my comment #2 above:

EntityViewController::view() (page controller for "full entity" pages) does an explicit $entity->title->view('full'), to extract the entity title as page title, this creates the FieldItem/FieldItemList objects and required the FieldStorage for various stuff

Fixed in #2314359: Optimization: do not iterate on $entity fields in EntityViewBuilder::viewField()

From my comment #4 above:

getType() lazy loads the field type if not set, and toArray() makes sure it's set and will be present in the config record. Not a huge fan, but it's what I have for now

Attached patch calls getType() to make sure $this->field_type is initialized in postCreate() rather than in toArray().
postCreate() already performs a somewhat similar check (call $this->getFieldStorageDefinition() to make sure we refer to a valid FieldStorage), and this removes the need to override toArray().

Hm. Then FieldInstanceConfigEntityUnitTest::testToArray(), which simply does

$instance = new FieldInstanceConfig($values);
$instance->toArray()

fails, since it skips postCreate()...

Not sure what's best here. Adding an explicit call to $instance->postCreate() in there feels lame...

OK, gave it some more thought.

- Most/all of the current FIC::__construct() (massaging $values) is what preCreate() is there for. The only thing that stops me from moving it there atm is the fact the phpdoc for FIC::__construct() is where we document the various things you can pass to entity_create('FIC', $values), and moving that doc to preCreate() is not as obvious.
- Following the same reasoning, populating the denormalized 'field_type' really is a postCreate() thing.
- Unit tests that skip entity_create('FIC', $values) and directly do new FieldinstanceConfig($values) need to adjust their $values accordingly - in this case, pass an explicit 'field_type'.

Thus, attached patch does the following :
- 'field_type' is fetched from the field storage in FIC::postCreate(), if needed,
- FIC::getType() no longer lazy-loads, it just returns $this->field_type. The regular entity_create() flow ensures it's present, if you skip that flow, it's your job to make sure it's present in the object you build.
- adjusts the new FIC() values in FieldInstanceConfigEntityUnitTest accordingly
- side fix: passing a $values['field_storage'] in entity_create('FIC', $values) currently results in the $instance having both ->field_storage and ->fieldStorage properties. The internal property is the latter.

Going away from my coding env. for 10ish days tomorrow, not sure I'll have time to work on tests before that.

Also, not sure what to test exactly:
- the basic behavior is that an entity_create('f_i_c') automatically populates $instance->field_type. We can add a dedicated test (cannot be a PhpUnit test, we can't test entity_create with those, can we ?), but tens of tests would fail already if that broke.
- the more general behavior is that you can fetch the field type of a field definition without loading the storage definition, or mere globally that you can load and render an entity from the caches without loading the field storage. Would be cool to track regressions on that, but not sure how this is testable ?

In short : help welcome for the tests.

Sure, that doesn't test much though - merely that getType() returns the 'field_type' you pass to the FIC construct.

It also passes in HEAD because this unit test's setUp() prepares a mock FieldStorageConfig, which, in HEAD, is where FIC::getType() reads from.

So, not sure this will really help prevent regressions ?

Oops, sorry, was too quick, I saw some green, but there are no results yet :-/

So, sure, good enough for me if it's good enough for you :-)

Actually, I'd rather wait for #2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields and re-do the patch with BaseFieldOverrides in mind.

Updated for FieldConfigBase and BaseFieldOverride.

No interdiff because of the reroll - basically, the "populate $this->field_type on postCreate()" behavior is moved to FieldConfigBase.