Handle query parameters in ETag caching
This came up in https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/9627#note_146970778. From https://docs.gitlab.com/ee/development/polling.html:
Do not use query parameters (for example ?scope=all) for endpoints where you want to enable ETag caching. The middleware takes into account only the request path and ignores query parameters. All parameters should be included in the request path. By doing this we avoid query parameter ordering problems and make route matching easier.
We have an issue here because consider the following paths:
/gitlab-org/gitlab-ce/pipelines
/gitlab-org/gitlab-ce/pipelines?page=1
/gitlab-org/gitlab-ce/pipelines?page=2
/gitlab-org/gitlab-ce/pipelines?page=1
/gitlab-org/gitlab-ce/pipelines?expanded=true
This is how things work right now:
- Query strings are ignored in ETag matching.
- User requests
/gitlab-org/gitlab-ce/pipelines?page=1
. This gets cached as ETag12345
. - If the frontend sends a GET request for
/gitlab-org/gitlab-ce/pipelines?page=2
with noIf-None-Match
header, it will get the right result. - If the frontend sends a GET request for
/gitlab-org/gitlab-ce/pipelines?page=2
or/gitlab-org/gitlab-ce/pipelines?expanded=true
with anIf-None-Match
ETag12345
, it will get the wrong result. - If a new pipeline is created,
/gitlab-org/gitlab-ce/pipelines
gets expired, so any subsequent requests to/gitlab-org/gitlab-ce/pipelines?XXXX
will get invalidated.
Another words, we're putting the burden on the frontend to be intelligent as to whether to send the ETag along with a request, but this doesn't feel right.
We could hash the key-value pairs as part of the ETag route matching, but we'd have to be careful here. We can't simply treat /gitlab-org/gitlab-ce/pipelines
and /gitlab-org/gitlab-ce/pipelines?expanded=true
as completely unrelated routes because we need to ensure we invalidate both routes with a new pipeline.
https://stackoverflow.com/a/2784613 has some discussion about this problem.