Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[HttpFoundation] Adds support for the immutable directive in the cache-control header#22932
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
cba133a toeb37144Compareeb37144 to4b03f1bCompare
|
4b03f1b tocbf1dccCompareabcdae4 tof9bbae7Comparecf6bc1f tofba0d33Compare| * | ||
| * @return $this | ||
| */ | ||
| public function setImmutable($immutable = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
a default value doesn't make much sense here IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
OK I understand. My idea was calling thesetImmutable method without argument like setPublic/setPrivate method.
return (newResponse('SOME RESPONSE DATA')) ->setPublic() ->setImmutable();
return (newResponse('SOME RESPONSE DATA')) ->setPublic() ->setImmutable(true);
I like the first example of code (callingsetImmutable without boolean value). Maybe it will be better to implement opposite methodsetMutable (which remove the flag immutable from theCache-Control header) and to remove the argument fromsetImmutable method completely. What is your opinion about it?
| @@ -621,6 +621,34 @@ public function setPublic() | |||
| } | |||
| /** | |||
| * Marks the response as "immutable". | |||
| * | |||
| * @param bool $immutable Enables or disabled the immutable directive. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"disables"
| @@ -992,6 +1020,10 @@ public function setCache(array $options) | |||
| } | |||
| } | |||
| if (isset($options['immutable'])) { | |||
| $this->setImmutable($options['immutable'] ? true : false); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
How about a cast to book?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
*bool lol
fba0d33 to2143effCompare60ca02e to7f603b7Compare| */ | ||
| public function isImmutable() | ||
| { | ||
| return $this->headers->getCacheControlDirective('immutable') ? true : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
justreturn $this->headers->hasCacheControlDirective('immutable');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fixed, thanks for hint
695fd9a to4cf01a1Compare4cf01a1 to33573c6CompareThank you@twoleds. |
…ive in the cache-control header (twoleds)This PR was merged into the 3.4 branch.Discussion----------[HttpFoundation] Adds support for the immutable directive in the cache-control header| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#21425| License | MIT| Doc PR |Added support for the immutable directive in the cache-control header, tries toresolve#21425.Commits-------33573c6 Added support for the immutable directive in the cache-control header
Uh oh!
There was an error while loading.Please reload this page.
Added support for the immutable directive in the cache-control header, tries toresolve#21425.