Upgrade to Slim 3 #213
Upgrade to Slim 3 #213
Conversation
| $res = $res->withHeader('Content-Type', 'application/json'); | ||
| $res->getBody()->write($content); | ||
| } else{ | ||
| $res->getBody()->write($content . "\n<!-- Cached response -->"); |
There was a problem hiding this comment.
is this comment fine to append to a JSON content type?
There was a problem hiding this comment.
I don't think so. That's why I'm only appending it when it isn't a json document.
There was a problem hiding this comment.
Stupid me :) didn't look at it properly
app/helpers/CacheMiddleware.php
Outdated
| private function getCacheKey($req) | ||
| { | ||
| private function isJsonData(Request $req) { | ||
| return strpos($req->getUri()->getPath(), 'data/') !== false; |
There was a problem hiding this comment.
I haven't tested it, but what if the URL contains data/ as in https://developer.piwik.org/guides/log-data/xxx? This URL wouldn't work but there might be something in the future. I haven't looked yet at the structure of data URLs but maybe we can check if it starts with a certain URL or something else? Just a thought...
There was a problem hiding this comment.
You are right, that could cause issues in the future. I’ll replace it with something more specific
There was a problem hiding this comment.
This should be fixed now in 667cc0b
|
The search was broken because now that the json is send with the correct header it is parsed by jquery and doesn't need to be parsed again. I quickly fixed it in 31bc37f Keep in mind that I have already written a new search that mores far better on the frontend and backend (see https://issues.lw1.at/) |
|
@tsteur |
|
@Findus23 so far everything seems to work but the logger as you mentioned. Is there a chance we can somehow keep the |
|
@tsteur The log class doesn't work anymore as Slim 3 doesn't include any logging feature anymore, but recommends to use monolog instead. Unfortunately I don't know how to access the class in the |
|
@Findus23 what's the current status here? |
|
I think this is the branch where I extracted the PHP changes and it should be working. (apart from logging) |
|
@Findus23 Be awesome if we could merge your work soon and benefit from the improvements 👍 |
|
🚀 |
Cherry-picked all changes from #200 that don't affect JS/CSS/Twig Templates. Also doesn't include the full text search.
I'll test if this breaks anything soon.To do after/while merge: