Launchpad itself

Merge lp:~jtv/launchpad/exportstorage-cleanup into lp:launchpad

  1. exportstorage-cleanup
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 11178
Proposed branch: lp:~jtv/launchpad/exportstorage-cleanup
Merge into: lp:launchpad
Diff against target: 289 lines (+53/-39)
4 files modified
lib/lp/translations/utilities/gettext_mo_exporter.py (+6/-5)
lib/lp/translations/utilities/gettext_po_exporter.py (+7/-3)
lib/lp/translations/utilities/tests/test_export_file_storage.py (+18/-12)
lib/lp/translations/utilities/translation_export.py (+22/-19)
To merge this branch: bzr merge lp:~jtv/launchpad/exportstorage-cleanup
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Robert Collins (community) Approve
Review via email: mp+30418@code.launchpad.net

Commit message

Set translations export storage MIME type when adding file, not on __init__.

Description of the change

= Export Storage Strategy cleanup =

This is preparation for an overhaul of the translations export machinery.

Sometimes the export needs to store a single file. Sometimes it needs to store several, which have to be bundled up into a tarball first. A neat little "strategy" pattern hides this inside an object that just accepts files.

Currently the storage strategy is made to contain files of a single MIME type. This fits the use cases—we actually don't care about MIME types inside a tarball!—but not the use pattern once we start dumping multiple files into one storage object.

In this branch I provide the MIME type not while creating the storage object, but when adding a file. To test,
{{{
./bin/test -vvc -m lp.translations.utilities -t Storage
}}}

No lint.

Jeroen

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

Looks good - thanks.

review: Approve
Revision history for this message
Graham Binns (gmb) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/translations/utilities/gettext_mo_exporter.py'
--- lib/lp/translations/utilities/gettext_mo_exporter.py 2009-07-17 00:26:05 +0000
+++ lib/lp/translations/utilities/gettext_mo_exporter.py 2010-07-20 14:50:26 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Export module for gettext's .mo file format."""4"""Export module for gettext's .mo file format."""
@@ -7,7 +7,7 @@
77
8__all__ = [8__all__ = [
9 'GettextMOExporter',9 'GettextMOExporter',
10 'POCompiler'10 'POCompiler',
11 ]11 ]
1212
13import os13import os
@@ -70,7 +70,8 @@
70 translation_exporter.getExporterProducingTargetFileFormat(70 translation_exporter.getExporterProducingTargetFileFormat(
71 TranslationFileFormat.PO))71 TranslationFileFormat.PO))
7272
73 storage = ExportFileStorage('application/x-gmo')73 mime_type = 'application/x-gmo'
74 storage = ExportFileStorage()
7475
75 for translation_file in translation_files:76 for translation_file in translation_files:
76 # To generate MO files we need first its PO version and then,77 # To generate MO files we need first its PO version and then,
@@ -103,7 +104,7 @@
103 # GTranslator.104 # GTranslator.
104 content_type = 'application/x-gmo'105 content_type = 'application/x-gmo'
105106
106 storage.addFile(file_path, file_extension, exported_file_content)107 storage.addFile(
108 file_path, file_extension, exported_file_content, mime_type)
107109
108 return storage.export()110 return storage.export()
109
110111
=== modified file 'lib/lp/translations/utilities/gettext_po_exporter.py'
--- lib/lp/translations/utilities/gettext_po_exporter.py 2010-01-14 09:07:00 +0000
+++ lib/lp/translations/utilities/gettext_po_exporter.py 2010-07-20 14:50:26 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Export module for gettext's .po file format.4"""Export module for gettext's .po file format.
@@ -39,6 +39,7 @@
39 else:39 else:
40 return text40 return text
4141
42
42def comments_text_representation(translation_message):43def comments_text_representation(translation_message):
43 """Return text representation of the comments.44 """Return text representation of the comments.
4445
@@ -103,6 +104,7 @@
103 it.104 it.
104 :param wrap_width: The width where the text should be wrapped.105 :param wrap_width: The width where the text should be wrapped.
105 """106 """
107
106 def local_escape(text):108 def local_escape(text):
107 ret = text.replace(u'\\', u'\\\\')109 ret = text.replace(u'\\', u'\\\\')
108 ret = ret.replace(ur'"', ur'\"')110 ret = ret.replace(ur'"', ur'\"')
@@ -297,7 +299,8 @@
297 def exportTranslationFiles(self, translation_files, ignore_obsolete=False,299 def exportTranslationFiles(self, translation_files, ignore_obsolete=False,
298 force_utf8=False):300 force_utf8=False):
299 """See `ITranslationFormatExporter`."""301 """See `ITranslationFormatExporter`."""
300 storage = ExportFileStorage('application/x-po')302 mime_type = 'application/x-po'
303 storage = ExportFileStorage()
301304
302 for translation_file in translation_files:305 for translation_file in translation_files:
303 dirname = os.path.dirname(translation_file.path)306 dirname = os.path.dirname(translation_file.path)
@@ -377,7 +380,8 @@
377 encoded_file_content = self._encode_file_content(380 encoded_file_content = self._encode_file_content(
378 translation_file, exported_file_content)381 translation_file, exported_file_content)
379382
380 storage.addFile(file_path, file_extension, encoded_file_content)383 storage.addFile(
384 file_path, file_extension, encoded_file_content, mime_type)
381385
382 return storage.export()386 return storage.export()
383387
384388
=== modified file 'lib/lp/translations/utilities/tests/test_export_file_storage.py'
--- lib/lp/translations/utilities/tests/test_export_file_storage.py 2009-07-17 00:26:05 +0000
+++ lib/lp/translations/utilities/tests/test_export_file_storage.py 2010-07-20 14:50:26 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Tests for `ExportFileStorage`."""4"""Tests for `ExportFileStorage`."""
@@ -19,7 +19,8 @@
1919
20 def testEmpty(self):20 def testEmpty(self):
21 """Behaviour of empty storage."""21 """Behaviour of empty storage."""
22 storage = ExportFileStorage('application/x-po')22 mime = 'application/x-po'
23 storage = ExportFileStorage()
23 # Try not inserting any files, so the storage object remains empty.24 # Try not inserting any files, so the storage object remains empty.
24 self.assertTrue(storage._store.isEmpty())25 self.assertTrue(storage._store.isEmpty())
25 self.assertFalse(storage._store.isFull())26 self.assertFalse(storage._store.isFull())
@@ -28,25 +29,29 @@
2829
29 def testFull(self):30 def testFull(self):
30 """Behaviour of isFull."""31 """Behaviour of isFull."""
31 storage = ExportFileStorage('application/x-po')32 mime = 'application/x-po'
32 storage.addFile('/tmp/a/test/file.po', 'po', 'test file')33 storage = ExportFileStorage()
34 storage.addFile('/tmp/a/test/file.po', 'po', 'test file', mime)
33 # The storage object starts out with a SingleFileStorageStrategy, so35 # The storage object starts out with a SingleFileStorageStrategy, so
34 # it's full now that we've added one file.36 # it's full now that we've added one file.
35 self.assertTrue(storage._store.isFull())37 self.assertTrue(storage._store.isFull())
36 # If we add another file however, the storage object transparently38 # If we add another file however, the storage object transparently
37 # switches to a TarballFileStorageStrategy. That type of storage39 # switches to a TarballFileStorageStrategy. That type of storage
38 # object is never full.40 # object is never full.
39 storage.addFile('/tmp/another/test/file.po', 'po', 'test file two')41 storage.addFile(
42 '/tmp/another/test/file.po', 'po', 'test file two', mime)
40 self.assertFalse(storage._store.isFull())43 self.assertFalse(storage._store.isFull())
41 # We can now add any number of files without filling the storage44 # We can now add any number of files without filling the storage
42 # object.45 # object.
43 storage.addFile('/tmp/yet/another/test/file.po', 'po', 'test file 3')46 storage.addFile(
47 '/tmp/yet/another/test/file.po', 'po', 'test file 3', mime)
44 self.assertFalse(storage._store.isFull())48 self.assertFalse(storage._store.isFull())
4549
46 def testSingle(self):50 def testSingle(self):
47 """Test export of single file."""51 """Test export of single file."""
48 storage = ExportFileStorage('application/x-po')52 mime = 'application/x-po'
49 storage.addFile('/tmp/a/test/file.po', 'po', 'test file')53 storage = ExportFileStorage()
54 storage.addFile('/tmp/a/test/file.po', 'po', 'test file', mime)
50 outfile = storage.export()55 outfile = storage.export()
51 self.assertEquals(outfile.path, '/tmp/a/test/file.po')56 self.assertEquals(outfile.path, '/tmp/a/test/file.po')
52 self.assertEquals(outfile.file_extension, 'po')57 self.assertEquals(outfile.file_extension, 'po')
@@ -54,9 +59,11 @@
5459
55 def testTarball(self):60 def testTarball(self):
56 """Test export of tarball."""61 """Test export of tarball."""
57 storage = ExportFileStorage('application/x-po')62 mime = 'application/x-po'
58 storage.addFile('/tmp/a/test/file.po', 'po', 'test file')63 storage = ExportFileStorage()
59 storage.addFile('/tmp/another/test.po', 'po', 'another test file')64 storage.addFile('/tmp/a/test/file.po', 'po', 'test file', mime)
65 storage.addFile(
66 '/tmp/another/test.po', 'po', 'another test file', mime)
60 outfile = storage.export()67 outfile = storage.export()
61 tarball = TarFile.open(mode='r|gz', fileobj=StringIO(outfile.read()))68 tarball = TarFile.open(mode='r|gz', fileobj=StringIO(outfile.read()))
62 elements = set(tarball.getnames())69 elements = set(tarball.getnames())
@@ -68,4 +75,3 @@
68 suite = unittest.TestSuite()75 suite = unittest.TestSuite()
69 suite.addTest(unittest.makeSuite(ExportFileStorageTestCase))76 suite.addTest(unittest.makeSuite(ExportFileStorageTestCase))
70 return suite77 return suite
71
7278
=== modified file 'lib/lp/translations/utilities/translation_export.py'
--- lib/lp/translations/utilities/translation_export.py 2009-10-13 17:25:19 +0000
+++ lib/lp/translations/utilities/translation_export.py 2010-07-20 14:50:26 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Components for exporting translation files."""4"""Components for exporting translation files."""
@@ -27,12 +27,13 @@
27from lp.translations.interfaces.translationfileformat import (27from lp.translations.interfaces.translationfileformat import (
28 TranslationFileFormat)28 TranslationFileFormat)
2929
30
30class ExportedTranslationFile:31class ExportedTranslationFile:
31 """See `IExportedTranslationFile`."""32 """See `IExportedTranslationFile`."""
32 implements(IExportedTranslationFile)33 implements(IExportedTranslationFile)
3334
34 def __init__(self, content_file):35 def __init__(self, content_file):
35 self._content_file = content_file36 self._content_file = content_file
36 self.content_type = None37 self.content_type = None
37 self.path = None38 self.path = None
38 self.file_extension = None39 self.file_extension = None
@@ -178,7 +179,8 @@
178 Storage for single files is implemented by `SingleFileStorageStrategy`;179 Storage for single files is implemented by `SingleFileStorageStrategy`;
179 multiple files go into a `TarballFileStorageStrategy`.180 multiple files go into a `TarballFileStorageStrategy`.
180 """181 """
181 def addFile(self, path, extension, content):182
183 def addFile(self, path, extension, content, content_type):
182 """Add a file to be stored."""184 """Add a file to be stored."""
183 raise NotImplementedError()185 raise NotImplementedError()
184186
@@ -208,18 +210,15 @@
208210
209 path = None211 path = None
210 extension = None212 extension = None
211 mimetype = None213
212214 def addFile(self, path, extension, content, mime_type):
213 def __init__(self, mimetype):
214 self.mimetype = mimetype
215
216 def addFile(self, path, extension, content):
217 """See `StorageStrategy`."""215 """See `StorageStrategy`."""
218 assert path is not None, "Storing file without path."216 assert path is not None, "Storing file without path."
219 assert self.path is None, "Multiple files added; expected just one."217 assert self.path is None, "Multiple files added; expected just one."
220 self.path = path218 self.path = path
221 self.extension = extension219 self.extension = extension
222 self.content = content220 self.content = content
221 self.mime_type = mime_type
223222
224 def isEmpty(self):223 def isEmpty(self):
225 """See `StorageStrategy`."""224 """See `StorageStrategy`."""
@@ -238,7 +237,7 @@
238 output = ExportedTranslationFile(StringIO(self.content))237 output = ExportedTranslationFile(StringIO(self.content))
239 output.path = self.path238 output.path = self.path
240 # We use x-po for consistency with other .po editors like GTranslator.239 # We use x-po for consistency with other .po editors like GTranslator.
241 output.content_type = self.mimetype240 output.content_type = self.mime_type
242 output.file_extension = self.extension241 output.file_extension = self.extension
243 return output242 return output
244243
@@ -251,6 +250,8 @@
251 as soon as it is added. There is no need to keep the full contents of the250 as soon as it is added. There is no need to keep the full contents of the
252 tarball in memory at any single time.251 tarball in memory at any single time.
253 """252 """
253 mime_type = 'application/x-gtar'
254
254 empty = False255 empty = False
255256
256 def __init__(self, single_file_storage=None):257 def __init__(self, single_file_storage=None):
@@ -258,11 +259,13 @@
258 self.buffer = tempfile.TemporaryFile()259 self.buffer = tempfile.TemporaryFile()
259 self.tar_writer = LaunchpadWriteTarFile(self.buffer)260 self.tar_writer = LaunchpadWriteTarFile(self.buffer)
260 if single_file_storage is not None:261 if single_file_storage is not None:
261 self.addFile(single_file_storage.path,262 self.addFile(
262 single_file_storage.extension, single_file_storage.content)263 single_file_storage.path, single_file_storage.extension,
264 single_file_storage.content, single_file_storage.mime_type)
263265
264 def addFile(self, path, extension, content):266 def addFile(self, path, extension, content, mime_type):
265 """See `StorageStrategy`."""267 """See `StorageStrategy`."""
268 # Tarballs don't store MIME types, so ignore that.
266 self.empty = False269 self.empty = False
267 self.tar_writer.add_file(path, content)270 self.tar_writer.add_file(path, content)
268271
@@ -288,19 +291,20 @@
288 # For tar.gz files, the standard content type is application/x-gtar.291 # For tar.gz files, the standard content type is application/x-gtar.
289 # You can see more info on292 # You can see more info on
290 # http://en.wikipedia.org/wiki/List_of_archive_formats293 # http://en.wikipedia.org/wiki/List_of_archive_formats
291 output.content_type = 'application/x-gtar'294 output.content_type = self.mime_type
292 output.file_extension = 'tar.gz'295 output.file_extension = 'tar.gz'
293 return output296 return output
294297
295298
296class ExportFileStorage:299class ExportFileStorage:
297 """Store files to export, either as tarball or plain single file."""300 """Store files to export, either as tarball or plain single file."""
298 def __init__(self, mimetype):301
302 def __init__(self):
299 # Start out with a single file. We can replace that strategy later if303 # Start out with a single file. We can replace that strategy later if
300 # we get more than one file.304 # we get more than one file.
301 self._store = SingleFileStorageStrategy(mimetype)305 self._store = SingleFileStorageStrategy()
302306
303 def addFile(self, path, extension, content):307 def addFile(self, path, extension, content, mime_type):
304 """Add file to be stored.308 """Add file to be stored.
305309
306 :param path: location and name of this file, relative to root of tar310 :param path: location and name of this file, relative to root of tar
@@ -312,10 +316,9 @@
312 # We're still using a single-file storage strategy, but we just316 # We're still using a single-file storage strategy, but we just
313 # received our second file. Switch to tarball strategy.317 # received our second file. Switch to tarball strategy.
314 self._store = TarballFileStorageStrategy(self._store)318 self._store = TarballFileStorageStrategy(self._store)
315 self._store.addFile(path, extension, content)319 self._store.addFile(path, extension, content, mime_type)
316320
317 def export(self):321 def export(self):
318 """Export as `ExportedTranslationFile`."""322 """Export as `ExportedTranslationFile`."""
319 assert not self._store.isEmpty(), "Got empty list of files to export."323 assert not self._store.isEmpty(), "Got empty list of files to export."
320 return self._store.export()324 return self._store.export()
321