Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Proxy response caching #247

Merged
merged 2 commits into from
Aug 19, 2020
Merged

Fix Proxy response caching #247

merged 2 commits into from
Aug 19, 2020

Conversation

giggsey
Copy link
Contributor

@giggsey giggsey commented Aug 18, 2020

Only add the Response stream if the Request results in a cache miss.

Fixes #207

@giggsey
Copy link
Contributor Author

giggsey commented Aug 18, 2020

I think the distribution function in ProxyController can also do the same thing here

@codecov
Copy link

codecov bot commented Aug 18, 2020

Codecov Report

Merging #247 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #247   +/-   ##
=========================================
  Coverage     99.68%   99.68%           
  Complexity     1513     1513           
=========================================
  Files           250      250           
  Lines          4396     4404    +8     
=========================================
+ Hits           4382     4390    +8     
  Misses           14       14           
Impacted Files Coverage Δ Complexity Δ
src/Controller/ProxyController.php 100.00% <100.00%> (ø) 9.00 <4.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3730b87...6fb4f4a. Read the comment docs.

@giggsey
Copy link
Contributor Author

giggsey commented Aug 18, 2020

Before:

curl -w "@.curl-format.txt" 'http://127.0.0.1:8000/p/symfony/http-foundation' -H 'If-Modified-Since: Tue, 18 Aug 2020 18:37:05 GMT' -H 'Host: repo.repman.wip' -v
*   Trying 127.0.0.1:8000...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8000 (#0)
> GET /p/symfony/http-foundation HTTP/1.1
> Host: repo.repman.wip
> User-Agent: Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; Trident/5.0)
> Accept: */*
> Referer: 
> If-Modified-Since: Tue, 18 Aug 2020 18:37:05 GMT
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Accept-Ranges: bytes
< Cache-Control: public
< Content-Length: 2374308
< Content-Type: application/json
< Date: Tue, 18 Aug 2020 19:06:20 GMT
< Date: Tue, 18 Aug 2020 19:06:20 GMT
< Host: repo.repman.wip
< Last-Modified: Tue, 18 Aug 2020 18:37:05 GMT
< 
{"packages":{"notadd/wechat":{"dev-master":{"name":"notadd/wechat","description":"Notadd's Wechat Module.","keywords":["framework","cms","member","notadd"],"homepage":"https://notadd.com","version":"dev-m
...
time_namelookup:  0.000021s
       time_connect:  0.000121s
    time_appconnect:  0.000000s
   time_pretransfer:  0.000210s
      time_redirect:  0.000000s
 time_starttransfer:  0.351384s
                    ----------
         time_total:  0.872798s

After:

curl -w "@.curl-format.txt" 'http://127.0.0.1:8000/p/symfony/http-foundation' -H 'If-Modified-Since: Tue, 18 Aug 2020 18:37:05 GMT' -H 'Host: repo.repman.wip' -v
*   Trying 127.0.0.1:8000...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8000 (#0)
> GET /p/symfony/http-foundation HTTP/1.1
> Host: repo.repman.wip
> User-Agent: Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; Trident/5.0)
> Accept: */*
> Referer: 
> If-Modified-Since: Tue, 18 Aug 2020 18:37:05 GMT
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 304 Not Modified
< Accept-Ranges: bytes
< Cache-Control: public
< Date: Tue, 18 Aug 2020 19:07:38 GMT
< Date: Tue, 18 Aug 2020 19:07:38 GMT
< Host: repo.repman.wip
< 
* Connection #0 to host 127.0.0.1 left intact
    time_namelookup:  0.000024s
       time_connect:  0.000246s
    time_appconnect:  0.000000s
   time_pretransfer:  0.000960s
      time_redirect:  0.000000s
 time_starttransfer:  0.331560s
                    ----------
         time_total:  0.331640s

@akondas
Copy link
Member

akondas commented Aug 18, 2020

Nice, looks promising, I need to verify it, but we'll probably merge it. Thanks 🍻

Response::isNotModified() doesn't need to be wrapped in the if statement, as it'll remove the content (even for a StreamedResponse)
@giggsey giggsey requested a review from akondas August 19, 2020 07:22
@akondas akondas merged commit 139c4f3 into repman-io:master Aug 19, 2020
@akondas
Copy link
Member

akondas commented Aug 19, 2020

Thanks @giggsey 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTP Caching - metadata endpoint
2 participants