Problem/Motivation

There is currently no easy way to register additional objects implementing SessionBagInerface on the session.

Proposed resolution

  • Choose a container tag for session attribute bags. E.g., session_bag
  • Add service_collector tag to the session service definition
  • Add tests for custom session bags

Remaining tasks

  1. Implement
  2. Review
  3. Commit

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#9 3108967-9.patch9.88 KBandypost
#9 interdiff.txt3.44 KBandypost
#8 interdiff-8.txt885 bytesznerol
#8 3108967-8.diff9.56 KBznerol
#7 3108967-7.patch9.86 KBalexpott
#7 2-7-interdiff.txt2.92 KBalexpott
#2 3108967-2.diff8.22 KBznerol

Comments

znerol created an issue. See original summary.

znerol’s picture

znerol commented
Title: Introduce a compiler pass and a container tag to register attribute bags on the session » Introduce a container tag to register session bags
Issue summary: View changes
StatusFileSize
new 3108967-2.diff8.22 KB
znerol’s picture

znerol commented
Status: Active » Needs review
andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +Needs change record

It looks great, except CR I'd say RTBC

znerol’s picture

znerol commented

Right. CR draft is here.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs change record

I think it really for commitrer's eyes

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2-7-interdiff.txt2.92 KB
new 3108967-7.patch9.86 KB

This looks like a really great addition.

I think the only thing that is missing is documentation in the session section of core/core.api.php - I've added most of the CR to the docs but I think the last part

 * Note that Drupal aggressively destroys empty sessions in order to allow for
 * page level caching as much as possible. Thus, it is important for session bag
 * implementations to provide methods for cleaning up obsolete data. I.e.,
 * default state must map to the empty session bag. Also client code accessing a
 * custom session bag must be designed such that it cleans up obsolete data from
 * the session bag.

could do with some work. It feels a bit obscure and not prescriptive enough.

There's a couple of coding standards to be fixed too...


FILE: ...ts/modules/session_test/src/Controller/SessionTestController.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 241 | ERROR | [x] Expected 1 blank line after function; 0 found
 242 | ERROR | [x] The closing brace for the class must have an empty
     |       |     line before it
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 62ms; Memory: 8MB


FILE: ...system/tests/modules/session_test/src/Session/TestSessionBag.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 31 | ERROR | [x] Parameter comment must be on the next line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 50ms; Memory: 8MB

I've done that.

znerol’s picture

znerol commented
StatusFileSize
new 3108967-8.diff9.56 KB
new interdiff-8.txt885 bytes

We already have the following in the overview section:

<?php
 * @section sec_intro Overview
 * The Drupal session management subsystem is built on top of the Symfony
 * session component. It is optimized in order to minimize the impact of
 * anonymous sessions on caching proxies. A session is only started if necessary
 * and the session cookie is removed from the browser as soon as the session
 * has no data. For this reason it is important for contributed and custom
 * code to remove session data if it is not used anymore.
?>

Thus let's just reference that section from the note:

<?php
 * Session data must be deleted from custom session bags as soon as it is no
 * longer needed (see @ref sec_intro above).
?>
andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new interdiff.txt3.44 KB
new 3108967-9.patch9.88 KB

Bit more clean-up for CS

znerol’s picture

znerol commented
Status: Needs review » Reviewed & tested by the community

Only changes since last RTBC was docs. Back to committers.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed d1daa7e to 9.0.x and 9151160391 to 8.9.x. Thanks!

  • alexpott committed d1daa7e on 9.0.x 
    Issue #3108967 by znerol, andypost, alexpott: Introduce a container tag...

  • alexpott committed 9151160 on 8.9.x 
    Issue #3108967 by znerol, andypost, alexpott: Introduce a container tag...

Status: Fixed » Closed (fixed)

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