Problem/Motivation

We have the session cache context in core, to vary something per session. A related, but far less specific and therefore better cacheable variant of that is to vary something per "session or not". (This is a boolean.)

We need this for BigPipe: #2469431-198: BigPipe for auth users: first send+render the cheap parts of the page, then the expensive parts.

A similar one is user vs. user.is_super_user: the former has many, many possible values, the second is a boolean.

Proposed resolution

Add session.exists cache context.

Remaining tasks

  1. Code.
  2. Test coverage.
  3. Review.
  4. RTBC.
  5. Commit.
  6. Update https://www.drupal.org/developing/api/8/cache/contexts.

User interface changes

None.

API changes

API addition: new session.exists cache context.

Data model changes

None.

CommentFileSizeAuthor
#2 session_exists_cache_context-2671988-2.patch7.23 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: Add SessionExistsCacheContext » Add SessionExistsCacheContext ('session.exists')
Assigned: Wim Leers » Unassigned
Status: Active » Needs review
Issue tags: +API addition
FileSize
session_exists_cache_context-2671988-2.patch7.23 KB

Lifted this from the https://www.drupal.org/project/big_pipe module.

Test coverage was added at #2671958: SessionExistsCacheContext test coverage.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Issue summary: View changes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented 
+++ b/core/core.services.yml
@@ -55,6 +55,11 @@ services:
+      - { name: cache.context}

Nit: missing a space.

(I'll fix this after others have reviewed this, or a committer could also fix this on commit.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commented
Status: Needs review » Reviewed & tested by the community

RTBC - Looks great to me :)

  • catch committed 4a67b9d on 8.1.x 
    Issue #2671988 by Wim Leers: Add SessionExistsCacheContext ('session....
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

I wasn't sure about the naming here, but all the other options are worse (like HasSession etc.). Committed/pushed to 8.1.x, thanks!

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Issue summary: View changes

Did Update https://www.drupal.org/developing/api/8/cache/contexts., now this issue is completely done. (Diff: https://www.drupal.org/node/2459039/revisions/view/9158150/9416754.)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Related issues: +#2675534: Fix 'Drupal.Commenting.FileComment' coding standard again

Very very minor coding standard followup... #2675534: Fix 'Drupal.Commenting.FileComment' coding standard again

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

RTBC'd :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.