Read $_COOKIE superglobal into the request object, add tests, and fix request query config issue. #860

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

mikegreiling commented Mar 19, 2013

Integrates $_COOKIE superglobal into the request object at $request->cookies.

In the process I found a bug in which query params passed into the constructor would overwrite rather than merge with the $_GET superglobal. I fixed this by re-working the parent classes to call parent::__construct() at the end of the constructor so they don't override anything done in _init().

Member

blainesch commented Mar 19, 2013

Shouldn't you use the Cookie session adapter instead of reading it from the request? It may be part of the request, but this gives it a more centralized api. I assume the cookie adapter updates the current array of data, but wouldn't be able to update the Request object, when new cookies are set.

Owner

nateabele commented Mar 19, 2013

This was a previously-discussed API change. It wouldn't necessarily preclude integration with the existing API, we'd just need some sort of binding interface, similar to the FirePHP log adapter.

Contributor

mikegreiling commented Mar 19, 2013

@blainesch would it need to update the request object when cookies are set? The $request->cookies should represent the cookies sent by the user agent, not the current set of cookies returned in the response, so I don't see any reason it should change after initialization.

Contributor

mikegreiling commented Mar 19, 2013

Looks like travis is complaining about something I didn't see in my own tests... I'll look into it...

Contributor

mikegreiling commented Mar 20, 2013

The request object already had a $cookies property inherited from \net\http\Request, this PR merely populates it...

Contributor

mikegreiling commented Apr 23, 2013

Closing in favor of a future PR to be based on #902

mikegreiling deleted the pixelcog:request-cookies branch Apr 23, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment