launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02190
[Merge] lp:~henninge/launchpad/devel-bug-338234-leading-slashes into lp:launchpad
Henning Eggers has proposed merging lp:~henninge/launchpad/devel-bug-338234-leading-slashes into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#338234 Strip leading slashes in tarball paths
https://bugs.launchpad.net/bugs/338234
= Summary =
This is a simple fix to add "lstrip('/')" in the right place to remove
leading slashes from file paths. The branch also contains a new test case
for the TranslationImportQueue class because it has only being tested in doc
tests so far.
In preparation I moved the useful LaunchpadWriteTarFile helper class in a
module under lp.services because it is not translation specific
== Proposed fix ==
The method addOrUpdateEntriesFromTarball calls _makePath to modify and
filter the path. This is the right place to strip any leading slashes.
== Pre-implementation notes ==
I chatted with Danilo about the location and name for tarfile_helpers.py.
== Tests ==
bin/test -vv \
-t translationimportqueue \
-t tarfile_helpers
== Demo and Q/A ==
For QA upload a tarfile which contains filenames with leading slashes and
verify that the queue entries do not have the leadind slashes.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/utilities/translation_export.py
lib/lp/translations/model/translationimportqueue.py
lib/lp/translations/scripts/language_pack.py
lib/lp/translations/doc/translationimportqueue.txt
lib/lp/services/tarfile_helpers.py
lib/lp/services/doc/tarfile_helpers.txt
lib/lp/translations/tests/test_translationimportqueue.py
--
https://code.launchpad.net/~henninge/launchpad/devel-bug-338234-leading-slashes/+merge/44054
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/devel-bug-338234-leading-slashes into lp:launchpad.
=== renamed file 'lib/lp/translations/utilities/doc/launchpad_write_tarfile.txt' => 'lib/lp/services/doc/tarfile_helpers.txt'
--- lib/lp/translations/utilities/doc/launchpad_write_tarfile.txt 2010-04-26 16:00:31 +0000
+++ lib/lp/services/doc/tarfile_helpers.txt 2010-12-17 14:19:59 +0000
@@ -1,11 +1,11 @@
-= Launchpad helper to write tar files =
+Launchpad helper to write tar files
+===================================
LaunchpadWriteTarFile is a helper class to make .tar.gz generation
easy.
>>> from StringIO import StringIO
- >>> from lp.translations.utilities.translation_export import (
- ... LaunchpadWriteTarFile)
+ >>> from lp.services.tarfile_helpers import LaunchpadWriteTarFile
First, we are going to define a function we are going to use to validate the
output we will get.
@@ -99,9 +99,9 @@
... 'uno/dos/tres/cuatro': 'blah'
... })
>>> examine_tarfile(archive)
- uno |
- uno/dos |
- uno/dos/tres |
+ uno |
+ uno/dos |
+ uno/dos/tres |
uno/dos/tres/cuatro | blah
Also, if there is a duplicated file, last one is the one that remains there.
=== added file 'lib/lp/services/tarfile_helpers.py'
--- lib/lp/services/tarfile_helpers.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/tarfile_helpers.py 2010-12-17 14:19:59 +0000
@@ -0,0 +1,108 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Helpers to work with tar files more easily."""
+
+__metaclass__ = type
+
+__all__ = [
+ 'LaunchpadWriteTarFile',
+ ]
+
+import os
+from StringIO import StringIO
+import tarfile
+import tempfile
+import time
+
+# A note about tarballs, StringIO and unicode. SQLObject returns unicode
+# values for columns which are declared as StringCol. We have to be careful
+# not to pass unicode instances to the tarfile module, because when the
+# tarfile's filehandle is a StringIO object, the StringIO object gets upset
+# later when we ask it for its value and it tries to join together its
+# buffers. This is why the tarball code is sprinkled with ".encode('ascii')".
+# If we get separate StringCol and UnicodeCol column types, we won't need this
+# any longer.
+# -- Dafydd Harries, 2005-04-07.
+
+class LaunchpadWriteTarFile:
+ """Convenience wrapper around the tarfile module.
+
+ This class makes it convenient to generate tar files in various ways.
+ """
+
+ def __init__(self, stream):
+ self.tarfile = tarfile.open('', 'w:gz', stream)
+ self.closed = False
+
+ @classmethod
+ def files_to_stream(cls, files):
+ """Turn a dictionary of files into a data stream."""
+ buffer = tempfile.TemporaryFile()
+ archive = cls(buffer)
+ archive.add_files(files)
+ archive.close()
+ buffer.seek(0)
+ return buffer
+
+ @classmethod
+ def files_to_string(cls, files):
+ """Turn a dictionary of files into a data string."""
+ return cls.files_to_stream(files).read()
+
+ @classmethod
+ def files_to_tarfile(cls, files):
+ """Turn a dictionary of files into a tarfile object."""
+ return tarfile.open('', 'r', cls.files_to_stream(files))
+
+ def close(self):
+ """Close the archive.
+
+ After the archive is closed, the data written to the filehandle will
+ be complete. The archive may not be appended to after it has been
+ closed.
+ """
+
+ self.tarfile.close()
+ self.closed = True
+
+ def add_file(self, path, contents):
+ """Add a file to the archive."""
+ assert not self.closed, "Can't add a file to a closed archive"
+
+ now = int(time.time())
+ path_bits = path.split(os.path.sep)
+
+ # Ensure that all the directories in the path are present in the
+ # archive.
+ for i in range(1, len(path_bits)):
+ joined_path = os.path.join(*path_bits[:i])
+
+ try:
+ self.tarfile.getmember(joined_path)
+ except KeyError:
+ tarinfo = tarfile.TarInfo(joined_path)
+ tarinfo.type = tarfile.DIRTYPE
+ tarinfo.mtime = now
+ tarinfo.mode = 0755
+ tarinfo.uname = 'launchpad'
+ tarinfo.gname = 'launchpad'
+ self.tarfile.addfile(tarinfo)
+
+ tarinfo = tarfile.TarInfo(path)
+ tarinfo.time = now
+ tarinfo.mtime = now
+ tarinfo.mode = 0644
+ tarinfo.size = len(contents)
+ tarinfo.uname = 'launchpad'
+ tarinfo.gname = 'launchpad'
+ self.tarfile.addfile(tarinfo, StringIO(contents))
+
+ def add_files(self, files):
+ """Add a number of files to the archive.
+
+ :param files: A dictionary mapping file names to file contents.
+ """
+
+ for filename in sorted(files.keys()):
+ self.add_file(filename, files[filename])
=== modified file 'lib/lp/translations/doc/translationimportqueue.txt'
--- lib/lp/translations/doc/translationimportqueue.txt 2010-12-01 11:26:57 +0000
+++ lib/lp/translations/doc/translationimportqueue.txt 2010-12-17 14:19:59 +0000
@@ -1103,8 +1103,7 @@
addOrUpdateEntry adds a new entry to the import queue so we can handle it
later with poimport script.
- >>> from lp.translations.utilities.translation_export import (
- ... LaunchpadWriteTarFile)
+ >>> from lp.services.tarfile_helpers import LaunchpadWriteTarFile
>>> potemplate_set = getUtility(IPOTemplateSet)
>>> potemplate_subset = potemplate_set.getSubset(
... productseries=evolution_productseries)
=== modified file 'lib/lp/translations/model/translationimportqueue.py'
--- lib/lp/translations/model/translationimportqueue.py 2010-12-01 11:26:57 +0000
+++ lib/lp/translations/model/translationimportqueue.py 2010-12-17 14:19:59 +0000
@@ -992,7 +992,7 @@
def _makePath(self, name, path_filter):
"""Make the file path from the name stored in the tarball."""
- path = posixpath.normpath(name)
+ path = posixpath.normpath(name).lstrip('/')
if path_filter:
path = path_filter(path)
return path
=== modified file 'lib/lp/translations/scripts/language_pack.py'
--- lib/lp/translations/scripts/language_pack.py 2010-08-20 20:31:18 +0000
+++ lib/lp/translations/scripts/language_pack.py 2010-12-17 14:19:59 +0000
@@ -39,7 +39,7 @@
TranslationFileFormat,
)
from lp.translations.interfaces.vpoexport import IVPOExportSet
-from lp.translations.utilities.translation_export import LaunchpadWriteTarFile
+from lp.services.tarfile_helpers import LaunchpadWriteTarFile
def iter_sourcepackage_translationdomain_mapping(series):
=== modified file 'lib/lp/translations/tests/test_translationimportqueue.py'
--- lib/lp/translations/tests/test_translationimportqueue.py 2010-11-03 06:14:23 +0000
+++ lib/lp/translations/tests/test_translationimportqueue.py 2010-12-17 14:19:59 +0000
@@ -3,6 +3,8 @@
__metaclass__ = type
+import posixpath
+
import transaction
from zope.component import getUtility
@@ -11,6 +13,7 @@
LaunchpadFunctionalLayer,
LaunchpadZopelessLayer,
)
+from lp.services.tarfile_helpers import LaunchpadWriteTarFile
from lp.services.worlddata.interfaces.language import ILanguageSet
from lp.testing import (
person_logged_in,
@@ -330,3 +333,78 @@
productseries=self.product.series[0])
self.product.owner = self.new_owner
self.assertEqual(self.old_owner, old_entry.importer)
+
+
+class TestTranslationImportQueue(TestCaseWithFactory):
+ """Tests for `TranslationImportQueue`."""
+
+ layer = LaunchpadZopelessLayer
+
+ def setUp(self):
+ super(TestTranslationImportQueue, self).setUp()
+ self.productseries = self.factory.makeProductSeries()
+ self.importer = self.factory.makePerson()
+ self.import_queue = getUtility(ITranslationImportQueue)
+
+ def _makeFile(self, extension=None, directory=None):
+ """Create a file with arbitrary name and content.
+
+ Returns a tuple (name, content).
+ """
+ filename = self.factory.getUniqueString()
+ if extension is not None:
+ filename = "%s.%s" % (filename, extension)
+ if directory is not None:
+ filename = posixpath.join(directory, filename)
+ content = self.factory.getUniqueString()
+ return (filename, content)
+
+ def _getQueuePaths(self):
+ entries = self.import_queue.getAllEntries(target=self.productseries)
+ return [entry.path for entry in entries]
+
+ def test_addOrUpdateEntriesFromTarball_baseline(self):
+ # Files from a tarball are placed in the queue.
+ files = dict((
+ self._makeFile('pot'),
+ self._makeFile('po'),
+ self._makeFile('xpi'),
+ ))
+ tarfile_content = LaunchpadWriteTarFile.files_to_string(files)
+ self.import_queue.addOrUpdateEntriesFromTarball(
+ tarfile_content, True, self.importer,
+ productseries=self.productseries)
+ self.assertContentEqual(files.keys(), self._getQueuePaths())
+
+ def test_addOrUpdateEntriesFromTarball_only_translation_files(self):
+ # Only files with the right extensions are added.
+ files = dict((
+ self._makeFile(),
+ ))
+ tarfile_content = LaunchpadWriteTarFile.files_to_string(files)
+ self.import_queue.addOrUpdateEntriesFromTarball(
+ tarfile_content, True, self.importer,
+ productseries=self.productseries)
+ self.assertEqual([], self._getQueuePaths())
+
+ def test_addOrUpdateEntriesFromTarball_path(self):
+ # File names are store with full path.
+ files = dict((
+ self._makeFile('pot', 'directory'),
+ ))
+ tarfile_content = LaunchpadWriteTarFile.files_to_string(files)
+ self.import_queue.addOrUpdateEntriesFromTarball(
+ tarfile_content, True, self.importer,
+ productseries=self.productseries)
+ self.assertEqual(files.keys(), self._getQueuePaths())
+
+ def test_addOrUpdateEntriesFromTarball_path_leading_slash(self):
+ # Leading slashes are stripped from path names.
+ path, content = self._makeFile('pot', '/directory')
+ files = dict(((path, content),))
+ tarfile_content = LaunchpadWriteTarFile.files_to_string(files)
+ self.import_queue.addOrUpdateEntriesFromTarball(
+ tarfile_content, True, self.importer,
+ productseries=self.productseries)
+ stripped_path = path.lstrip('/')
+ self.assertEqual([stripped_path], self._getQueuePaths())
=== modified file 'lib/lp/translations/utilities/translation_export.py'
--- lib/lp/translations/utilities/translation_export.py 2010-08-20 20:31:18 +0000
+++ lib/lp/translations/utilities/translation_export.py 2010-12-17 14:19:59 +0000
@@ -9,18 +9,15 @@
'ExportedTranslationFile',
'ExportFileStorage',
'TranslationExporter',
- 'LaunchpadWriteTarFile',
]
-import os
from StringIO import StringIO
-import tarfile
import tempfile
-import time
from zope.component import subscribers
from zope.interface import implements
+from lp.services.tarfile_helpers import LaunchpadWriteTarFile
from lp.translations.interfaces.translationexporter import (
IExportedTranslationFile,
ITranslationExporter,
@@ -101,99 +98,6 @@
return storage.export()
-# A note about tarballs, StringIO and unicode. SQLObject returns unicode
-# values for columns which are declared as StringCol. We have to be careful
-# not to pass unicode instances to the tarfile module, because when the
-# tarfile's filehandle is a StringIO object, the StringIO object gets upset
-# later when we ask it for its value and it tries to join together its
-# buffers. This is why the tarball code is sprinkled with ".encode('ascii')".
-# If we get separate StringCol and UnicodeCol column types, we won't need this
-# any longer.
-# -- Dafydd Harries, 2005-04-07.
-
-class LaunchpadWriteTarFile:
- """Convenience wrapper around the tarfile module.
-
- This class makes it convenient to generate tar files in various ways.
- """
-
- def __init__(self, stream):
- self.tarfile = tarfile.open('', 'w:gz', stream)
- self.closed = False
-
- @classmethod
- def files_to_stream(cls, files):
- """Turn a dictionary of files into a data stream."""
- buffer = tempfile.TemporaryFile()
- archive = cls(buffer)
- archive.add_files(files)
- archive.close()
- buffer.seek(0)
- return buffer
-
- @classmethod
- def files_to_string(cls, files):
- """Turn a dictionary of files into a data string."""
- return cls.files_to_stream(files).read()
-
- @classmethod
- def files_to_tarfile(cls, files):
- """Turn a dictionary of files into a tarfile object."""
- return tarfile.open('', 'r', cls.files_to_stream(files))
-
- def close(self):
- """Close the archive.
-
- After the archive is closed, the data written to the filehandle will
- be complete. The archive may not be appended to after it has been
- closed.
- """
-
- self.tarfile.close()
- self.closed = True
-
- def add_file(self, path, contents):
- """Add a file to the archive."""
- assert not self.closed, "Can't add a file to a closed archive"
-
- now = int(time.time())
- path_bits = path.split(os.path.sep)
-
- # Ensure that all the directories in the path are present in the
- # archive.
- for i in range(1, len(path_bits)):
- joined_path = os.path.join(*path_bits[:i])
-
- try:
- self.tarfile.getmember(joined_path)
- except KeyError:
- tarinfo = tarfile.TarInfo(joined_path)
- tarinfo.type = tarfile.DIRTYPE
- tarinfo.mtime = now
- tarinfo.mode = 0755
- tarinfo.uname = 'launchpad'
- tarinfo.gname = 'launchpad'
- self.tarfile.addfile(tarinfo)
-
- tarinfo = tarfile.TarInfo(path)
- tarinfo.time = now
- tarinfo.mtime = now
- tarinfo.mode = 0644
- tarinfo.size = len(contents)
- tarinfo.uname = 'launchpad'
- tarinfo.gname = 'launchpad'
- self.tarfile.addfile(tarinfo, StringIO(contents))
-
- def add_files(self, files):
- """Add a number of files to the archive.
-
- :param files: A dictionary mapping file names to file contents.
- """
-
- for filename in sorted(files.keys()):
- self.add_file(filename, files[filename])
-
-
class StorageStrategy:
"""Implementation strategy for `ExportFileStorage`.
Follow ups
