8000 [HttpFoundation] Cookie mutation · Issue #35212 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation] Cookie mutation #35212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ns3777k opened this issue Jan 4, 2020 · 1 comment · Fixed by #35215
Closed

[HttpFoundation] Cookie mutation #35212

ns3777k opened this issue Jan 4, 2020 · 1 comment · Fixed by #35215
Labels
Feature HttpFoundation

Comments

@ns3777k
Copy link
Contributor
ns3777k commented Jan 4, 2020 •
edited
Loading

Description
Cookie class is immutable and thus its settings can be set only during construction via constructor or static create method. Both methods accept 1 required argument (which is cookie name) and 8 optional arguments.

Most of the time, I don't need httpOnly flag which makes me specify 7 arguments almost every time I wanna create a cookie :-) Setters can bring much more effort (IMO. see example below). Setting sameSite will require me to specify all 9 previous arguments.

Since now only cookie name is required we can avoid BC by moving validation logic into setters and calling them from the constructor.

Example
For now, I create a cookie like this (I specify 7 arguments 'cause I wanna disable httpOnly):

$cookie = Cookie::create(
    RegionSwitcher::REGION_COOKIE, $regionSlug, new DateTime('+1 year'), '/',
    $baseDomain, null, false
);

With setters, I can only call 4 methods which looks much nicer to me:

$cookie = Cookie::create(RegionSwitcher::REGION_COOKIE)
    ->setValue($regionSlug)
    ->setExpiration(new DateTime('+1 year'))
    ->setDomain($baseDomain)
    ->disableHttpOnly();

I can make a PR if accepted. Thanks in advance!
@ns3777k ns3777k mentioned this issue Jan 4, 2020
Merged
@florianajir
Copy link

I'm stuck to a similar problem, I get my cookie from a external service after an http request and I need to set the domain to an other value so the easy way could be

$cookie= Cookie::fromString(...);
$cookie->setDomain(...);

@fabpot fabpot closed this as completed Jan 30, 2020
fabpot added a commit that referenced this issue Jan 30, 2020
@fabpot
feature #35215 [HttpFoundation] added withers to Cookie class (ns3777k)
a2b6085 
This PR was squashed before being merged into the 5.1-dev branch (closes #35215).

Discussion
----------

[HttpFoundation] added withers to Cookie class

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #35212
| License       | MIT
| Doc PR        | -

I was quite descriptive in the issue :-)

The main idea is to get the interface for changing a cookie to avoid every unneeded argument in the constructor.

Current:

```php
$cookie = Cookie::create(
    RegionSwitcher::REGION_COOKIE, $regionSlug, new DateTime('+1 year'), '/',
    $baseDomain, null, false
);
```

This PR:

```php
$cookie = Cookie::create('foo')
            ->withValue('bar')
            ->withExpiresTime(strtotime('Fri, 20-May-2011 15:25:52 GMT'))
            ->withDomain('.myfoodomain.com')
            ->withSecure(true);
```

Every `wither` returns a copy of current cookie with requested setting set. Cookie class remains immutable.

Commits
-------

549afaa [HttpFoundation] added withers to Cookie class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature HttpFoundation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[HttpFoundation] added withers to Cookie class ns3777k/symfony
4 participants
@fabpot @xabbuh @florianajir @ns3777k
0