shelf
shelf copied to clipboard
dart-lang
Add context to `Response` for including `file` and `file_not_found` to be identified by other `shelf` `Middleware`.
- [x ] I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Updated:
With the Response.context populated with the processed File, other Middleware can manipulate the Response in an optimized way and also ensure the correct processed File. Without that, another Middleware would need to guess the processed File, and in some cases, re-resolve File.stat and also fix the directory path to the "index.html" path.
i see where you're going here.
I'd want to chat with @natebosch or @devoncarew on their thoughts
This change should be documented and tested, too!
With the Response.context populated with the processed File, other Middleware can manipulate the Response in an optimized way and also ensure the correct processed File. Without that, another Middleware would need to guess the processed File, and in some cases, re-resolve File.stat and also fix the directory path to the "index.html" path.
In my use case, I maintain an in-memory cache of shelf Responses. Then I need to verify if the cached Response File has changed using File.stat.
However, my Middleware currently has to recalculate the Response File before caching the Response. This leads to a doubling of the initial Response time due to the additional File resolution, involving calls to .existsSync and statSync, as well as the resolution of the Directory to index.html.
Keep in mind that the primary purpose of a cache is to enhance response time, and the negative impact of these extra calls to statSync is significant.
Is this still something you're interested in landing - can you take a look at adding tests?
This is important.
I will try to add some tests, but I don't have time for that in the next 15 days.
Can you give an example of how you plan to use this? Did you consider any alternative patterns, such as always storing the key shelf_static:file, and storing a separate boolean field for whether it was found?
Also cc @brianquinlan - can you think of any risks in keeping the File object in the heap for longer than we would have previously.
Can you give an example of how you plan to use this? Did you consider any alternative patterns, such as always storing the key
shelf_static:file, and storing a separate boolean field for whether it was found?
You can take a look at real-world code that is waiting for this: https://github.com/Colossus-Services/bones_api/blob/master/lib/src/bones_api_server.dart#L1788
Note that if 'shelf_static:file' and 'shelf_static:file_not_found' are not present, we need to recalculate the File path, validate it, and ensure (again) that it's inside the "root" directory (simulating the same shelf:createStaticHandler logic).
The context['file'] is then used by the cache "middleware", which will use the File instance to validate the cached files and dispose of the cached data if the File changes:
https://github.com/Colossus-Services/bones_api/blob/master/lib/src/bones_api_server_cache.dart#L351
Can you give an example of how you plan to use this? Did you consider any alternative patterns, such as always storing the key
shelf_static:file, and storing a separate boolean field for whether it was found?Also cc @brianquinlan - can you think of any risks in keeping the
Fileobject in the heap for longer than we would have previously.
A File object doesn't represent any native resources so keeping it alive shouldn't do anything except cost a bit of memory.
