From 0281cbbe4d512f568cee88f77716a1c8ddf6a615 Mon Sep 17 00:00:00 2001
From: Jeremy Weinstein <jeremydw@gmail.com>
Date: Tue, 2 Aug 2016 09:17:32 -0700
Subject: [PATCH 1/5] Use remap instead of walk for translation untagging. #142

---
 grow/common/utils.py      | 57 +++++++++++++++++++++++---------------
 grow/common/utils_test.py | 58 +++++++++++++++++++++++++++++++++++++--
 requirements.txt          |  1 +
 3 files changed, 92 insertions(+), 24 deletions(-)

diff --git a/grow/common/utils.py b/grow/common/utils.py
index 31b05deae..dc585b139 100644
--- a/grow/common/utils.py
+++ b/grow/common/utils.py
@@ -6,6 +6,7 @@
         import StringIO
     except ImportError:
         from io import StringIO
+from boltons import iterutils
 import bs4
 import csv as csv_lib
 import functools
@@ -16,11 +17,11 @@
 import logging
 import os
 import re
-import urllib
 import sys
 import threading
 import time
 import translitcodec
+import urllib
 import yaml
 
 # The CLoader implementation of the PyYaml loader is orders of magnitutde
@@ -33,7 +34,9 @@
     from yaml import Loader as yaml_Loader
 
 
+LOCALIZED_KEY_REGEX = re.compile('(.*)@([\w|-]+)$')
 SENTINEL = object()
+SLUG_REGEX = re.compile(r'[\t !"#$%&\'()*\-/<=>?@\[\\\]^_`{|},.]+')
 
 
 class Error(Exception):
@@ -269,12 +272,11 @@ def dump_yaml(obj):
         obj, allow_unicode=True, width=800, default_flow_style=False)
 
 
-_slug_re = re.compile(r'[\t !"#$%&\'()*\-/<=>?@\[\\\]^_`{|},.]+')
 def slugify(text, delim=u'-'):
     if not isinstance(text, basestring):
         text = str(text)
     result = []
-    for word in _slug_re.split(text.lower()):
+    for word in SLUG_REGEX.split(text.lower()):
         word = word.encode('translit/long')
         if word:
             result.append(word)
@@ -296,29 +298,40 @@ def default(self, obj):
 
 
 @memoize
-def untag_fields(fields):
+def untag_fields(fields, locale=None):
     """Untags fields, handling translation priority."""
-    untagged_keys_to_add = {}
-    nodes_and_keys_to_add = []
-    nodes_and_keys_to_remove = []
-    def callback(item, key, node):
+    keys_to_drop = set()
+
+    def visit(path, key, value):
         if not isinstance(key, basestring):
-            return
+            return key, value
         if key.endswith('@#'):
-            nodes_and_keys_to_remove.append((node, key))
+            return False
         if key.endswith('@'):
-            untagged_key = key.rstrip('@')
-            content = item
-            nodes_and_keys_to_remove.append((node, key))
-            untagged_keys_to_add[untagged_key] = True
-            nodes_and_keys_to_add.append((node, untagged_key, content))
-    walk(fields, callback)
-    for node, key in nodes_and_keys_to_remove:
-        if isinstance(node, dict):
-            del node[key]
-    for node, untagged_key, content in nodes_and_keys_to_add:
-        if isinstance(node, dict):
-            node[untagged_key] = content
+            key = key[:-1]
+        match = LOCALIZED_KEY_REGEX.match(key)
+        if not match:
+            if key in keys_to_drop:
+                return False
+            return key, value
+        untagged_key, locale_from_key = match.groups()
+        if locale_from_key != locale:
+            return False
+        return untagged_key, value
+
+    def enter(path, key, value):
+        if not isinstance(key, basestring):
+            return iterutils.default_enter(path, key, value)
+        match = LOCALIZED_KEY_REGEX.match(key)
+        if not match:
+            return iterutils.default_enter(path, key, value)
+        untagged_key, locale_from_key = match.groups()
+        if locale == locale_from_key:
+            keys_to_drop.add(untagged_key)
+        new_parent, items = iterutils.default_enter(path, key, value)
+        return new_parent, items
+
+    fields = iterutils.remap(fields, visit=visit, enter=enter)
     return fields
 
 
diff --git a/grow/common/utils_test.py b/grow/common/utils_test.py
index b960f9bae..36fe1b9a3 100644
--- a/grow/common/utils_test.py
+++ b/grow/common/utils_test.py
@@ -1,9 +1,10 @@
 from grow.testing import testing
 from grow.common.sdk_utils import get_this_version, LatestVersionCheckError
 from . import utils
-import unittest
-import semantic_version
+import copy
 import mock
+import semantic_version
+import unittest
 
 
 class UtilsTestCase(unittest.TestCase):
@@ -46,6 +47,59 @@ def test_version_enforcement(self):
             with self.assertRaises(LatestVersionCheckError):
                 pod = testing.create_test_pod()
 
+    def test_untag_fields(self):
+        fields_to_test = {
+            'title': 'value-none',
+            'title@fr': 'value-fr',
+            'list': [
+                {
+                    'list-item-title': 'value-none',
+                    'list-item-title@fr': 'value-fr',
+                },
+            ],
+            'sub-nested': {
+                'sub-nested': {
+                    'nested@': 'sub-sub-nested-value',
+                },
+            },
+            'nested': {
+                'nested-none': 'nested-value-none',
+                'nested-title@': 'nested-value-none',
+            },
+            'nested@fr': {
+                'nested-title@': 'nested-value-fr',
+            },
+            'list@de': [
+                'list-item-de',
+            ]
+        }
+        fields = copy.deepcopy(fields_to_test)
+        self.assertDictEqual({
+            'title': 'value-fr',
+            'list': [{'list-item-title': 'value-fr'},],
+            'nested': {'nested-title': 'nested-value-fr',},
+            'sub-nested': {
+                'sub-nested': {
+                    'nested': 'sub-sub-nested-value',
+                },
+            },
+        }, utils.untag_fields(fields, locale='fr'))
+
+        fields = copy.deepcopy(fields_to_test)
+        self.assertDictEqual({
+            'title': 'value-none',
+            'list': ['list-item-de',],
+            'nested': {
+                'nested-none': 'nested-value-none',
+                'nested-title': 'nested-value-none',
+            },
+            'sub-nested': {
+                'sub-nested': {
+                    'nested': 'sub-sub-nested-value',
+                },
+            },
+        }, utils.untag_fields(fields, locale='de'))
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/requirements.txt b/requirements.txt
index 555a2cbb8..0c84f47fc 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -9,6 +9,7 @@ Twisted==16.0.0
 WebOb==1.3.1
 Werkzeug==0.9.4
 beautifulsoup4==4.4.0
+boltons==16.4.1
 boto==2.40.0
 certifi==1.0.0
 click==4.0

From c2c0661725ff8344b4de86134100839a28e5051e Mon Sep 17 00:00:00 2001
From: Jeremy Weinstein <jeremydw@gmail.com>
Date: Tue, 2 Aug 2016 11:05:35 -0700
Subject: [PATCH 2/5] Implement localized field untagging, add tests covering
 documents. #142

---
 grow/common/utils.py        | 11 +++++------
 grow/common/utils_test.py   | 24 ++++++++++++++++++++++++
 grow/pods/documents.py      |  2 +-
 grow/pods/documents_test.py | 36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/grow/common/utils.py b/grow/common/utils.py
index dc585b139..9f7258a46 100644
--- a/grow/common/utils.py
+++ b/grow/common/utils.py
@@ -300,7 +300,7 @@ def default(self, obj):
 @memoize
 def untag_fields(fields, locale=None):
     """Untags fields, handling translation priority."""
-    keys_to_drop = set()
+    paths_to_keys_to_update = {}
 
     def visit(path, key, value):
         if not isinstance(key, basestring):
@@ -310,9 +310,9 @@ def visit(path, key, value):
         if key.endswith('@'):
             key = key[:-1]
         match = LOCALIZED_KEY_REGEX.match(key)
+        if paths_to_keys_to_update.get(path) == key:
+            return False
         if not match:
-            if key in keys_to_drop:
-                return False
             return key, value
         untagged_key, locale_from_key = match.groups()
         if locale_from_key != locale:
@@ -327,12 +327,11 @@ def enter(path, key, value):
             return iterutils.default_enter(path, key, value)
         untagged_key, locale_from_key = match.groups()
         if locale == locale_from_key:
-            keys_to_drop.add(untagged_key)
+            paths_to_keys_to_update[path] = untagged_key
         new_parent, items = iterutils.default_enter(path, key, value)
         return new_parent, items
 
-    fields = iterutils.remap(fields, visit=visit, enter=enter)
-    return fields
+    return iterutils.remap(fields, visit=visit, enter=enter)
 
 
 def LocaleIterator(iterator, locale):
diff --git a/grow/common/utils_test.py b/grow/common/utils_test.py
index 36fe1b9a3..0f67c840a 100644
--- a/grow/common/utils_test.py
+++ b/grow/common/utils_test.py
@@ -100,6 +100,30 @@ def test_untag_fields(self):
             },
         }, utils.untag_fields(fields, locale='de'))
 
+        fields_to_test = {
+            'foo': 'bar-base',
+            'foo@de': 'bar-de',
+            'foo@fr': 'bar-fr',
+            'nested': {
+                'nested': 'nested-base',
+                'nested@fr': 'nested-fr',
+            },
+        }
+        fields = copy.deepcopy(fields_to_test)
+        self.assertDictEqual({
+            'foo': 'bar-fr',
+            'nested': {
+                'nested': 'nested-fr',
+            },
+        }, utils.untag_fields(fields, locale='fr'))
+        fields = copy.deepcopy(fields_to_test)
+        self.assertDictEqual({
+            'foo': 'bar-de',
+            'nested': {
+                'nested': 'nested-base',
+            },
+        }, utils.untag_fields(fields, locale='de'))
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/grow/pods/documents.py b/grow/pods/documents.py
index c2ff125e1..31e9a2df3 100644
--- a/grow/pods/documents.py
+++ b/grow/pods/documents.py
@@ -104,7 +104,7 @@ def default_locale(self):
     @utils.cached_property
     def fields(self):
         tagged_fields = self.get_tagged_fields()
-        fields = utils.untag_fields(tagged_fields)
+        fields = utils.untag_fields(tagged_fields, locale=str(self.locale))
         return {} if not fields else fields
 
     def get_tagged_fields(self):
diff --git a/grow/pods/documents_test.py b/grow/pods/documents_test.py
index 73bc78207..0146881c7 100644
--- a/grow/pods/documents_test.py
+++ b/grow/pods/documents_test.py
@@ -253,6 +253,42 @@ def test_multi_file_localization(self):
         keys = ['$title', '$order', '$titles', 'key', 'root_key', '$locale']
         self.assertItemsEqual(keys, fr_doc.fields.keys())
 
+    def test_locale_override(self):
+        pod = testing.create_pod()
+        pod.write_yaml('/podspec.yaml', {
+            'localization': {
+                'default_locale': 'en',
+                'locales': [
+                    'de',
+                    'fr',
+                ]
+            }
+        })
+        pod.write_yaml('/content/pages/_blueprint.yaml', {
+            '$path': '/{base}/',
+            '$view': '/views/base.html',
+            '$localization': {
+                'path': '/{locale}/{base}/',
+            },
+        })
+        pod.write_yaml('/content/pages/a.yaml', {
+            'foo': 'bar-base',
+            'foo@de': 'bar-de',
+            'foo@fr': 'bar-fr',
+            'nested': {
+                'nested': 'nested-base',
+                'nested@fr': 'nested-fr',
+            },
+        })
+        doc = pod.get_doc('/content/pages/a.yaml')
+        self.assertEqual(doc.foo, 'bar-base')
+        de_doc = doc.localize('de')
+        self.assertEqual(de_doc.foo, 'bar-de')
+        self.assertEqual(de_doc.nested['nested'], 'nested-base')
+        fr_doc = doc.localize('fr')
+        self.assertEqual(fr_doc.foo, 'bar-fr')
+        self.assertEqual(fr_doc.nested['nested'], 'nested-fr')
+
 
 if __name__ == '__main__':
     unittest.main()

From f1840579ecfc6d498dd7ed0efa483db001b6e043 Mon Sep 17 00:00:00 2001
From: Jeremy Weinstein <jeremydw@gmail.com>
Date: Tue, 2 Aug 2016 11:11:22 -0700
Subject: [PATCH 3/5] Add additional test coverage for lists. #142

---
 grow/common/utils_test.py | 53 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/grow/common/utils_test.py b/grow/common/utils_test.py
index 0f67c840a..cd3162a3a 100644
--- a/grow/common/utils_test.py
+++ b/grow/common/utils_test.py
@@ -124,6 +124,59 @@ def test_untag_fields(self):
             },
         }, utils.untag_fields(fields, locale='de'))
 
+        fields_to_test = {
+            'list': [
+                {
+                    'item': 'value-1',
+                    'item@de': 'value-1-de',
+                    'item@fr': 'value-1-fr',
+                },
+                {
+                    'item': 'value-2',
+                },
+                {
+                    'item@fr': 'value-3-fr',
+                },
+            ]
+        }
+        fields = copy.deepcopy(fields_to_test)
+        self.assertDictEqual({
+            'list': [
+                {
+                    'item': 'value-1-fr',
+                },
+                {
+                    'item': 'value-2',
+                },
+                {
+                    'item': 'value-3-fr',
+                },
+            ]
+        }, utils.untag_fields(fields, locale='fr'))
+        fields = copy.deepcopy(fields_to_test)
+        self.assertDictEqual({
+            'list': [
+                {
+                    'item': 'value-1-de',
+                },
+                {
+                    'item': 'value-2',
+                },
+                {},
+            ]
+        }, utils.untag_fields(fields, locale='de'))
+        self.assertDictEqual({
+            'list': [
+                {
+                    'item': 'value-1',
+                },
+                {
+                    'item': 'value-2',
+                },
+                {},
+            ]
+        }, utils.untag_fields(fields, locale='ja'))
+
 
 if __name__ == '__main__':
     unittest.main()

From 4998fdef8a3ffb48e5dc9eda4e2bfe13be735bc9 Mon Sep 17 00:00:00 2001
From: Jeremy Weinstein <jeremydw@gmail.com>
Date: Tue, 2 Aug 2016 21:58:30 -0700
Subject: [PATCH 4/5] Implement fix for string tagging, handling unordered
 dictionary. #142

---
 grow/common/utils.py        | 22 ++++++----------------
 grow/common/utils_test.py   | 35 +++++++++++++++++++++++++++++++++++
 grow/pods/collection.py     | 12 +++++++++++-
 grow/pods/documents.py      | 19 ++++++++-----------
 grow/pods/documents_test.py | 27 ++++++++++++++++++++++-----
 5 files changed, 82 insertions(+), 33 deletions(-)

diff --git a/grow/common/utils.py b/grow/common/utils.py
index 9f7258a46..7153ff1fb 100644
--- a/grow/common/utils.py
+++ b/grow/common/utils.py
@@ -300,9 +300,12 @@ def default(self, obj):
 @memoize
 def untag_fields(fields, locale=None):
     """Untags fields, handling translation priority."""
-    paths_to_keys_to_update = {}
+
+    updated_localized_paths = set()
 
     def visit(path, key, value):
+        if (path, key) in updated_localized_paths:
+            return False
         if not isinstance(key, basestring):
             return key, value
         if key.endswith('@#'):
@@ -310,28 +313,15 @@ def visit(path, key, value):
         if key.endswith('@'):
             key = key[:-1]
         match = LOCALIZED_KEY_REGEX.match(key)
-        if paths_to_keys_to_update.get(path) == key:
-            return False
         if not match:
             return key, value
         untagged_key, locale_from_key = match.groups()
         if locale_from_key != locale:
             return False
+        updated_localized_paths.add((path, untagged_key))
         return untagged_key, value
 
-    def enter(path, key, value):
-        if not isinstance(key, basestring):
-            return iterutils.default_enter(path, key, value)
-        match = LOCALIZED_KEY_REGEX.match(key)
-        if not match:
-            return iterutils.default_enter(path, key, value)
-        untagged_key, locale_from_key = match.groups()
-        if locale == locale_from_key:
-            paths_to_keys_to_update[path] = untagged_key
-        new_parent, items = iterutils.default_enter(path, key, value)
-        return new_parent, items
-
-    return iterutils.remap(fields, visit=visit, enter=enter)
+    return iterutils.remap(fields, visit=visit)
 
 
 def LocaleIterator(iterator, locale):
diff --git a/grow/common/utils_test.py b/grow/common/utils_test.py
index cd3162a3a..beaae1129 100644
--- a/grow/common/utils_test.py
+++ b/grow/common/utils_test.py
@@ -177,6 +177,41 @@ def test_untag_fields(self):
             ]
         }, utils.untag_fields(fields, locale='ja'))
 
+        fields_to_test = {
+            '$view': '/views/base.html',
+            '$view@ja': '/views/base-ja.html',
+            'qaz': 'qux',
+            'qaz@ja': 'qux-ja',
+            'qaz@de': 'qux-de',
+            'qaz@ja': 'qux-ja',
+            'foo': 'bar-base',
+            'foo@en': 'bar-en',
+            'foo@de': 'bar-de',
+            'foo@ja': 'bar-ja',
+            'nested': {
+                'nested': 'nested-base',
+                'nested@ja': 'nested-ja',
+            },
+        }
+        fields = copy.deepcopy(fields_to_test)
+        self.assertDictEqual({
+            '$view': '/views/base-ja.html',
+            'qaz': 'qux-ja',
+            'foo': 'bar-ja',
+            'nested': {
+                'nested': 'nested-ja',
+            },
+        }, utils.untag_fields(fields, locale='ja'))
+        fields = copy.deepcopy(fields_to_test)
+        self.assertDictEqual({
+            '$view': '/views/base.html',
+            'qaz': 'qux-de',
+            'foo': 'bar-de',
+            'nested': {
+                'nested': 'nested-base',
+            },
+        }, utils.untag_fields(fields, locale='de'))
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/grow/pods/collection.py b/grow/pods/collection.py
index c5b36167c..b3ca61eb4 100644
--- a/grow/pods/collection.py
+++ b/grow/pods/collection.py
@@ -55,7 +55,6 @@ def __init__(self, pod_path, _pod):
         self.collection_path = regex.sub('', pod_path).strip('/')
         self.pod_path = pod_path
         self.basename = os.path.basename(self.collection_path)
-        self._default_locale = _pod.podspec.default_locale
         self._blueprint_path = os.path.join(
             self.pod_path, Collection.BLUEPRINT_PATH)
 
@@ -85,6 +84,17 @@ def fields(self):
     def tagged_fields(self):
         return copy.deepcopy(self.yaml)
 
+    @utils.cached_property
+    def default_locale(self):
+        if self.localization and 'default_locale' in self.localization:
+            locale = self.localization['default_locale']
+        else:
+            locale = self.pod.podspec.default_locale
+        locale = locales.Locale.parse(locale)
+        if locale:
+            locale.set_alias(self.pod)
+        return locale
+
     @classmethod
     def list(cls, pod):
         items = []
diff --git a/grow/pods/documents.py b/grow/pods/documents.py
index 31e9a2df3..e85a2e66d 100644
--- a/grow/pods/documents.py
+++ b/grow/pods/documents.py
@@ -90,21 +90,18 @@ def _clean_basename(cls, pod_path):
     def default_locale(self):
         if ('$localization' in self.fields
             and 'default_locale' in self.fields['$localization']):
-            locale = self.fields['$localization']['default_locale']
-        elif (self.collection.localization
-              and 'default_locale' in self.collection.localization):
-            locale = self.collection.localization['default_locale']
-        else:
-            locale = self.pod.podspec.default_locale
-        locale = locales.Locale.parse(locale)
-        if locale:
-            locale.set_alias(self.pod)
-        return locale
+            identifier = self.fields['$localization']['default_locale']
+            locale = locales.Locale.parse(identifier)
+            if locale:
+                locale.set_alias(self.pod)
+            return locale
+        return self.collection.default_locale
 
     @utils.cached_property
     def fields(self):
+        identifier = self.locale or self.collection.default_locale
         tagged_fields = self.get_tagged_fields()
-        fields = utils.untag_fields(tagged_fields, locale=str(self.locale))
+        fields = utils.untag_fields(tagged_fields, locale=str(identifier))
         return {} if not fields else fields
 
     def get_tagged_fields(self):
diff --git a/grow/pods/documents_test.py b/grow/pods/documents_test.py
index 0146881c7..db2291dcd 100644
--- a/grow/pods/documents_test.py
+++ b/grow/pods/documents_test.py
@@ -261,6 +261,7 @@ def test_locale_override(self):
                 'locales': [
                     'de',
                     'fr',
+                    'it',
                 ]
             }
         })
@@ -272,7 +273,14 @@ def test_locale_override(self):
             },
         })
         pod.write_yaml('/content/pages/a.yaml', {
+            '$view': '/views/base.html',
+            '$view@fr': '/views/base-fr.html',
+            'qaz': 'qux',
+            'qaz@fr': 'qux-fr',
+            'qaz@de': 'qux-de',
+            'qaz@fr': 'qux-fr',
             'foo': 'bar-base',
+            'foo@en': 'bar-en',
             'foo@de': 'bar-de',
             'foo@fr': 'bar-fr',
             'nested': {
@@ -281,13 +289,22 @@ def test_locale_override(self):
             },
         })
         doc = pod.get_doc('/content/pages/a.yaml')
-        self.assertEqual(doc.foo, 'bar-base')
+        self.assertEqual('en', doc.locale)
+        self.assertEqual('bar-en', doc.foo)
+        self.assertEqual('qux', doc.qaz)
         de_doc = doc.localize('de')
-        self.assertEqual(de_doc.foo, 'bar-de')
-        self.assertEqual(de_doc.nested['nested'], 'nested-base')
+        self.assertEqual('bar-de', de_doc.foo)
+        self.assertEqual('/views/base.html', de_doc.view)
+        self.assertEqual('nested-base', de_doc.nested['nested'])
+        self.assertEqual('qux-de', de_doc.qaz)
         fr_doc = doc.localize('fr')
-        self.assertEqual(fr_doc.foo, 'bar-fr')
-        self.assertEqual(fr_doc.nested['nested'], 'nested-fr')
+        self.assertEqual('bar-fr', fr_doc.foo)
+        self.assertEqual('/views/base-fr.html', fr_doc.view)
+        self.assertEqual('nested-fr', fr_doc.nested['nested'])
+        self.assertEqual('qux-fr', fr_doc.qaz)
+        it_doc = doc.localize('it')
+        self.assertEqual('bar-base', it_doc.foo)
+        self.assertEqual('qux', it_doc.qaz)
 
 
 if __name__ == '__main__':

From 9e17fdde24a17834fe53e2a971c995b674aea49a Mon Sep 17 00:00:00 2001
From: Jeremy Weinstein <jeremydw@gmail.com>
Date: Tue, 2 Aug 2016 22:06:37 -0700
Subject: [PATCH 5/5] Add test verifying behavior for tagged, localized
 strings. #142

---
 grow/common/utils_test.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/grow/common/utils_test.py b/grow/common/utils_test.py
index beaae1129..fb0deb4f7 100644
--- a/grow/common/utils_test.py
+++ b/grow/common/utils_test.py
@@ -212,6 +212,21 @@ def test_untag_fields(self):
             },
         }, utils.untag_fields(fields, locale='de'))
 
+        fields_to_test = {
+            'foo@': 'bar',
+            'foo@fr@': 'bar-fr',
+        }
+        fields = copy.deepcopy(fields_to_test)
+        self.assertDictEqual({
+            'foo': 'bar',
+        }, utils.untag_fields(fields))
+        self.assertDictEqual({
+            'foo': 'bar',
+        }, utils.untag_fields(fields, locale='de'))
+        self.assertDictEqual({
+            'foo': 'bar-fr',
+        }, utils.untag_fields(fields, locale='fr'))
+
 
 if __name__ == '__main__':
     unittest.main()