Add Field cache for 'Currently Open/Closed' Status formatter

The current dev-code now has 2 formatters: The time slots and the status.
So we could incorporate your solution in the latter. Can you upload your code, or better, evaluate the new formatter code?
I (maintainer) am not up to speed with twig, so the current formatter is broken - it does not show a line per time slot.

Over the last years, I understood that the caching mechanisme of D8 was drastically overhauled, and that you can (per page) determine the caching method per block.

Why don't you test this?
- create a new block, showing the node with the field formatter 'Status' in it. Set the cache options.
- add the block to the node page.
- see what happens.
I guess you need 'View modes' under the Manage display tab, hiding every field except the Office Hours.

I don't think this is a problem specific for this module. There are several components to get to your desired result.
- Current version has a separate formatter for 'status'
- use Views or Blocks, with their own caching mechanism.

There is a Cache API : https://www.drupal.org/docs/8/api/cache-api/cache-tags

See the example for

class GenericFileFormatter extends FileFormatterBase {
  public function viewElements(FieldItemListInterface $items, $langcode) {
    $elements = [];

    foreach ($this->getEntitiesToView($items, $langcode) as $delta => $file) {
      $item = $file->_referringItem;
      $elements[$delta] = [
        '#theme' => 'file_link',
        '#file' => $file,
        '#description' => $item->description,
        '#cache' => [
          'tags' => $file->getCacheTags(),    // Adds a Cache tag.
        ],
      ];
      // ...
    }
    return $elements;
  }
}

What opening hours prolly needs to do is use the max-age cache property.

The time period for the max-age cache can be dynamicly calculated based on the current time and the first upcoming change

e.g.

opening hours say 10h - 12h

cache miss on 11h, thus the max-age is dynamicly set to 1h
cache miss on 11:55h, thus the max-age is dynamicly set to 5min
cache miss on 8h, thus the max-age is dynamicly set to 2h

This way we can cache this field as much as possible without showing wrong data.

Patch attached which adds max-age cache property based on first upcoming status change.

Needs testing

Updated patch to work with latest dev

Please test latest dev. It now has a fairly simple patch using the cache API.
It revealed this error: #2898247: 'Show next open day' formatter is incorrect if only open 1 day/week

Aren't settings in "Number of days to show" also affected by wrong caching? "Show next open day" and "Show only current day" should have a max-age other than -1 as well. Probably setting it to 00:00:00 of the next day should be correct, right?

I tried achieving this in a similar to the code in the last commit, but that didn't have any effect an the caching behavior.

Should we create a new issue for that? Attaching a patch here for now.

@Lukas von Blarer
Since the original issue was fixed in 8.x-1.x, I have created a followup issue at #2904673: Add field cache for "Next/Current open day" formatter. Can you upload your patch in the related issue so you can be credited for your contribution?

I have uploaded that patch in the other issue.

Patch #13 has an error for entities with no open time slots. This is fixed here : #2902422: Warning in Field cache for Status formatter, when no open time slots exist

Is this supposed to work? Drupal core has a bug related to max-age, which this module tries to solve https://www.drupal.org/project/cache_control_override

In my tests, it worked. But the tests were without Varnish or other tools. You have problems?

I tried to test this without varnish bypassing it in our local development environment, but max-age is always 3600 which we have set in the performance configuration.

How did you test it? It says on the module page quite clearly that ...max age settings are applied globally to all pages, and there is no way to set a different max age per page or leverage cache metadata max-age to override Cache-Control max-age value. https://www.drupal.org/project/cache_control_override

So when the OfficeHoursFormatterStatus.php is trying to set the #cache max-age value, I don't think it works:

'#cache' => [
    'max-age' => $this->getStatusTimeLeft($items, $langcode),
  ],

I noticed this when I was working on some custom JSON feed which was supposed to be cached to maximum one minute to make sure the data is "real-time". Working with Drupal's CacheableJsonResponse just didn't work, I had to use Symfony's JsonResponse instead:

  // Custom response which works with max-age.
  $response = new JsonResponse($results);
  $response->setMaxAge($cache_rules['max-age']);
  // When max-age is set, ETag should have some value too.
  $response->setEtag(time());

Plan is to set a cache not on the page, but on each block/entity in the page.
https://www.drupal.org/docs/8/api/cache-api/cache-api

Many years later, this problem still continues.... Caching don't work at all for anonymous users. Especially after a cron job

An important possible relevant core issue is postponed (https://www.drupal.org/project/drupal/issues/2352009). In the mean time, I wanted to share something that is working for us.

In our use case, we have the page cache enabled and set to 900 seconds (15 minutes). This is a global setting that I do not want to sacrifice. Therefore, I am leveraging the existing office_hours:field.status cache tag and changing the "max-age" and "expires" in the HTTP header based on the presence of that cache tag. Now, any page that the "closed/open" status is displayed on will on be cached for 60 seconds. This could be refined to use a more precise max-age, but we didn't need the additional optimization.

1. Add an event listener (borrowed heavily from https://www.drupal.org/project/cache_control_override)
MY_MODULE.services.yml

services:
  MY_MODULE.cache_control_override_subscriber:
    class: Drupal\MY_MODULE\EventSubscriber\CacheControlOverrideSubscriber
    arguments:
      - '@datetime.time'
    tags:
      - { name: event_subscriber }

namespace Drupal\MY_MODULE\EventSubscriber;

use Drupal\Component\Datetime\TimeInterface;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\CacheableResponseInterface;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
 * Cache Control Override.
 */
class CacheControlOverrideSubscriber implements EventSubscriberInterface {

  /**
   * The time service.
   *
   * @var \Drupal\Component\Datetime\TimeInterface
   */
  protected $timeService;

  /**
   * ProductHelper constructor.
   *
   * @param \Drupal\Component\Datetime\TimeInterface $time_service
   *   The time service.
   */
  public function __construct(TimeInterface $time_service) {
    $this->timeService = $time_service;
  }

  /**
   * Overrides cache control header if any of override methods are enabled.
   *
   * @param \Symfony\Component\HttpKernel\Event\ResponseEvent $event
   *   The event to process.
   */
  public function onRespond(ResponseEvent $event) {
    if (!$event->isMasterRequest()) {
      return;
    }

    $response = $event->getResponse();

    // If the current response isn't an implementation of the
    // CacheableResponseInterface, then there is nothing we can override.
    if (!$response instanceof CacheableResponseInterface) {
      return;
    }

    // If FinishResponseSubscriber didn't set the response as cacheable, then
    // don't override anything.
    if (!$response->headers->hasCacheControlDirective('max-age') || !$response->headers->hasCacheControlDirective('public')) {
      return;
    }

    $cache_tags = $response->getCacheableMetadata()->getCacheTags();
    // Check for our custom cache tag so that we can detect if the office hours
    // are being displayed on this page.
    if (!in_array('office_hours:field.status', $cache_tags)) {
      return;
    }

    $max_age = $response->getCacheableMetadata()->getCacheMaxAge();
    // Set max age to 60 seconds (from 900 seconds UTDK default).
    $new_max_age = '60';
    // We treat permanent cache max-age as default therefore we don't override
    // the max-age.
    if ($max_age != Cache::PERMANENT) {
      $response->headers->set('Cache-Control', 'public, max-age=' . $new_max_age);
      // Set expires.
      $expires_timestamp = $this->timeService->getRequestTime() + $new_max_age;
      $expires_datetime = new \DateTime();
      $expires_datetime->setTimestamp($expires_timestamp);
      $response->setExpires($expires_datetime);
    }
  }

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents() {
    $events[KernelEvents::RESPONSE][] = ['onRespond'];
    return $events;
  }

}

Reopening this, to get it back in the issue list.

I tried doing this, first by changing the OfficeHoursFormatterStatus to use lazy building & big pipe, then by adjusting the cache modules (big_pipe_sessionless, cache_control_override), but concluded that this is impossible to get working for anonymous users when page cache is in use, at least as long as #2352009: Bubbling of elements' max-age to the page's headers and the page cache is unresolved.

So I chose completely different approach. A maintainer may decide if this solution is ok in this issue or not - it solves the problem but not by adjusting the caching. This change creates a controller and uses JavaScript to fetch the rendered Office Hours field content and swaps it with the original one. So this is immune to caching issues. The user might be able to see the earlier version for a brief moment though, but this works for us.

No new dependencies to jQuery and other legacy things introduced, and because of drupal/once library, the module version requirement is set to ^9.2.

A minor thing fixed from patch #28.

Fix another issue: remove unnecessary dependency to node entity.

Patch #30 looks good and fixed the issue in our project!

As I understand now, the 'max-age' value for each setting of the formatter should be OK, now.
But drupal core does not behave properly.

As a maintainer, the proposed patch is too big/complex to add this to this module, just to avoid a core issue.
It should be addressed at the correct code.

As I understand it, the problem you have in the module code, can't be fixed properly. Drupal can't reliably handle rendered content that should update when the caching is on, and on the other hand, the caching shouldn't be disabled.

That's why I was suggesting a different approach. If you need the content to update by the time, you need to work around that problem.

Hello,
so what is status of this? I'm using Drupal 9.4 RC and this field show me closed even few minutes after openning. Does this have some solution?
Thank you.

I noticed that when using a view and rendering using display modes the status would never update. My workaround was to display as fields in the view and add the "Hours of operation" field. Then, I made sure to disable caching on the specific view.

Also worth noting that the status was ALWAYS cached in my view if I used the "status" field formatter for my "Hours of operation" field. I was forced to use the "plain text" formatter in my case and made sure that the "Current status" section had "Current status position" set to "Before hours". This seems to bring in the status and respects my desired caching option on my view.

I tried #30 patch and my table with open hours is shown multiple times.

Regarding the status (#34),
In my opinion, as stated earlier, it should not be the task of a module to solve a foundational problem in Core.
This module just sets the 'max-age' attribute on field level, as documented in the API.
When this attribute is wrong, I am happy to solve it.
FYI, the calculation is in function getStatusTimeLeft(array $settings, array $field_settings).
If not, I am not able to simulate all caching mechanisms.
In my humble test system, the pages are refreshed at the correct moment. (unless my tests are wrong, and StatusFormatter interferes with HoursFormatter.

Perhaps the problem is the following:

The field contains 1,2 or 3 formatters for Slots, Status (open/closed), and Schema. so, there may be 3 'max-age' declared, but not on field level, but on formatter-array-item level. If so, perhaps the first is taken, and the Cache behavior may be different if the Status Formatter is before or after the Slots Formatter.

Please check the latest code.
As mentioned above, the caching time was not correct when the open/closed formatter was before the normal formatter.

There is a @todo: The tag is now like this: "node.ctoh.field_office_hours_2"
It should be changed into ":"

Attribute '#cache' now has correct tags, e.g., "node:18".

Please test. Reopen if still no good.

Please be aware that an additional fix has been implemented here: #3318551: Fix Field cache for 'Currently Open/Closed' status formatter

I'm sorry to tell but the caching is still not quite right: please reopen the issue. (Found this issue again because my workaround patch stopped applying.)

The max-age calculation works, and the status flag kind of works - but only if the caching layers are not doing their things. In a project, we have the Page Cache and Dynamic Page Cache on, then Varnish and a CDN in front. To my understanding, Varnish is repeating what the Page Cache knows after an additional page load, and maybe there's no mechanism for them to update their knowledge about the max-age being spent already at a moment of time.

But to my surprise, the problem actually is reproducible on a local too, just having the Drupal caches on!

Steps to reproduce:
* have all Drupal caching on
* set the closing time to be something that happens soon and let the time pass
* reload page. The status flag should change to "closed" but it doesn't. Neither for authenticated nor unauthenticated requests.

The content updates on reload if page_cache & dynamic_page_cache modules are disabled, or if the node is re-saved with no changes, or drush cr is run.

For us to be able to use this open/closed formatter at all, we really need to have a mechanism to update its content dynamically. Hence, my patch rerolled here against the current dev / 1.8. You can test the patch by retrying the steps above. You might see the old value flashing briefly before it gets updated.

@jhuhta, thanks for your feedback.
Normally drupal best practice says to open a new issue. But let us decide on that later.

I havesome questions, but they are nicely anwered in your post: anonymous user vs. authenticated user? Both mentioned caching modules activated or not:
-- Internal Dynamic Page Cache - "Caches pages, including those with dynamic content, for all users."
-- Internal Page Cache - "Caches pages for anonymous users and can be used when external page cache is not available."

Drupal core handles both differently, and has some other problems, so reasons for the problem might be:
- The module calculates a wrong expiration timetimestamp (This should be corrected in the module)
- The expiration timestamp is not propagated correctly from field level to page level (This should not be a problem of this module,and must be solved in core)
- The caching for anonymous user is flawed. As work-around, a cron job for table cache_render is added in #3312511: Fix Field cache for anonymous users using Cron job in Status formatter. Regarding this, please check #3350699: Cron for cache handling breaks badly where we are actively working on it.

The problem for anonymous users should also be solved: display the node, wait for the time to expire, run cron, re-display the node.
Is that the case, too?
There should be no problem with authenticated users (at least in my basic test system). So, I am surprised by that test result.

I understand your solution is to move the open/closed status to JS. In the past, I discarded that solution, since all errors were due to calculation errors in the office_hours module.

At this point, I am happy with your solution, since it is better than the cron job solution.
I will review it. I think it is beter to open a new issue "Handle Status formatter in JS"
Please check the other issues.

Alos, it might be needed to refresh the complete formatter, since the module now supports exception dates and seasons.

Thank you for your reply! As per your advice, I opened a follow up issue: #3351280: Fix Field cache for anonymous users using JS callback in Status formatter. Let's continue the discussion there about my potential solution.

By removing just one of the *_cache modules, I was able to get the status working for either anonymous or authenticated users, but I don't have the exact results here anymore.

I'm not yet familiar with the new Seasons feature, and we have implemented the opening time exceptions in a custom way long time ago.