Trigger route rebuild when new bundles/fields are added/removed

👍

I knew this was familiar, that I recently worked on something very similar.

Turns out I did, in jsonapi_extras, for #2982133: No longer works with JSON API >=1.21 (so either 1.21 or 1.22)!

This is the goal, per the IS.

Note that I found another, related TODO, in ResourceTypeRepository:

  // @codingStandardsIgnoreStart
  // @todo implement \Drupal\Core\Plugin\CachedDiscoveryClearerInterface?
  // @todo implement \Drupal\Component\Plugin\Discovery\DiscoveryInterface?
  public function clearCachedDefinitions() {
    $this->all = [];
  }
  // @codingStandardsIgnoreEnd

This implements the original goal, which fixes most of the problem!

And this then addresses the repository portion.

Oops.

In having done the above, I saw an easier and better solution for ResourceTypeRepository's weird static caching. Or so I think. Because this is not yet working. I need to work on other things first.

Also: if this works, then those ResourceTypeRepository changes belong in a separate issue.

Rebased.

All the \Drupal\jsonapi\ResourceType\ResourceTypeRepository::calculateRelatableResourceTypes() stuff only worked because everything was passed by reference into ResourceType::setRelatableResourceTypes()… 😵 🤐 This took quite a bit of effort to untangle.

Might as well make this way faster now…

And here are the necessary test expectation updates.

This should be green.

Still not yet green, the logic changes (fixes, really) in ResourceTypeRepository are still causing some failures. Investigating that is for later.

There's still something wrong in ResourceTypeRepository. Too bad. There's higher-priority things to deal with first, so going to stop working on this for a while.

+1 — that's exactly what I always suspected!

I already spent _hours_ on fixing the logic in ResourceTypeRepository. I think a fresh, new set of eyes would be a good idea.

I'd like to have manual testing for this when installing a distribution that includes configuration to create bundles out of the box (like Contenta). See #2982133-38: No longer works with JSON API >=1.21 (so either 1.21 or 1.22).

Please set back to needs review when #39 is cleared.

Please feel free to merge.

+++ b/jsonapi.module
@@ -44,3 +46,37 @@ function jsonapi_entity_bundle_field_info(EntityTypeInterface $entity_type, $bun
+function jsonapi_entity_bundle_create() {
+  JsonApiRoutes::rebuild();
+}
...
+function jsonapi_entity_bundle_delete() {
+  JsonApiRoutes::rebuild();
+}
...
+function jsonapi_entity_create(EntityInterface $entity) {
+  if (in_array($entity->getEntityTypeId(), ['field_storage_config', 'field_config'])) {
+    // @todo: only do this when relationship fields are updated, not just any field.
+    JsonApiRoutes::rebuild();
+  }
...
+function jsonapi_entity_delete(EntityInterface $entity) {
+  if (in_array($entity->getEntityTypeId(), ['field_storage_config', 'field_config'])) {
+    // @todo: only do this when relationship fields are updated, not just any field.
+    JsonApiRoutes::rebuild();
+  }
+}

😞

Can you explain this:

I'm not clear if this will catch an existing field that is added to an existing bundle yet, since that's technically not changing any field storage stuff.

Which unfortunately turned out to be an issue.

Adding an existing field to an existing bundle surely is also invalidating that cache tag?

That event is not fired when an existing field is added to an existing bundle

That sounds like a pretty severe Drupal core bug.

This issue introduced an unnecessary file JsonApiRegressionTest.php.orig.

Committed & pushed #50.

#48: but the entity_field_info cache tag is NOT about field storage definitions, it's about all field definitions. See for example \Drupal\Core\Entity\EntityFieldManager::clearCachedFieldDefinitions(). A more convincing example: grep entity.api.php for *_field_info, and note how there's hooks for "base", "bundle", "storage" and "extra".
I tried to find an existing issue for this, but failed. The only related issue I could find was #2926139: EntityFieldManager's field map not invalidated as promised.

Oh, but wait, you're saying that your storage event listener approach from #28 suffered from this problem. I can see that. But I don't see why my cache tag-based approach is unable from #22 cannot work?

Nah, this is now working, so let's not spend more time on it. It's just unfortunate that now jsonapi.module is a lot meatier again…

To everyone following this issue: sorry for the noise!

The issue I linked in #53 was marked as a duplicate of #2994398: Not properly clearing EntityFieldManager's fieldMap leads to fatals, often after migration or bundle creation, which fixes the problem more thoroughly!

@e0ipso noticed in #49 that this commit introduced tests/src/Functional/JsonApiRegressionTest.php.orig. I just noticed it also introduced src/Controller/EntryPoint.php.orig.

Hi fellows,

I am afraid this problem still exists when you have multiple sub-websites. The main website has no problem but all the subsites encounter the following error:

TypeError: Argument 2 passed to Drupal\jsonapi\Routing\Routes::Drupal\jsonapi\Routing\{closure}() must be an instance of Drupal\jsonapi\ResourceType\ResourceType, null given in Drupal\jsonapi\Routing\Routes::Drupal\jsonapi\Routing\{closure}() (line 330 of modules/contrib/jsonapi/src/Routing/Routes.php).

I have tried to manually rebuild the `router.builder` in the subsites but it didn't help. Besides, I have checked other jsonapi versions and noticed that the problem doesn't exist in the version 1.0.

Sorry for reopening the issue and thank you very much for your support!

@hedeshy The issue you linked to and the error message you cite are unrelated to what we did here. Could you please open a new issue? I promise we'll look at it swiftly :) Thanks!

Moving to Drupal core's issue queue.

I'm working on https://www.drupal.org/project/drupal/issues/3122113