Contrib.social

Cache invalidation of http_response tag when viewing a node with office hours exception (holiday hours) while having jsonapi enabled

Open on Drupal.org →
Created on 25 February 2025, 13 days ago

Problem/Motivation

When using both office hours (office_hours_exceptions specifically) and jsonapi,
when viewing a node with office hours that contains an exception (office_hours_exceptions),
FieldConfig::create runs in
https://git.drupalcode.org/project/office_hours/-/blob/8.x-1.21/src/Plug....
This triggers jsonapi_entity_create in web/core/modules/jsonapi/jsonapi.module.
This then runs JsonApiRoutes::rebuild(); which causes practically my entire cache to be invalidated through the http_response cache tag.

I wonder if FieldConfig::create should perhaps not run every time we view an office hours exception. Does anyone know why it's being done this way? @johnv perhaps?

Potentially related commit: https://git.drupalcode.org/project/office_hours/-/commit/3d0f921957a23a0...

My Stack when this happens:

OfficeHoursItemList.php:141, Drupal\office_hours\Plugin\Field\FieldType\OfficeHoursItemList->getFieldDefinition()
OfficeHoursItemList.php:63, Drupal\office_hours\Plugin\Field\FieldType\OfficeHoursItemList->createItem()
ItemList.php:71, Drupal\Core\TypedData\Plugin\DataType\ItemList->setValue()
FieldItemList.php:107, Drupal\Core\Field\FieldItemList->setValue()
OfficeHoursItemList.php:84, Drupal\office_hours\Plugin\Field\FieldType\OfficeHoursItemList->setValue()
TypedDataManager.php:216, Drupal\Core\TypedData\TypedDataManager->getPropertyInstance()
FieldTypePluginManager.php:92, Drupal\Core\Field\FieldTypePluginManager->createFieldItemList()
ContentEntityBase.php:633, Drupal\Core\Entity\ContentEntityBase->getTranslatedField()
ContentEntityBase.php:597, Drupal\Core\Entity\ContentEntityBase->get()
metatag.tokens.inc:155, metatag_tokens()
ModuleHandler.php:416, call_user_func_array:{/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:416}()
ModuleHandler.php:416, Drupal\Core\Extension\ModuleHandler->Drupal\Core\Extension\{closure:/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:415-423}()
ModuleHandler.php:395, Drupal\Core\Extension\ModuleHandler->invokeAllWith()
ModuleHandler.php:423, Drupal\Core\Extension\ModuleHandler->invokeAll()
Token.php:364, Drupal\Core\Utility\Token->generate()
token.tokens.inc:1094, token_tokens()
ModuleHandler.php:416, call_user_func_array:{/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:416}()
ModuleHandler.php:416, Drupal\Core\Extension\ModuleHandler->Drupal\Core\Extension\{closure:/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:415-423}()
ModuleHandler.php:395, Drupal\Core\Extension\ModuleHandler->invokeAllWith()
ModuleHandler.php:423, Drupal\Core\Extension\ModuleHandler->invokeAll()
Token.php:364, Drupal\Core\Utility\Token->generate()
Token.php:241, Drupal\Core\Utility\Token->doReplace()
Token.php:191, Drupal\Core\Utility\Token->replace()
MetatagToken.php:66, Drupal\metatag\MetatagToken->replace()
MetatagManager.php:768, Drupal\metatag\MetatagManager->processTagValue()
MetatagManager.php:614, Drupal\metatag\MetatagManager->generateRawElements()
MetatagManager.php:550, Drupal\metatag\MetatagManager->generateElements()
metatag.module:524, metatag_get_tags_from_route()
metatag.module:278, _metatag_remove_duplicate_entity_tags()
metatag.module:220, metatag_entity_view_alter()
ModuleHandler.php:552, Drupal\Core\Extension\ModuleHandler->alter()
EntityViewBuilder.php:305, Drupal\Core\Entity\EntityViewBuilder->buildMultiple()
EntityViewBuilder.php:239, Drupal\Core\Entity\EntityViewBuilder->build()
DoTrustedCallbackTrait.php:113, call_user_func_array:{/web/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php:113}()
DoTrustedCallbackTrait.php:113, Drupal\Core\Render\Renderer->doTrustedCallback()
Renderer.php:870, Drupal\Core\Render\Renderer->doCallback()
Renderer.php:432, Drupal\Core\Render\Renderer->doRender()
Renderer.php:248, Drupal\Core\Render\Renderer->render()
HtmlRenderer.php:238, Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure:/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php:231-239}()
Renderer.php:638, Drupal\Core\Render\Renderer->executeInRenderContext()
HtmlRenderer.php:239, Drupal\Core\Render\MainContent\HtmlRenderer->prepare()
HtmlRenderer.php:128, Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse()
MainContentViewSubscriber.php:90, Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray()
ContainerAwareEventDispatcher.php:111, call_user_func:{/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111}()
ContainerAwareEventDispatcher.php:111, Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch()
HttpKernel.php:186, Symfony\Component\HttpKernel\HttpKernel->handleRaw()
HttpKernel.php:76, Symfony\Component\HttpKernel\HttpKernel->handle()
Session.php:53, Drupal\Core\StackMiddleware\Session->handle()
KernelPreHandle.php:48, Drupal\Core\StackMiddleware\KernelPreHandle->handle()
ContentLength.php:28, Drupal\Core\StackMiddleware\ContentLength->handle()
BanMiddleware.php:50, Drupal\ban\BanMiddleware->handle()
ReverseProxyMiddleware.php:48, Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
NegotiationMiddleware.php:51, Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
AjaxPageState.php:36, Drupal\Core\StackMiddleware\AjaxPageState->handle()
StackedHttpKernel.php:51, Drupal\Core\StackMiddleware\StackedHttpKernel->handle()
DrupalKernel.php:741, Drupal\Core\DrupalKernel->handle()
index.php:19, {main}()

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Active

Version

1.0

Component

Code

Created by

🇺🇸United States olivier.bouwman Portland, Oregon

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @olivier.bouwman
  • Comment 13 days ago →
    🇳🇱Netherlands johnv

    Indeed,
    the code is needed to be able to create subclasses of the OfficeHours item.
    I find the API very hard to handle.
    Perhaps there is a better way to create those special items. I did not check the commit, yet.

    Do you encounter the problem after an upgrade, and did it work in an older version? I might check if the commit can be reversed.

  • Comment 12 days ago →
    🇳🇱Netherlands johnv
  • Comment 12 days ago →
    🇳🇱Netherlands johnv

    Checking the commit, the buffer makes that the ::create() is called only once per page request. Without, it will be triggered upon every Exception/Season time slot.
    I guess it will not be triggered when the page is fetched from cache.

    So adding the buffer will add performance, but will not remove the create. Unless you have problem with the 'many' calls, and do not mind the buffered 'one-time' call?

  • Comment 9 days ago →
    🇺🇸United States olivier.bouwman Portland, Oregon

    Do you think an approach like this can work? It seems to work from my limited testing.
    (excuse my patch file, I can do a merge request if needed)

  • Comment 9 days ago →
    🇳🇱Netherlands johnv

    No need for MR. I do not understand them. I am not sure how your patch works.

    Some remarks on your patch:
    The problem, IIUC, is FieldConfig::create() for every exception, season item. That can be reduced to 1 time per page by bringing back the buffer, but I do not think that it will solve your problem.
    There is no actual field change happening, I only want to have several subclasses of OfficeHoursItem in the ItemList.

    I do not understand how the cache service helps. It does invalidates your cache, too, Not?
    At first glance it might help to refresh stale field caches, for which several issues exist. However, a chache is also invalid when a opening/closing time passes.

    We should investigate how we can 'hide' this creation for other modules. Some core functions have this '$notify' parameter. Or is that exaclty the point of you cache service?

  • Comment 9 days ago →
    🇺🇸United States olivier.bouwman Portland, Oregon

    The problem, IIUC, is FieldConfig::create() for every exception, season item. That can be reduced to 1 time per page by bringing back the buffer, but I do not think that it will solve your problem.
    There is no actual field change happening, I only want to have several subclasses of OfficeHoursItem in the ItemList.

    I do not understand how the cache service helps. It does invalidates your cache, too, Not?
    At first glance it might help to refresh stale field caches, for which several issues exist. However, a chache is also invalid when a opening/closing time passes.

    Correct, caching on a per page basis would not help with this issue. The cache solution I used is persistent across page loads, it is permanent until the field config is saved or a manual cache clear is done.

    Whenever FieldConfig::create runs, any hook_entity_create will run (like jsonapi_entity_create). I don't think this is desired or needed.
    The caching approach I went with causes the field config to be saved permanently for each combination of:
    - entity_type (node for example)
    - bundle (location for example)
    - field_name (field_open_hours for example)
    - field_type (office_hours_exceptions, office_hours_season_header, office_hours_season_item)
    Whenever the field config for any of the related fields is saved (through /admin/structure/types/manage/location/fields/node.location.field_open_hours in our example), the related cache tag is invalidated and FieldConfig::create will run on the next page load to catch any changes.

    I tested with multiple nodes with multiple exception hours per node. Exception hours show correctly and jsonapi_entity_create is no longer triggered.
    Is there a reason to run FieldConfig::create once per page load? I don't see why that would be desired.
    Can you think of any test I could do that would help us understand this better or ensure this solution works for other use-cases?

    > We should investigate how we can 'hide' this creation for other modules. Some core functions have this '$notify' parameter. Or is that exaclty the point of you cache service?

    I don't think this is necesarry as long as we can limit the FieldConfig::create to be done infrequently (not on every page load).

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024