feat: add last modified information to response #1844
Conversation
| $rows = $this->rowService->findAllByTable($tableId, $this->userId, $limit, $offset); | ||
| $response = new DataResponse($this->rowService->formatRows($rows)); | ||
| $table = $this->tableService->find($tableId); | ||
| $lastModified = new \DateTime($table->getLastEditAt()); |
There was a problem hiding this comment.
Are we sure that any change of rows would update the getLastEditAt date? Otherwise this would not have much benefit i think.
There was a problem hiding this comment.
I did not double check it yet, it is merely my expectation. I was wondering whether this is also updated, when a row is being deleted.
| return new DataResponse($this->rowService->formatRows($this->rowService->findAllByTable($tableId, $this->userId, $limit, $offset))); | ||
| $rows = $this->rowService->findAllByTable($tableId, $this->userId, $limit, $offset); | ||
| $response = new DataResponse($this->rowService->formatRows($rows)); | ||
| $table = $this->tableService->find($tableId); |
There was a problem hiding this comment.
We could move this line first and return a 304 early in case the if-modified-since matches, before doing the possibly heavier operations of fetching the rows
There was a problem hiding this comment.
Yes, definitely. First I thought i had to analyze the rows that were fetched, then i was reminded about the last modificated set in the table (if that really works as i think). I pushed this to not have it rotting away locally and turning to legend in first place 🙂
Signed-off-by: Arthur Schiwon <[email protected]>
enjeck
force-pushed
the
poc/noid/last-modified
branch
from
0af84d9 to
458b886
Compare
Quick PoC: NotModifiedMiddleware respects last modified information stored with a data response. The business logic still runs, but at least clients will not need to do any heavy lifting, as only 304 is returned if the condition pertains.
Dropping it here quickly in case someone wants to pick this up.