Simplify ResourceTypeRepository; use a protected property in place of an in-memory cache bin.

I wanted to start doing this so we can fix the fact that the commit of #3014232-62: [regression] ResourceTypeRepository is significantly slower in JSON:API 2, becomes noticeable when handling hundreds of interlinked resources broke HEAD, but I don't quite understand what this issue aims to do. Hoping either of you can do this.

I think that once this lands, we should tag RC3, to get this bugfix (and #3014380: EntityReference base fields that are optional are not empty, but its sole item is empty, causing EntityReferenceFieldNormalizer to fail) into the hands of as many people as possible.

They must know if the resource types could be invalid as opposed to hiding that information.

I think that's totally fine for an internal API.

+++ b/src/ResourceType/ResourceTypeRepository.php
@@ -79,22 +77,19 @@ class ResourceTypeRepository implements ResourceTypeRepositoryInterface {
+    if (!isset($this->cache[$cid]) || $reset) {

@@ -107,9 +102,9 @@ class ResourceTypeRepository implements ResourceTypeRepositoryInterface {
+      $this->cache[$cid] = $resource_types;

We don't want to set the cache when $reset is TRUE

This issue argues for the introduction of a persistent cache. If so, would we still want to do this? Would we go for a chained cache?

Sorry for the noise, I didn't think we'd need persistent cache (and I still don't think that non-paragraphs data models will need it).

Let's make a decision on what to do before spending more time writing code.

#3018287: ResourceTypeRepository computes ResourceType value objects on *every request*

Closing per:

Closing per:

Screenshots of chats are not accessible, textual chat logs are. So adding that too :)

e0ipso [6:42 PM]
I'm OK with whatever. I think that if we can get away without an additional service we should.
But if #3018287: ResourceTypeRepository computes ResourceType value objects on *every request* will need it regardless
then we should use it in the other places as well

wimleers [6:43 PM]
but 3018287 cannot continue either since HEAD is broken …
:stuck_out_tongue:

e0ipso [6:43 PM]
what I meant is

wimleers [6:43 PM]
it’s an annoying dependency circle we need to break

e0ipso [6:43 PM]
that if we will want to do what 3018287 proposes
we should do it now
and close 3016733
Once that's closed
we should fix the remaining bits on https://www.drupal.org/project/jsonapi/issues/3014232#comment-12888538 and commit it
so we can merge the Media tests fix
and carry on
@wimleers @gabesullice :+1::skin-tone-4: :question:

wimleers [6:47 PM]
WFM

gabesullice [6:47 PM]
I agree that we should fix the remaining bit of 3014232. Then merge the Media fix.

Then, I'm mildly in favor of closing 733 and introducing a persistent cache.

e0ipso [6:48 PM]
concur

gabesullice [6:48 PM]
So, it's up to @e0ipso and @wimleers about that

wimleers [6:48 PM]
wfm

e0ipso [6:48 PM]
it sucks that we have to though
but it's because we are generating as much as we can statically, which is good

wimleers [6:49 PM]
What sucks?

e0ipso [6:49 PM]
having to cache the resource types

wimleers [6:53 PM]
But it doesn’t make sense to recompute them on every request either.
Either way, I don’t have a strong opinion.
My priorities: 1) have it work at all (pass tests), 2) simplicity/maintainability, 3) performance
And sometimes 3 trumps 2
if it’s a big enough slowdown :slightly_smiling_face:

e0ipso [6:54 PM]
> But it doesn’t make sense to recompute them on every request either.
yeah, that's why we *have to* (IMHO) introduce the complexity
all agree, then!

gabesullice [6:55 PM]
onward!

wimleers [6:55 PM]
> yeah, that’s why we *have to* (IMHO) introduce the complexity
Yep!