Backport Rack::Request cache helpers. #2056
Conversation
ioquatix
force-pushed
the
3-0-rack-request-cache-for
branch
from
d893b2b
to
a71e559
Compare
* Per-class cache keys for cached query/body parameters. * Use the query parser class as the default cache key.
ioquatix
force-pushed
the
3-0-rack-request-cache-for
branch
from
a71e559
to
b41fb6d
Compare
There was a problem hiding this comment.
I would consider this a new feature, not a bug fix, and would usually be against backporting. However, considering what has already been backported was much more invasive/disruptive, I guess there isn't much additional harm in backporting this. So I'm OK with the idea of backporting this. See inline comment regarding cache_key (which was changed after I approved the related PR), which I think should be fixed before backporting.
| # behaviour. This includes sub-classes that override query_parser or | ||
| # expand_params. | ||
| def cache_key | ||
| query_parser.class |
There was a problem hiding this comment.
I still think this is a mistake, and think the onus should be on the framework to override this appropriately. If you have to base it on the query parser, query_parser would be better than query_parser.class, considering that the default is to use a singleton QueryParser instance (Utils.default_query_parser), and singleton classes allow for the changing of behavior for individual instances. It would be best to fix that first, and include that in the backport.
|
We decided to give up on this, and change the query parser behaviour back to 2.x where nil values can be exposed. |
I would characterise this as a bug fix, as without it, the cache is not computed correctly.
#2054