Invalid order of applying formats

[Artem Skoretskiy]

I found that DATE_INPUT_FORMATS used for JS I18N is not format I defined in app/formats/*/formats.py file (FORMAT_MODULE_PATH = 'app.formats')

Django version is 1.2.4

I found that get_formats uses invalid order of formats - so format I defined is being overwritten by Django's default ones.

That is serious bug as Django JS uses different formatting than server-side code.

To prove that I changed file django/views/i18n.py to debug the issue:

def get_formats():
    """
    Returns all formats strings required for i18n to work
    """
    FORMAT_SETTINGS = (
        'DATE_FORMAT', 'DATETIME_FORMAT', 'TIME_FORMAT',
        'YEAR_MONTH_FORMAT', 'MONTH_DAY_FORMAT', 'SHORT_DATE_FORMAT',
        'SHORT_DATETIME_FORMAT', 'FIRST_DAY_OF_WEEK', 'DECIMAL_SEPARATOR',
        'THOUSAND_SEPARATOR', 'NUMBER_GROUPING',
        'DATE_INPUT_FORMATS', 'TIME_INPUT_FORMATS', 'DATETIME_INPUT_FORMATS'
    )
    result = {}
    modules = [settings] + get_format_modules(reverse=True) # DEBUG
    log = '%r:\n' % modules # DEBUG
    for module in modules:
        for attr in FORMAT_SETTINGS:
            try:
                if attr == 'DATE_INPUT_FORMATS': # DEBUG
                    log += "    %r from %r\n" % (getattr(module, attr, None), module) #DEBUG
                result[attr] = getattr(module, attr)
            except AttributeError:
                pass
    log += "=> %r from %r" % (result.get('DATE_INPUT_FORMATS'), module) # DEBUG
    if result.get('DATE_INPUT_FORMATS') != ('%Y-%m-%d', '%m/%d/%Y', '%m/%d/%y'): # DEBUG
        logging.getLogger('coupon.localebug').error(log) # DEBUG

    # remaining code is cut

That's the output I got:

[<django.conf.LazySettings object at 0x2359fd0>, <module 'app.formats.fr.formats' from '/XXX/app/formats/fr/formats.pyc'>, <module 'django.conf.locale.fr.formats' from '/XXX/pyenv/lib/python2.5/site-packages/django/conf/locale/fr/formats.pyc'>]:
    ('%Y-%m-%d', '%m/%d/%Y', '%m/%d/%y', '%b %d %Y', '%b %d, %Y', '%d %b %Y', '%d %b, %Y', '%B %d %Y', '%B %d, %Y', '%d %B %Y', '%d %B, %Y') from <django.conf.LazySettings object at 0x2359fd0>
    ('%Y-%m-%d', '%m/%d/%Y', '%m/%d/%y') from <module 'app.formats.fr.formats' from '/XXX/app/formats/fr/formats.pyc'>
    ('%d/%m/%Y', '%d/%m/%y', '%d.%m.%Y', '%d.%m.%y', '%Y-%m-%d', '%y-%m-%d') from <module 'django.conf.locale.fr.formats' from '/XXX/pyenv/lib/python2.5/site-packages/django/conf/locale/fr/formats.pyc'>
=>  ('%d/%m/%Y', '%d/%m/%y', '%d.%m.%Y', '%d.%m.%y', '%Y-%m-%d', '%y-%m-%d') from <module 'django.conf.locale.fr.formats' from '/XXX/pyenv/lib/python2.5/site-packages/django/conf/locale/fr/formats.pyc'>

That means that my format is used, but it is not the last in the order, so it is being overwritten.

[Artem Skoretskiy]

It seems I had found the issue in django/utils/formats.py:

def get_format_modules(reverse=False):
    """
    Returns an iterator over the format modules found
    """
    lang = get_language()
    modules = _format_modules_cache.setdefault(lang, list(iter_format_modules(lang)))
    if reverse:
        modules.reverse()
    return modules

Modules are stored in _format_modules_cache variable and get reversed in place. So if we call the same method multiple times - order will be incorrect.

It seems all we need is just to reverse in memory, not in place:

def get_format_modules(reverse=False):
    """
    Returns an iterator over the format modules found
    """
    lang = get_language()
    modules = _format_modules_cache.setdefault(lang, list(iter_format_modules(lang)))
    if reverse:
        return list(reversed(modules)) # FIX
    return modules

[Artem Skoretskiy]

Yeah, that fixed the problem. Please, include it into Django as soon as possible.

[Artem Skoretskiy]

Patch that does the fix

[Artem Skoretskiy]

What's wrong with current patch? I may improve it if needed.

The only thing I found inconsistent is that new code returns list, not an iterator. However, previous version returned list too and making it return iterator would require to fix django/views/i18n.py file too:

--- a/django/views/i18n.py
+++ b/django/views/i18n.py
@@ -1,4 +1,5 @@
 import os
+import itertools
 import gettext as gettext_module
 
 from django import http
@@ -47,7 +48,7 @@
         'DATE_INPUT_FORMATS', 'TIME_INPUT_FORMATS', 'DATETIME_INPUT_FORMATS'
     )
     result = {}
-    for module in [settings] + get_format_modules(reverse=True):
+    for module in itertools.chain([settings], get_format_modules(reverse=True)):
         for attr in FORMAT_SETTINGS:
             try:
                 result[attr] = getattr(module, attr)

--- a/django/utils/formats.py
+++ b/django/utils/formats.py
@@ -39,7 +39,7 @@
     lang = get_language()
     modules = _format_modules_cache.setdefault(lang, list(iter_format_modules(lang)))
     if reverse:
-        modules.reverse()
+        return reversed(modules)
     return modules
 
 def get_format(format_type):

As of the test - we'll need a project with FORMAT_MODULE_PATH set and formats file existing. Then test is quite simple:

from django.utils import unittest
from django.utils.formats import get_format_modules

class MyTestCase(unittest.TestCase):
    def setUp(self):
        pass

    def testPreservedOrder(self):
        old = "%r" % get_format_modules(reverse=True) # serialize to prevent changing
        new = "%r" % get_format_modules(reverse=True) # second try
        self.assertEqual(new, old, 'get_formats_modules result must be the same during calls')

That check (that result changes during consequent calls) proves the issue on non-patched system.

[Ramiro Morales]

tonnzor's patch wth tests integrated with our test suite. Implementation of fix simply chanegs get_format_modules docstring to say it doesn

[Ramiro Morales]

I'e attached a version of tonnzor's patch with tests integrated with our test suite. Implementation of fix simply changes get_format_modules() docstring to not say it returns an iterator, and so describe the real current code behavior.

[Artem Skoretskiy]

Replying to ramiro:

I'e attached a version of tonnzor's patch with tests integrated with our test suite.

Thanks for patch, ramiro!

Please replace

    return reversed(modules)

with

    return list(reversed(modules))

because:

• reversed function returns iterator, not a list as described in docstring
• as reversed return iterator - that would lead to crash in django/views/i18n.py file on this line:

      for module in [settings] + get_format_modules(reverse=True):
  

[Artem Skoretskiy]

Ramiro patch with fixed iterator -> list

[Artem Skoretskiy]

I suppose that is ready for check-in

[Ramiro Morales]

In [15402]:

Fixed stability of data input/output L10N format modules priority order. Thanks tonnzor for the report and fix.

[Ramiro Morales]

In [15403]:

[1.2.X] Fixed #15129 -- Fixed stability of data input/output L10N format modules priority order. Thanks tonnzor for the report and fix.

Backport of [15402] from trunk

[Jacob]

Milestone 1.3 deleted