Skip to content

Commit

Permalink
Fix bug 1633149: Detect changes to translated resources on sync (#1753)
Browse files Browse the repository at this point in the history
When detecting changes to translated resources in VCS, we need to
identify which of the changed paths belongs to a localizable resource.

The logic that does that makes assumptions about the file and folder
structure which are not valid for projects using project configuration.

Hence this patch adds a new logic for such projects, which makes use of
the ProjectFiles matcher of the compare-locales library.

The logic also converts the changed paths to the canonical form for
representing resources used across Pontoon codebase (reference path
insted of l10n path) and assigns the list of changed locales to each
path.

This patch also fixes bug 1608045, which was another symptom of the
very same problem.
  • Loading branch information
mathjazz committed Nov 30, 2020
1 parent 67684fd commit f99732a
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 16 deletions.
66 changes: 65 additions & 1 deletion pontoon/sync/tests/test_vcs_models.py
Expand Up @@ -14,6 +14,7 @@
)
from pontoon.base.tests import (
CONTAINS,
LocaleFactory,
ProjectFactory,
RepositoryFactory,
ResourceFactory,
Expand Down Expand Up @@ -68,12 +69,61 @@ def setUp(self):
self.mock_checkout_path = checkout_path_patch.start()
self.addCleanup(checkout_path_patch.stop)

self.locale = LocaleFactory.create(code="XY")
self.project = ProjectFactory.create(
repositories__permalink="https://example.com/l10n/{locale_code}"
locales=[self.locale],
repositories__permalink="https://example.com/l10n/{locale_code}",
)
self.vcs_project = VCSProject(self.project)
super(VCSProjectTests, self).setUp()

@patch.object(VCSProject, "source_directory_path", new_callable=PropertyMock)
def test_get_relevant_files_with_config(self, source_directory_path_mock):
"""
Return relative reference paths and locales of paths found in project configuration.
"""
source_directory_path_mock.return_value = ""
paths = ["locale/path/to/localizable_file.ftl"]
self.vcs_project.configuration = VCSConfiguration(self.vcs_project)

# Return empty dict if no reference path found for any of the paths
with patch(
"pontoon.sync.vcs.models.VCSConfiguration.reference_path",
return_value=None,
):
files = self.vcs_project.get_relevant_files_with_config(paths)
assert files == {}

# Return empty dict if no reference path found for any of the paths
with patch(
"pontoon.sync.vcs.models.VCSConfiguration.reference_path",
return_value="reference/path/to/localizable_file.ftl",
):
files = self.vcs_project.get_relevant_files_with_config(paths)
assert files == {"reference/path/to/localizable_file.ftl": [self.locale]}

def test_get_relevant_files_without_config(self):
"""
Return relative paths and their locales if they start with locale repository paths.
"""
paths = [
"locales/xy/path/to/localizable_file.ftl",
"some.random.file",
".hidden_file",
]

locale_path_locales = {
"locales/ab": "AB",
"locales/cd": "CD",
"locales/xy": "XY",
}

files = self.vcs_project.get_relevant_files_without_config(
paths, locale_path_locales
)

assert files == {"path/to/localizable_file.ftl": ["XY"]}

def test_missing_permalink_prefix(self):
"""
Fail when the source repository of a project with the project
Expand Down Expand Up @@ -376,6 +426,20 @@ def test_l10n_path(self):
== l10n_path
)

def test_reference_path(self):
absolute_l10n_path = os.path.join(
PROJECT_CONFIG_CHECKOUT_PATH, "values-fr/amo.po",
)

reference_path = os.path.join(PROJECT_CONFIG_CHECKOUT_PATH, "values/amo.pot",)

assert (
self.vcs_project.configuration.reference_path(
self.locale, absolute_l10n_path,
)
== reference_path
)

def test_locale_resources(self):
assert sorted(
self.vcs_project.configuration.locale_resources(self.locale),
Expand Down
76 changes: 61 additions & 15 deletions pontoon/sync/vcs/models.py
Expand Up @@ -265,21 +265,13 @@ def changed_locales_files(self):
)
)

# Find relevant changes in repository by matching changed
# paths against locale repository paths
locale_path_locales = self.locale_path_locales(repo.checkout_path)
locale_paths = locale_path_locales.keys()

for path in changed_files:
if is_hidden(path):
continue

for locale_path in locale_paths:
if path.startswith(locale_path):
locale = locale_path_locales[locale_path]
path = path[len(locale_path) :].lstrip(os.sep)
files.setdefault(path, []).append(locale)
break
# Include only relevant (localizable) files
if self.configuration:
files = self.get_relevant_files_with_config(changed_files)
else:
files = self.get_relevant_files_without_config(
changed_files, self.locale_path_locales(repo.checkout_path)
)

log.info(
"Changed files in {} repository, relevant for enabled locales: {}".format(
Expand Down Expand Up @@ -311,6 +303,51 @@ def changed_config_files(self):
changed_files = set(self.changed_source_files[0])
return changed_files.intersection(config_files)

def get_relevant_files_with_config(self, paths):
"""
Check if given paths represent localizable files using project configuration.
Return a dict of relative reference paths of such paths and corresponding Locale
objects.
"""
files = {}

for locale in self.db_project.locales.all():
for path in paths:
absolute_path = os.path.join(self.source_directory_path, path)
reference_path = self.configuration.reference_path(
locale, absolute_path
)

if reference_path:
relative_reference_path = reference_path[
len(self.source_directory_path) :
].lstrip(os.sep)
files.setdefault(relative_reference_path, []).append(locale)

return files

def get_relevant_files_without_config(self, paths, locale_path_locales):
"""
Check if given paths represent localizable files by matching them against locale
repository paths. Return a dict of relative reference paths of such paths and
corresponding Locale objects.
"""
files = {}
locale_paths = locale_path_locales.keys()

for path in paths:
if is_hidden(path):
continue

for locale_path in locale_paths:
if path.startswith(locale_path):
locale = locale_path_locales[locale_path]
path = path[len(locale_path) :].lstrip(os.sep)
files.setdefault(path, []).append(locale)
break

return files

def locale_path_locales(self, repo_checkout_path):
"""
A map of relative locale directory paths and their respective locales.
Expand Down Expand Up @@ -665,6 +702,15 @@ def l10n_path(self, locale, reference_path):
m = project_files.match(reference_path)
return m[0] if m is not None else None

def reference_path(self, locale, l10n_path):
"""
Return reference path for the given locale and l10n path.
"""
project_files = self.get_or_set_project_files(locale.code)

m = project_files.match(l10n_path)
return m[1] if m is not None else None

def locale_resources(self, locale):
"""
Return a list of Resource instances, which need to be enabled for the
Expand Down

0 comments on commit f99732a

Please sign in to comment.