Django

Code

Ticket #5691 (closed: fixed)

Opened 2 years ago

Last modified 4 weeks ago

Add the active language to generate the cache key

Reported by: aaloy@apsl.net Assigned to: aaloy
Milestone: 1.2 Component: Cache system
Version: SVN Keywords: cache i18n jgd-blackboard
Cc: ramiro Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

If you use a cache per parge approach and i18n, and you use a cookie to change your default language, if the page are cached previously you have some chances to get the wrong translation.

lang = translation.get_language()
cache_key = 'views.decorators.cache.cache_header.%s.%s_%s' % (key_prefix, iri_to_uri(request.path),lang)

Attachments

django_utils_cache_patch.diff (2.4 kB) - added by aaloy <aaloy@apsl.net> on 10/05/07 17:36:17.
a patch with the proposed cache key
i18n_cache_keys_r7960_t5691.diff (2.7 kB) - added by andrewbadr on 07/18/08 16:34:35.
cache.patch (2.5 kB) - added by aaloy on 11/21/08 06:17:29.
cache pacth to use lang in the cache key
patch_cache_i18n.patch (14.5 kB) - added by aaloy on 11/25/08 09:19:12.
patch and tests for cache i18n key generation
5691-use-lan-for-cache-key-r10338.diff (7.8 kB) - added by ramiro on 04/01/09 17:04:22.
Updated patch to port-r10335 and simplified tests
5691.patch (6.8 kB) - added by yml on 02/20/10 15:52:50.
update the patch against the latest trunk r12453
5691.2.patch (7.3 kB) - added by aaloy on 02/23/10 08:54:24.
Patch documentation

Change History

10/05/07 17:36:17 changed by aaloy <aaloy@apsl.net>

  • attachment django_utils_cache_patch.diff added.

a patch with the proposed cache key

10/22/07 15:58:53 changed by jdunck

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

aaloy: I don't think all keys should be affected by i18n. Perhaps there should be an alternate to the low-level cache that does add the active lang as a prefix?

03/02/08 09:55:03 changed by jacob

  • stage changed from Unreviewed to Accepted.

This should probably only be done when USE_I18N == True, but otherwise this is a good idea.

07/18/08 14:03:51 changed by andrewbadr

  • owner changed from nobody to andrewbadr.

07/18/08 14:04:11 changed by andrewbadr

  • needs_better_patch set to 1.
  • needs_tests set to 1.
  • needs_docs set to 1.

07/18/08 16:34:35 changed by andrewbadr

  • attachment i18n_cache_keys_r7960_t5691.diff added.

07/18/08 16:41:04 changed by andrewbadr

  • owner changed from andrewbadr to nobody.

This patch works against trunk and checks the USE_I18N setting. However, this patch is current blocked by #730. Incoming requests need to use the Session middleware, then Locale, then Cache, so that the language is determined by the time we hit the cache middleware. However, outgoing responses need Session before Cache as well so that the cookie-based Vary headers can be set.

08/10/08 20:15:38 changed by andrewbadr

Note that this is viable again after [8260]. Patch probably still works, but I didn't like the duplication of code, and it still needs tests and docs of course.

11/20/08 05:30:44 changed by aaloy

I'm working on a patch which could work as:

    cache_key = 'views.decorators.cache.cache_header.%s.%s' % (
                      key_prefix, iri_to_uri(request.path))
    if settings.USE_I18N:
       cache_key += ".%s" % translation.get_language()

I have tested using other ways of string concatenations and this one is the fast way (0.6 s for 106 interactions) for the kind of keys we're generating.

11/21/08 03:30:07 changed by aaloy

  • owner changed from nobody to aaloy.
  • status changed from new to assigned.

11/21/08 06:17:29 changed by aaloy

  • attachment cache.patch added.

cache pacth to use lang in the cache key

11/25/08 09:19:12 changed by aaloy

  • attachment patch_cache_i18n.patch added.

patch and tests for cache i18n key generation

11/25/08 09:28:02 changed by aaloy

  • needs_docs deleted.
  • needs_tests deleted.

Added the patch an the tests to check that we generate the right keys and that middleware recovers different content depending on the language.

12/01/08 17:44:09 changed by aaloy

  • needs_better_patch deleted.

03/18/09 04:43:16 changed by ramiro

  • cc set to ramiro.

03/19/09 06:44:26 changed by jdunck

  • keywords changed from cache i18n to cache i18n jgd-blackboard.

04/01/09 15:05:47 changed by mrts

/me wonders how the heck did my totally unrelated comments end up in this ticket!? My guess: nazi transgender aliens did it (it's still April 1 in my timezone, so don't you dare to disagree!).

04/01/09 17:04:22 changed by ramiro

  • attachment 5691-use-lan-for-cache-key-r10338.diff added.

Updated patch to port-r10335 and simplified tests

04/01/09 17:09:01 changed by ramiro

I've updated Antoni Aloy's patch to apply cleanly after r10335 and simplified tests to only include what's needed to run them as part of the Django test suite, also fixed them so they don't break other tests leaving test environment umodified.

09/23/09 07:27:10 changed by floledermann

Any progress on checking this in? This is looking good, isn't it?

02/20/10 03:57:41 changed by jezdez

  • milestone set to 1.2.

02/20/10 15:52:50 changed by yml

  • attachment 5691.patch added.

update the patch against the latest trunk r12453

02/22/10 17:48:04 changed by jezdez

  • needs_docs set to 1.

I'm not sure if we break a promise changing the cache key in 1.2 or if it's ok since _generate_cache_header_key is undocumented API anyway.

Anyway, a change needs to be documented.

02/23/10 07:35:45 changed by aaloy

  • status changed from assigned to closed.
  • resolution set to fixed.

jezdez in my oppinion this is not a new feature, is just a bug correction, as one may expect that on a multilingual content page the cache should depend also on the language. Anyway I would be glad to document it.

02/23/10 08:11:45 changed by jezdez

  • status changed from closed to reopened.
  • resolution deleted.

Please don't close the ticket if there wasn't an actual commit.

02/23/10 08:30:05 changed by aaloy

Opps. Sorry it was not my intention :(

02/23/10 08:54:24 changed by aaloy

  • attachment 5691.2.patch added.

Patch documentation

02/23/10 08:56:14 changed by aaloy

  • needs_docs deleted.

Added an updated patch with documentation warning about the new cache key generation. As is an internal feature I'm not really sure about if it has to be documented or not.

02/23/10 14:45:29 changed by jezdez

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [12546]) Fixed #5691 - Adds the active language to the cache key. Thanks, Antoni Aloy, Ramiro Morales and Yann Malet.


Add/Change #5691 (Add the active language to generate the cache key)




Change Properties
Action