Problem/Motivation

After #2484991: Add the session to the request in KernelTestBase, BrowserTestBase, and drush a session is now always on the request. Hence, calls to hasSession() should be removed.

Steps to reproduce

Proposed resolution

Trivial changes

Trivial changes do not require any additional modifications (e.g., no changes in tests or other areas of the code base).

  • Remove 3 occurrences from core/includes/install.core.inc (trivial)
  • Remove 1 occurrence from core/lib/Drupal/Core/Entity/EntityDeleteMultipleAccessCheck.php. This one could be replaced by a call to \Drupal::service('session_configuration')->hasSession($request) which might have been the original intention anyway.
  • Remove 1 occurrence from core/lib/Drupal/Core/StackMiddleware/Session.php (trivial)
  • Remove 1 occurrence from core/lib/Drupal/Core/TempStore/SharedTempStore.php (trivial)
  • Remove 1 occurrence from core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php (trivial)
  • Remove 1 occurrence from core/lib/Drupal/Core/Update/UpdateKernel.php (trivial)
  • Remove 2 occurrences from core/lib/Drupal/Core/DrupalKernel.php (trivial). Leave one occurrence in preHandle().
  • Remove 1 occurrence from core/modules/big_pipe/src/Controller/BigPipeController.php. This one could be replaced by a call to \Drupal::service('session_configuration')->hasSession($request) which might have been the original intention anyway.
  • Remove 1 occurrence from core/modules/comment/src/Controller/CommentController.php (trivial)
  • Remove 2 occurrences from core/modules/user/src/Authentication/Provider/Cookie.php (trivial)
  • Remove 1 occurrence from core/modules/views/src/ViewExecutable.php (trivial)

Non-trivial changes

These changes require additional modifications (e.g., changes in tests or other areas of the code base).

  • Remove 1 occurrence from core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php. Requires deletion of one Test in core/tests/Drupal/Tests/Core/Cache/Context/SessionCacheContextTest.php.
  • Remove 1 occurrence from core/lib/Drupal/Core/Form/FormBuilder.php. Requires a fix in core/modules/views_ui/src/ViewUI.php for the views preview functionality. Also requires a fix in core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php.
  • Remove manual session start code from core/lib/Drupal/Core/TempStore/PrivateTempStore.php. This has been dead code in production for a long time. Requires a fix in core/tests/Drupal/KernelTests/Core/TempStore/AnonymousPrivateTempStoreTest.php.

Note to reviewers

This issue requires manual testing. Areas to test:

  • Installation via the web installer
  • Running update.php
  • Using views and views previews
  • Using the layout builder (private temp store)
  • Using big pipe

Remaining tasks

  • Implement
  • Review
  • Manual Testing

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3413153

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

znerol created an issue. See original summary.

prabuela’s picture

.

andypost’s picture

znerol’s picture

Status: Active » Needs work

znerol’s picture

Status: Needs work » Needs review
daffie’s picture

Status: Needs review » Reviewed & tested by the community

All the code changes look good to me.

znerol’s picture

Issue summary: View changes
znerol’s picture

Issue summary: View changes
alexpott’s picture

Status: Reviewed & tested by the community » Needs work

I think we need to deprecate the startSession method in PrivateTempStore - can an MR be opened against 10.3.x that does this. There are plenty of things that extent PrivateTempStore in contrib and one of them may well be relying on this method - or it may happen in custom code too.

znerol’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Deprecation appears correct and points to the CR link.

alexpott’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 0d54ea9 and pushed to 11.x. Thanks!
Committed 57ce2e3 and pushed to 10.3.x. Thanks!

  • alexpott committed 57ce2e37 on 10.3.x
    Issue #3413153 by znerol, alexpott: Remove calls to Request::hasSession...

  • alexpott committed 0d54ea9c on 11.x
    Issue #3413153 by znerol, alexpott: Remove calls to Request::hasSession...

Status: Fixed » Closed (fixed)

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