Convert all uses of $_SESSION to symfony session syntax

Adding tags...

This is my first pass for your review @cosmicdreams. It's the same patch from the previous thread, moved here for continuity.

Paul

drupal_get_session is an existing function? Can't find the function itself in the patch.

@aspilicious I'm building this while following other work. I'm working from an isolated branch off HEAD. Once the patch implementing the function goes in, I can backport it.

I believe it or something similar is coming in from another source/Symfony conversion. I suspect it'll be found in namespace Drupal\Core\something\something. If not, I'll investigate the mechanism Drupal will be using to access the $_SESSION data and apply the means recommended.

Using $session->set(), $session->get(), $session->has(), $session->remove(), and the rest become the means to access and manipulate the $_SESSION contents.

http://api.symfony.com/2.0/Symfony/Component/HttpFoundation/Session.html

As I understand it, drupal_get_session() was removed some time ago, I'm not yet certain how we'll be accessing the Symfony session object in HttpFoundation. I'm still figuring out namespace and scoping issues. The $_SESSION is a static global (bad for direct use in the new scheme of things). AFAIK, this puts the code in compliance with Symfony's methods, and will improve testing with a series of tests that cover all the session handling code.

There exists in HttpFoundation MockArraySessionStorage to be used for testing purposes.

Please note the 'do-not-patch' portion of the patch name. What's contained within the file is WIP and not intended to ever be applied to core in its current state. We're all learning Symfony here. I'm also still learning Drupal.

@cosmicdreams: Thanks a lot. :) I'm still ferreting out the code from Symfony. Didn't like the syntactically wrong $session->set() I was using, and knew it wouldn't stand.

How is the $_SESSION data accessed the Symfony way?

I get:

adding the 'use Drupal/Core/Session;'

only one $_SESSION

I don't yet understand:

how we obtain the session object when we need to access it.

Which is why I threw in the drupal_get_session() stuff. When I understand how the session will be accessed, I can replace it with $session = new Session() or DrupalSession(), $session = some_global_utility_function(), or whatever the community determines is the best implementation method.

My thinking, at this point, leans toward creating a $session variable whenever we need to access the data on the fly. There may be a global function for 'bc' as long as it's decided to offer it, but that's actually a separate issue unless we need something in the interim.

A lot of the references remaining are in the tests. All the actual module code has been surveyed, and I believe all the single dimensional references have been ported (except the tests at this point).

Just saw #9, which do we use? Drupal\Core\Session? or Drupal\Core\Session\Session? or something more or else?

Minor changes to comments... tenses, word choice, case, etc.

More corrections, removed several missed uses of $_SESSION, added $session = drupal_session_get(); where $_SESSION (multi-dimensional) references obtain in modules (aggregator--user).

Here's a question:

In the case of:

modules/dblog/dblog.admin.inc:  $_SESSION[] = array();

I'm not entirely sure what's happening. The $_SESSION is being cleared and reinitialized with an empty array(), that I get.

Reviewing class Sessions in Drupal\Core\Sessions, I find no equivalent method to clear and re-initialize the Session object. There is only the one reference, and it is the last one standing.

Any ideas?

Here's a patch that removes (I believe) all but 1 reference to $_SESSION from everything (except tests) in the core subdir and children.

The suggested syntax for multidimensional $session arrays has been implemented as well.

I'm reluctant to submit for automatic testing, until the new tests (for Drupal's Symfony Session usage) have been written and tested.

I'll start working on it. It's growing into one monster patch. 135K and rising. Managed to eliminate the usages in the test modules. Going back to change the $session_all = $session->all() usages to the recommended practice. Hope to finish by nightfall. Need the weekend for school projects so I won't be available for more than quick questions. 4 projects due Monday.

@cosmicdreams: Here's the final edits (I think). I rolled it up again.

Accomplished:

-- All references to $_SESSION in code from core directory and below have been ported to Symfony usage as discussed.

-- All references to $_SESSION tests have been ported to Symfony usage as discussed.

Remaining references:

In lib/Drupal/Core/Cache/DatabaseBackend.php

In lib/Drupal/Core/Session/Storage/DrupalSessionStorage.php

In vendor/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php

Anything else is in a comment or text string left unchanged as I don't know how it's going to be documented.

@cosmicdreams: Re: #21. Gonna need some help with that.

Was fairly clear on Git workflows until today. :) Now I'm confused.

Here's what I did:

git clone 8.x into local directory.
git branch local
git checkout local
git branch yourpatch - to isolate your changes from HEAD and mywork
git checkout yourpatch
git apply yourpatch
git commit -a
git branch mywork - created branch for my editing
git checkout mywork
did a bunch of editing.
git commit -a - in mywork branch.

At this point, I should have different versions. 8.x - base == local; yourpatch committed; then mywork also committed.
I figure at this point, I need to merge mywork with yourpatch, then diff to 8.x, is that right?

Anyway, it didn't work. lol I think I didn't do it right. Been reading Git Community book, Drupal's Tutorials, etc. and I come away clearer on some points and more confused on others. Can you help? My brain is beginning to hurt.

Added:
I tried again. I believe I successfully merged mywork and yourpatch branches. git status from local (8.x) shows it 2 commits ahead of 8.x. At this point, if I'm reading everything correctly, while on the local branch a ...

git diff -p 8.x > '1545680-SESSION-replaced-with-Symfony.patch

should yield the desired patch. Have I got that right?

@cosmicdreams: Here's the re-roll as described in #22.

Hope this fixes everything. *whew*

Notes on the patching/editing I did.

As far as passed by value vs. passed by reference, I assumed since we were operating on the $_SESSION variable, i.e. a global structure, that it was passed by reference each time, and was consistently so for the duration of the request, which should be longer than the duration of a given function. At least that was my thinking. If that's mistaken, let me know.

As far as the setting as soon as possible, I used drupal_session_get() in the broadest scope needed by the code to process the same data, but in no case greater than function scope. Occasionally I was able to narrow the scoping tighter than that. When it was clearly safe to do so, I did.

In examining the patch, I found the following:

File: bootstrap.inc
Func: drupal_get_messages()
Line: 1683

if ($messages = drupal_set_message()) {

+++ b/core/includes/bootstrap.inc
@@ -1674,10 +1679,13 @@ function drupal_set_message($message = NULL, $type = 'status', $repeat = TRUE) {
 function drupal_get_messages($type = NULL, $clear_queue = TRUE) {
+  $session = drupal_session_get();
+  $s_messages = $session->get('messages');
   if ($messages = drupal_set_message()) {
     if ($type) {
       if ($clear_queue) {
-        unset($_SESSION['messages'][$type]);
+        unset($s_messages[$type]);
+        $session->set('messages', $s_messages);
       }

My IDE (NetBeans) indicates accidental assignment in the if statement. I suspect it isn't accidental, but I don't know enough about the internal structures being referenced to be certain.

Saw changes you made in form_test.module. Your style here is much clearer than mine.

Anyway, that's all so you can see where I'm coming from. Any other questions, feel free to let me know.

Fixing bogus tags.

FYI messages should use the flash messenger, maybe this part of the code could still live using the old fashion $_SESSION access (which should work) and be the target of another issue?

Ho, I almost forgot this one! I should get back on working with sessions, still didn't get that much feedback.

Working on a big big rerole / usage of the container etc.

.

So ... this is just a rerole of the existing patch, with all the long leftover comments, todos, fixmes and all other kind of uglyness.

Additional this patch provides the session as part of the container.

Note: login doesn't work yet, see user.module::user_login_finalize().

__***sigh***__

Seems to be a duplicate of #1858196: [meta] Leverage Symfony Session components

Just in case that's important, this don't misses the session files.

there is some nice work done here, but it is definitely a dupe
#1858196: [meta] Leverage Symfony Session components

Restarted after the session is finally available from the $request: #2473875: Convert uses of $_SESSION to symfony session retrieved from the request