Bug 1318960 - Migrate files only when their messages change (#26)

Author: stasm

fluent/migrate/context.py

@@ -2,10 +2,14 @@
 from __future__ import unicode_literals
 
 import os
-import sys
 import codecs
 import logging
 
+try:
+    from itertools import zip_longest
+except ImportError:
+    from itertools import izip_longest as zip_longest
+
 import fluent.syntax.ast as FTL
 from fluent.syntax.parser import FluentParser
 from fluent.syntax.serializer import FluentSerializer
@@ -202,6 +206,29 @@ def get_source(self, path, key):
         resource = self.localization_resources[path]
         return resource.get(key, None)
 
+    def messages_equal(self, res1, res2):
+        """Compare messages of two FTL resources.
+
+        Uses FTL.BaseNode.equals to compare all messages in two FTL resources.
+        If the order or number of messages differ, the result is also False.
+        """
+        def message_id(message):
+            "Return the message's identifer name for sorting purposes."
+            return message.id.name
+
+        messages1 = sorted(
+            (entry for entry in res1.body if isinstance(entry, FTL.Message)),
+            key=message_id)
+        messages2 = sorted(
+            (entry for entry in res2.body if isinstance(entry, FTL.Message)),
+            key=message_id)
+        for msg1, msg2 in zip_longest(messages1, messages2):
+            if msg1 is None or msg2 is None:
+                return False
+            if not msg1.equals(msg2):
+                return False
+        return True
+
     def merge_changeset(self, changeset=None):
         """Return a generator of FTL ASTs for the changeset.
 
@@ -256,16 +283,14 @@ def in_changeset(ident):
                 self, reference, current, transforms, in_changeset
             )
 
-            # Skip this path if the merged snapshot is identical to the current
-            # state of the localization file. This may happen when:
+            # Skip this path if the messages in the merged snapshot are
+            # identical to those in the current state of the localization file.
+            # This may happen when:
             #
             #   - none of the transforms is in the changset, or
             #   - all messages which would be migrated by the context's
             #     transforms already exist in the current state.
-            #
-            # We compare JSON trees rather then use filtering by `in_changeset`
-            # to account for translations removed from `reference`.
-            if snapshot.to_json() == current.to_json():
+            if self.messages_equal(current, snapshot):
                 continue
 
             # Store the merged snapshot on the context so that the next merge

tests/migrate/fixtures/en-US/existing.ftl

@@ -0,0 +1,2 @@
+foo = foo
+bar = Bar

tests/migrate/fixtures/pl/existing.dtd

@@ -0,0 +1,2 @@
+<!ENTITY foo "FOO">
+<!ENTITY bar "BAR">

tests/migrate/fixtures/pl/existing.ftl

@@ -0,0 +1,2 @@
+bar = BAR
+foo = FOO

tests/migrate/test_context.py

@@ -292,25 +292,16 @@ def tearDown(self):
         logging.disable(logging.NOTSET)
 
     def test_missing_localization_file(self):
-        expected = {
-            'toolbar.ftl': ftl_resource_to_json('''
-        // This Source Code Form is subject to the terms of the Mozilla Public
-        // License, v. 2.0. If a copy of the MPL was not distributed with this
-        // file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-
-        [[ Toolbar items ]]
-            ''')
-        }
-
         self.maxDiff = None
         self.assertDictEqual(
             to_json(self.ctx.merge_changeset()),
-            expected
+            {}
         )
 
 
 class TestExistingTarget(unittest.TestCase):
+    maxDiff = None
+
     def setUp(self):
         # Silence all logging.
         logging.disable(logging.CRITICAL)
@@ -415,6 +406,33 @@ def test_existing_target_ftl_with_all_messages(self):
             {}
         )
 
+    def test_existing_target_ftl_with_all_messages_reordered(self):
+        self.ctx.add_transforms('existing.ftl', 'existing.ftl', [
+            FTL.Message(
+                id=FTL.Identifier('foo'),
+                value=COPY(
+                    'existing.dtd',
+                    'foo'
+                )
+            ),
+            FTL.Message(
+                id=FTL.Identifier('bar'),
+                value=COPY(
+                    'existing.dtd',
+                    'bar'
+                )
+            ),
+        ])
+
+        # All migrated messages are already in the target FTL but in a
+        # different order. The order of messages is explicitly ignored in the
+        # snapshot equality check. Consequently, the result of merge_changeset
+        # is an empty iterator.
+        self.assertDictEqual(
+            to_json(self.ctx.merge_changeset()),
+            {}
+        )
+
 
 class TestNotSupportedError(unittest.TestCase):
     def test_add_ftl(self):

tools/migrate/README.md

@@ -20,6 +20,13 @@ Then run migrations passing the `examples` directory as the reference:
 Here's what the output should look like:
 
     Annotating /home/stas/moz/l10n-central/it
+    Running migration examples.bug_1291693
+      Writing to /home/stas/moz/l10n-central/it/browser/branding/official/brand.ftl
+        Committing changeset: Bug 1291693 - Migrate the menubar to FTL, part 1
+      Writing to /home/stas/moz/l10n-central/it/browser/menubar.ftl
+      Writing to /home/stas/moz/l10n-central/it/browser/toolbar.ftl
+      Writing to /home/stas/moz/l10n-central/it/browser/branding/official/brand.ftl
+        Committing changeset: Bug 1291693 - Migrate the menubar to FTL, part 2
     Running migration examples.about_dialog
       Writing to /home/stas/moz/l10n-central/it/browser/about_dialog.ftl
         Committing changeset: Migrate about:dialog, part 1
@@ -30,13 +37,3 @@ Here's what the output should look like:
         Committing changeset: Migrate about:download in Firefox for Android, part 2
       Writing to /home/stas/moz/l10n-central/it/mobile/about_downloads.ftl
         Committing changeset: Migrate about:download in Firefox for Android, part 3
-    Running migration examples.bug_1291693
-      Writing to /home/stas/moz/l10n-central/it/browser/menubar.ftl
-      Writing to /home/stas/moz/l10n-central/it/browser/toolbar.ftl
-        Committing changeset: Bug 1291693 - Migrate the menubar to FTL, part 1
-      Writing to /home/stas/moz/l10n-central/it/browser/branding/official/brand.ftl
-        Committing changeset: Bug 1291693 - Migrate the menubar to FTL, part 2
-      Writing to /home/stas/moz/l10n-central/it/browser/menubar.ftl
-      Writing to /home/stas/moz/l10n-central/it/browser/toolbar.ftl
-      Writing to /home/stas/moz/l10n-central/it/browser/branding/official/brand.ftl
-        Committing changeset: Bug 1291693 - Migrate the menubar to FTL, part 3