Removed cookie binding to session

[ekmst]

Hello!

• Type: improvement
• Link to issue: Cookie is coupled with the Session #11770

In raising this pull request, I confirm the following:

• I have read and understood the Contributing Guidelines
• I have checked that another pull request for this purpose does not exist
• I wrote some tests for this PR
• I have updated the relevant CHANGELOG
• I have created a PR for the documentation about this change

Small description of change:
Removed cookie binding to session
Phalcon\Http\Cookie no longer depends on the session service and data will not be duplicated in the session. This made it difficult to use cookies in stateless applications (SPA).

Thanks

[niden]

@ekmst can you rebase and send this to the 4.1.x branch please?

[sergeyklay]

I'm not opposed to this change, but I would like to see a more detailed description of these changes. Not just "fixed issue XXX". What suggested in return? Does it breaks backward compatibility? If so, what are workarounds? Why can't we go further with this?

Regarding change log: why you put these changes in the "Changed" but not in the "Removed" section? Also move please this to the 4.1.x version in the change log.

[codecov]

Codecov Report

Merging #14649 into 4.1.x will increase coverage by 0.11%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##            4.1.x   #14649      +/-   ##
==========================================
+ Coverage   67.49%    67.6%   +0.11%     
==========================================
  Files         483      483              
  Lines      111403   111414      +11     
==========================================
+ Hits        75192    75324     +132     
+ Misses      36211    36090     -121

[niden]

This will definitely break BC not by means of interfaces but by functionality itself. For instance right now I set my cookies and I know they will go in the session. With this change they will not so I will need to write that code myself.

There are plans to completely rewrite the HTTP layer and offer cookie management with different storage engines i.e. files, session etc. but we are not there yet.

@ruudboon @sergeyklay Thoughts on this? If we are to use this we will need to introduce also some sort of a storage i.e. take the session storage from where it was (see this PR) and put it somewhere else so that people can use it at least for now.

[ekmst]

I am currently developing a SPA application where sessions are not used at all. But I need to work with cookies. But since the session is not used, I get errors that the session service was not found. And now I have to use a stub to avoid this.

I do not quite understand why cookies are also stored in sessions?

[niden]

@ekmst storing cookies in files or the session are the most common storage engines.

This code is really ancient and has never seen much love - we just let it be. Definitely needs to be reworked so what you did I would say is step 1 and we need step 2 to at least have the option to store the cookies in the session (current functionality)

I will see what I can come up with in the next few days and will ping you so that we can align efforts.

[ekmst]

@niden
Can we save cookies in the session only if the session service is defined in the DI container? If the session service is not defined, then we work with cookies and we will not throw an exception.

Or we can enter a flag to explicitly indicate that you want to save cookies in the session.

[niden]

@niden
Can we save cookies in the session only if the session service is defined in the DI container? If the session service is not defined, then we work with cookies and we will not throw an exception.

Or we can enter a flag to explicitly indicate that you want to save cookies in the session.

That is actually a great idea. If we mention this in the documentation it should be fine and most people would not be affected if they already have the session service registered.

@ekmst can you make the adjustments?

The above will definitely help and give us time until we redesign the http layer.

[ekmst]

@niden
Yes, I will try to do it soon.

[niden]

@ekmst looking good so far. Once you are done make sure the changelog is updated and ping me so that I can merge this

[ekmst]

@niden

Done

[ruudboon]

With these changes I wouldn't mind to push this into 4.0.x. Don't think we're breaking anything
Thnx ekmst

[niden]

With these changes I wouldn't mind to push this into 4.0.x. Don't think we're breaking anything
Thnx ekmst

We will have to cherry pick them because this is a different branch. We can talk about this in TG.

[niden]

Thank you @ekmst