Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Fix Adding X-Invalidate-Token header only when TOKEN is not null #133

Merged
merged 1 commit into from
Jul 30, 2020
Merged

Fix Adding X-Invalidate-Token header only when TOKEN is not null #133

merged 1 commit into from
Jul 30, 2020

Conversation

kaff
Copy link
Contributor

@kaff kaff commented Jul 29, 2020

Question Answer
JIRA issue EZP-31607
Type Bug
Target version 2.0
BC breaks no
Doc needed no

Prevent attaching X-Invalidate-Token header to Varnish request when http_cache.varnish_invalidate_token parameter is not set or null.

TODO:

  • Implement fix a bug.
  • Implement tests + specs and passing ($ composer test)
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@kaff kaff requested review from ViniTou, alongosz, mnocon and webhdx July 29, 2020 13:49
andrerom
andrerom approved these changes Jul 29, 2020
View reviewed changes
Copy link
Contributor

@andrerom andrerom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, but should perhaps also be backported to 1.0 ?

@ViniTou
Copy link
Contributor

ViniTou commented Jul 29, 2020

@andrerom
If I am right this seems to be ok in 1.0.

https://github.com/ezsystems/ezplatform-http-cache/blob/1.0/src/PurgeClient/VarnishPurgeClient.php#L94

@andrerom
Copy link
Contributor

If I am right this seems to be ok in 1.0.

you are very much right :)

mnocon
mnocon approved these changes Jul 30, 2020
View reviewed changes
Copy link
Member

@mnocon mnocon left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good both on Platform.sh and with regular Varnish usage, approved!

@alongosz alongosz merged commit d98a8ea into ezsystems:2.0 Jul 30, 2020
@kaff kaff deleted the fix/vanish_invalidate_token_only_when_param_not_null branch July 30, 2020 10:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@ViniTou ViniTou ViniTou approved these changes

@mnocon mnocon mnocon approved these changes

@alongosz alongosz alongosz approved these changes

@andrerom andrerom andrerom approved these changes

@webhdx webhdx Awaiting requested review from webhdx

Assignees
No one assigned
Labels
QA approved Ready for MERGE
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kaff @ViniTou @andrerom @alongosz @mnocon