Permalink
Browse files

Archive classes refactored to reuse code

  • Loading branch information...
1 parent 530c853 commit 4b0506889a8c92e37b1108a94fc139e7adaaf175 @dvarrazzo committed Sep 4, 2012
Showing with 229 additions and 188 deletions.
  1. +49 −16 pgxnclient/archive.py
  2. +65 −0 pgxnclient/tar.py
  3. +22 −18 pgxnclient/tests/test_commands.py
  4. +0 −63 pgxnclient/utils/tar.py
  5. +0 −91 pgxnclient/utils/zip.py
  6. +93 −0 pgxnclient/zip.py
View
@@ -6,9 +6,11 @@
# This file is part of the PGXN client
-from pgxnclient.utils.tar import unpack_tar, get_meta_from_tar
-from pgxnclient.utils.zip import unpack_zip, get_meta_from_zip
+import os
+from pgxnclient.i18n import _
+from pgxnclient.utils import load_jsons
+from pgxnclient.errors import PgxnClientException
def from_spec(spec):
"""Return an `Archive` instance to handle the file requested by *spec*
@@ -21,10 +23,12 @@ def from_file(filename):
"""
# Get the metadata from an archive file
if filename.endswith('.zip'):
+ from pgxnclient.utils.zip import ZipArchive
return ZipArchive(filename)
else:
# Tar files have many naming variants. Let's not
# guess them.
+ from pgxnclient.utils.tar import TarArchive
return TarArchive(filename)
@@ -33,28 +37,57 @@ class Archive(object):
def __init__(self, filename):
self.filename = filename
- def get_meta(self):
+ def open(self):
+ """Open the archive for usage.
+
+ Raise PgxnClientException if the archive can't be open.
+ """
raise NotImplementedError
- def unpack(self, destdir):
+ def close(self):
+ """Close the archive after usage."""
raise NotImplementedError
+ def list_files(self):
+ """Return an iterable with the list of file names in the archive."""
+ raise NotImplementedError
-class TarArchive(Archive):
- """Handle .tar archives"""
- def get_meta(self):
- return get_meta_from_tar(self.filename)
+ def read(self, fn):
+ """Return a file's data from the archive."""
+ raise NotImplementedError
def unpack(self, destdir):
- return unpack_tar(self.filename, destdir)
-
+ raise NotImplementedError
-class ZipArchive(Archive):
- """Handle .zip archives"""
def get_meta(self):
- return get_meta_from_zip(self.filename)
-
- def unpack(self, destdir):
- return unpack_zip(self.filename, destdir)
+ filename = self.filename
+
+ self.open()
+ try:
+ # Return the first file with the expected name
+ for fn in self.list_files():
+ if fn.endswith('META.json'):
+ return load_jsons(self.read(fn).decode('utf8'))
+ else:
+ raise PgxnClientException(
+ _("file 'META.json' not found in archive '%s'") % filename)
+ finally:
+ self.close()
+
+ def _find_work_directory(self, destdir):
+ """
+ Choose the directory where to work.
+
+ Because we are mostly a wrapper for pgxs, let's look for a makefile.
+ The tar should contain a single base directory, so return the first
+ dir we found containing a Makefile, alternatively just return the
+ unpacked dir
+ """
+ for dir in os.listdir(destdir):
+ for fn in ('Makefile', 'makefile', 'GNUmakefile', 'configure'):
+ if os.path.exists(os.path.join(destdir, dir, fn)):
+ return os.path.join(destdir, dir)
+
+ return destdir
View
@@ -0,0 +1,65 @@
+"""
+pgxnclient -- tar file utilities
+"""
+
+# Copyright (C) 2011-2012 Daniele Varrazzo
+
+# This file is part of the PGXN client
+
+import os
+import tarfile
+
+from pgxnclient.i18n import _
+from pgxnclient.errors import PgxnClientException
+from pgxnclient.archive import Archive
+
+import logging
+logger = logging.getLogger('pgxnclient.utils.tar')
+
+class TarArchive(Archive):
+ """Handle .tar archives"""
+ _file = None
+
+ def open(self):
+ assert not self._file, "archive already open"
+ try:
+ self._file = tarfile.open(self.filename, 'r')
+ except Exception, e:
+ raise PgxnClientException(
+ _("cannot open archive '%s': %s") % (self.filename, e))
+
+ def close(self):
+ if self._file is not None:
+ self._file.close()
+ self._file = None
+
+ def list_files(self):
+ assert self._file, "archive not open"
+ return self._file.getnames()
+
+ def read(self, fn):
+ assert self._file, "archive not open"
+ return self._file.extractfile(fn).read()
+
+ def unpack(self, destdir):
+ tarname = self.filename
+ logger.info(_("unpacking: %s"), tarname)
+ destdir = os.path.abspath(destdir)
+ self.open()
+ try:
+ for fn in self.list_files():
+ fname = os.path.abspath(os.path.join(destdir, fn))
+ if not fname.startswith(destdir):
+ raise PgxnClientException(
+ _("archive file '%s' trying to escape!") % fname)
+
+ self._file.extractall(path=destdir)
+ finally:
+ self.close()
+
+ return self._find_work_directory(destdir)
+
+
+def unpack(filename, destdir):
+ return TarArchive(filename).unpack(destdir)
+
@@ -322,6 +322,14 @@ def assertCallArgs(self, pattern, args):
if not a == p:
self.fail('%s is not a %s in %s' % (a, p, args))
+# With mock patching a method seems tricky: looks there's no way to get to
+# 'self' as the mock method is unbound.
+from pgxnclient.utils.tar import TarArchive
+TarArchive.unpack_orig = TarArchive.unpack
+
+from pgxnclient.utils.zip import ZipArchive
+ZipArchive.unpack_orig = ZipArchive.unpack
+
class InstallTestCase(unittest.TestCase, Assertions):
def setUp(self):
@@ -409,13 +417,13 @@ def test_install_sudo(self):
self.assertCallArgs(['gksudo', '-d', 'hello world', self.make],
self.mock_popen.call_args_list[1][0][0][:4])
- @patch('pgxnclient.archive.unpack_tar')
+ @patch('pgxnclient.utils.tar.TarArchive.unpack')
def test_install_local_tar(self, mock_unpack):
- from pgxnclient.utils.tar import unpack
- mock_unpack.side_effect = unpack
+ fn = get_test_filename('foobar-0.42.1.tar.gz')
+ mock_unpack.side_effect = TarArchive(fn).unpack_orig
from pgxnclient.cli import main
- main(['install', '--sudo', '--', get_test_filename('foobar-0.42.1.tar.gz')])
+ main(['install', '--sudo', '--', fn])
self.assertEquals(self.mock_popen.call_count, 2)
self.assertCallArgs([self.make], self.mock_popen.call_args_list[0][0][0][:1])
@@ -424,17 +432,16 @@ def test_install_local_tar(self, mock_unpack):
make_cwd = self.mock_popen.call_args_list[1][1]['cwd']
self.assertEquals(mock_unpack.call_count, 1)
- zipname, tmpdir = mock_unpack.call_args[0]
- self.assertEqual(zipname, get_test_filename('foobar-0.42.1.tar.gz'))
+ tmpdir, = mock_unpack.call_args[0]
self.assertEqual(make_cwd, os.path.join(tmpdir, 'foobar-0.42.1'))
- @patch('pgxnclient.archive.unpack_zip')
+ @patch('pgxnclient.utils.zip.ZipArchive.unpack')
def test_install_local_zip(self, mock_unpack):
- from pgxnclient.utils.zip import unpack
- mock_unpack.side_effect = unpack
+ fn = get_test_filename('foobar-0.42.1.zip')
+ mock_unpack.side_effect = ZipArchive(fn).unpack_orig
from pgxnclient.cli import main
- main(['install', '--sudo', '--', get_test_filename('foobar-0.42.1.zip')])
+ main(['install', '--sudo', '--', fn])
self.assertEquals(self.mock_popen.call_count, 2)
self.assertCallArgs([self.make], self.mock_popen.call_args_list[0][0][0][:1])
@@ -443,8 +450,7 @@ def test_install_local_zip(self, mock_unpack):
make_cwd = self.mock_popen.call_args_list[1][1]['cwd']
self.assertEquals(mock_unpack.call_count, 1)
- zipname, tmpdir = mock_unpack.call_args[0]
- self.assertEqual(zipname, get_test_filename('foobar-0.42.1.zip'))
+ tmpdir, = mock_unpack.call_args[0]
self.assertEqual(make_cwd, os.path.join(tmpdir, 'foobar-0.42.1'))
def test_install_local_dir(self):
@@ -599,12 +605,11 @@ def test_check_psql_options(self, mock_get):
args = self.mock_popen.call_args[0][0]
self.assertEqual('somewhere', args[args.index('--host') + 1])
- @patch('pgxnclient.archive.unpack_zip')
+ @patch('pgxnclient.utils.zip.ZipArchive.unpack')
@patch('pgxnclient.api.get_file')
def test_load_local_zip(self, mock_get, mock_unpack):
mock_get.side_effect = lambda *args: self.fail('network invoked')
- from pgxnclient.utils.zip import unpack
- mock_unpack.side_effect = unpack
+ mock_unpack.side_effect = ZipArchive.unpack_orig
from pgxnclient.cli import main
main(['load', '--yes', get_test_filename('foobar-0.42.1.zip')])
@@ -616,12 +621,11 @@ def test_load_local_zip(self, mock_get, mock_unpack):
self.assertEquals(communicate.call_args[0][0],
'CREATE EXTENSION foobar;')
- @patch('pgxnclient.archive.unpack_tar')
+ @patch('pgxnclient.utils.tar.TarArchive.unpack')
@patch('pgxnclient.api.get_file')
def test_load_local_tar(self, mock_get, mock_unpack):
mock_get.side_effect = lambda *args: self.fail('network invoked')
- from pgxnclient.utils.tar import unpack
- mock_unpack.side_effect = unpack
+ mock_unpack.side_effect = TarArchive.unpack_orig
from pgxnclient.cli import main
main(['load', '--yes', get_test_filename('foobar-0.42.1.tar.gz')])
View
@@ -1,63 +0,0 @@
-"""
-pgxnclient -- tar file utilities
-"""
-
-# Copyright (C) 2011-2012 Daniele Varrazzo
-
-# This file is part of the PGXN client
-
-import os
-import tarfile
-
-from pgxnclient.utils import load_jsons
-from pgxnclient.i18n import _
-from pgxnclient.errors import PgxnClientException
-
-import logging
-logger = logging.getLogger('pgxnclient.utils.tar')
-
-def unpack(tarname, destdir):
- logger.info(_("unpacking: %s"), tarname)
- destdir = os.path.abspath(destdir)
- tf = tarfile.open(tarname, 'r')
- try:
- for fn in tf.getnames():
- if fn.startswith('.') or fn.startswith('/'):
- raise PgxnClientException(_("insecure file name in archive: %s") % fn)
-
- tf.extractall(path=destdir)
- finally:
- tf.close()
-
- # Choose the directory where to work. Because we are mostly a wrapper for
- # pgxs, let's look for a makefile. The tar should contain a single base
- # directory, so return the first dir we found containing a Makefile,
- # alternatively just return the unpacked dir
- for dir in os.listdir(destdir):
- for fn in ('Makefile', 'makefile', 'GNUmakefile', 'configure'):
- if os.path.exists(os.path.join(destdir, dir, fn)):
- return os.path.join(destdir, dir)
-
- return destdir
-
-unpack_tar = unpack # utility alias
-
-
-def get_meta_from_tar(filename):
- try:
- tf = tarfile.open(filename, 'r')
- except Exception, e:
- raise PgxnClientException(
- _("cannot open archive '%s': %s") % (filename, e))
-
- try:
- # Return the first file with the expected name
- for fn in tf.getnames():
- if fn.endswith('META.json'):
- return load_jsons(tf.extractfile(fn).read().decode('utf8'))
- else:
- raise PgxnClientException(
- _("file 'META.json' not found in archive '%s'") % filename)
- finally:
- tf.close()
-
Oops, something went wrong.

0 comments on commit 4b05068

Please sign in to comment.