Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ConsiderUrlQuery flag #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Felices
Copy link

@Felices Felices commented Dec 20, 2024

Existing behavior

The existing simplecache code does not include URL query string parameters in the cache key. This means that a GET request to the same URL with different query strings will return the same cached response.

Issue

I have a video management server project where an endpoint returns an image, and the user can set the desired size with the URL query (e.g. http://my_server/image_endpoint?width=480). The remote cameras are connected via a slow metered connection, so I want to cache responses.

However, if the user requests:

  1. http://my_server/image_endpoint?width=480
  2. And then, http://my_server/image_endpoint?width=1920,
    The 480px-wide image, cached for (1), is used as the cached response for (2), even though the user requested a 1920px-wide image.

Why not have the user provide a JSON payload with the desired image width instead of a URL parameter?
The customer wants to easily access images by pasting a URL into their browser. They do not want to construct HTTP requests with payloads with CURL or similar tools.

Suggested change in this MR

This MR adds a ConsiderUrlQuery flag. In its default false state, existing behavior is preserved. If set to true, query parameters will be used in the cache key.

Perhaps in the future, it could be useful to provide a list of query string parameters to include/exclude from the cache key. However, I am new to this repo's conventions and would like to ensure I understand the maintainer's preferences before adding more changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant