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:

  1. /gitlab-org/gitlab-ce/pipelines
  2. /gitlab-org/gitlab-ce/pipelines?page=1
  3. /gitlab-org/gitlab-ce/pipelines?page=2
  4. /gitlab-org/gitlab-ce/pipelines?page=1
  5. /gitlab-org/gitlab-ce/pipelines?expanded=true

This is how things work right now:

  1. Query strings are ignored in ETag matching.
  2. User requests /gitlab-org/gitlab-ce/pipelines?page=1. This gets cached as ETag 12345.
  3. If the frontend sends a GET request for /gitlab-org/gitlab-ce/pipelines?page=2 with no If-None-Match header, it will get the right result.
  4. If the frontend sends a GET request for /gitlab-org/gitlab-ce/pipelines?page=2 or /gitlab-org/gitlab-ce/pipelines?expanded=true with an If-None-Match ETag 12345, it will get the wrong result.
  5. 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.

Edited by Stan Hu