🇺🇸United States @olivier.bouwman

olivier.bouwman → created an issue.

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).

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)

olivier.bouwman → created an issue.

greggles → credited olivier.bouwman → .

Thank you so much for the super quick fix! This works for my use case.

Here's a simplified version of my use case. We show wait times for locations. If a location is currently closed or was opened in the last hour, we don't show wait times. Because I have to do two isOpen checks for this use case, I had to add the $open_hours_clone = clone $location->field_open_hours; line. Without it, the result for the second isOpen is always the same as the first. I'm not sure how common this use case is but it seems like isOpen shouldn't always return the same value if a different timestamp is passed into it. Does that clarify things?

public function getWaitData($nid) {

    // See if this location is currently open according to the open hours.
    $location = Node::load($nid);
    $is_open = (int) $location->field_open_hours->isOpen();
    if (!$is_open) {
      return [
        'wait_time' => t('@disabled', ['@disabled' => self::WAIT_TIMES_DISABLED]),
      ];
    }

    // See if this location has opened in the last hour according to the open hours.
    $open_hours_clone = clone $location->field_open_hours;
    $time_one_hour_ago = \Drupal::time()->getRequestTime() - (1*60*60);
    $was_open_one_hour_ago = (int) $open_hours_clone->isOpen($time_one_hour_ago);
    if (!$was_open_one_hour_ago) {
      return [
        'wait_time' => t('@disabled', ['@disabled' => self::WAIT_TIMES_JUST_OPENED]),
      ];
    }

    // more code here....

olivier.bouwman → created an issue.