From ef89efee3d47117ffc2ad360939469ea9daf632e Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 22 2018 05:25:59 +0000 Subject: [PATCH 1/8] hub: throw GenericError in `get_archive_file()` relates: #719 --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 4e0e558..c9f32b5 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -4337,11 +4337,13 @@ def get_archive_file(archive_id, filename): size: uncompressed size of the file (integer) """ files = list_archive_files(archive_id) + if not files: + raise koji.GenericError('Archive#%s doesn\'t contain any files' % archive_id) for file_info in files: if file_info['name'] == filename: return file_info - #otherwise - return None + else: + raise koji.GenericError('No such file: %s in archive#%s' % (filename, archive_id)) def list_task_output(taskID, stat=False, all_volumes=False): """List the files generated by the task with the given ID. This diff --git a/tests/test_hub/test_get_archive_file.py b/tests/test_hub/test_get_archive_file.py new file mode 100644 index 0000000..4470c2f --- /dev/null +++ b/tests/test_hub/test_get_archive_file.py @@ -0,0 +1,43 @@ +import unittest +import mock + +import koji +import kojihub + +FILES = [{'archive_id': 1, + 'name': 'archive1.zip', + 'size': 1024}, + {'archive_id': 1, + 'name': 'archive2.jar', + 'size': 4096}] + +EMPTY_FILES = [] + + +class TestGetArchiveFile(unittest.TestCase): + + @mock.patch('kojihub.list_archive_files') + def test_simple(self, list_archive_files): + list_archive_files.return_value = FILES + + rv = kojihub.get_archive_file(1, 'archive1.zip') + list_archive_files.assert_called_with(1) + self.assertEqual(rv, FILES[0]) + + @mock.patch('kojihub.list_archive_files') + def test_empty_files(self, list_archive_files): + list_archive_files.return_value = EMPTY_FILES + + with self.assertRaises(koji.GenericError) as cm: + kojihub.get_archive_file(1, 'archive1.zip') + list_archive_files.assert_called_with(1) + self.assertEqual(cm.exception.args[0], 'Archive#1 doesn\'t contain any files') + + @mock.patch('kojihub.list_archive_files') + def test_non_existing_file(self, list_archive_files): + list_archive_files.return_value = FILES + + with self.assertRaises(koji.GenericError) as cm: + kojihub.get_archive_file(1, 'archive3.xml') + list_archive_files.assert_called_with(1) + self.assertEqual(cm.exception.args[0], 'No such file: archive3.xml in archive#1') From c093aa65dc01cad0b99bc41d1773c8a3daf24dce Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 22 2018 05:25:59 +0000 Subject: [PATCH 2/8] hub: add strict behaviors for `get_archive_file` and `list_archive_files` --- diff --git a/hub/kojihub.py b/hub/kojihub.py index c9f32b5..f425469 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -3958,7 +3958,6 @@ def list_archives(buildID=None, buildrootID=None, componentBuildrootID=None, hos buildroot with that ID. If hostID is not null it will restrict the list to archives built on the host with that ID. If filename, size, and/or checksum are not null it will filter the results to entries matching the provided values. - Returns a list of maps containing the following keys: id: unique id of the archive file (integer) @@ -4283,7 +4282,7 @@ def _get_tarball_list(archive_id, tarpath): archive.close() return result -def list_archive_files(archive_id, queryOpts=None): +def list_archive_files(archive_id, queryOpts=None, strict=False): """ Get information about the files contained in the archive with the given ID. Returns a list of maps with with following keys: @@ -4291,6 +4290,8 @@ def list_archive_files(archive_id, queryOpts=None): archive_id: id of the archive the file is contained in (integer) name: name of the file (string) size: uncompressed size of the file (integer) + + if strict is True, it will raise GenericError when there is no files found for specified archive_id. """ archive_info = get_archive(archive_id, strict=True) @@ -4315,18 +4316,23 @@ def list_archive_files(archive_id, queryOpts=None): archive_info.update(image_archive) file_path = os.path.join(koji.pathinfo.imagebuild(build_info), archive_info['filename']) - else: - return _applyQueryOpts([], queryOpts) + + result = [] if archive_type['name'] in ('zip', 'jar'): - return _applyQueryOpts(_get_zipfile_list(archive_id, file_path), queryOpts) + result = _applyQueryOpts(_get_zipfile_list(archive_id, file_path), queryOpts) elif archive_type['name'] == 'tar': - return _applyQueryOpts(_get_tarball_list(archive_id, file_path), queryOpts) + result = _applyQueryOpts(_get_tarball_list(archive_id, file_path), queryOpts) else: # XXX support other archive types - return _applyQueryOpts([], queryOpts) + result = _applyQueryOpts(result, queryOpts) + + if strict and not result: + raise koji.GenericError('Archive#%s doesn\'t contain any files' % archive_id) + return result + -def get_archive_file(archive_id, filename): +def get_archive_file(archive_id, filename, strict=False): """ Get information about a file with the given filename contained in the archive with the given ID. @@ -4335,15 +4341,20 @@ def get_archive_file(archive_id, filename): archive_id: id of the archive the file is contained in (integer) name: name of the file (string) size: uncompressed size of the file (integer) + + if strict is True, it will raise GenericError when there is no files found for specified archive_id, + else returns None. """ - files = list_archive_files(archive_id) - if not files: - raise koji.GenericError('Archive#%s doesn\'t contain any files' % archive_id) + files = list_archive_files(archive_id, strict=strict) for file_info in files: if file_info['name'] == filename: return file_info else: - raise koji.GenericError('No such file: %s in archive#%s' % (filename, archive_id)) + if strict: + raise koji.GenericError('No such file: %s in archive#%s' % (filename, archive_id)) + else: + return None + def list_task_output(taskID, stat=False, all_volumes=False): """List the files generated by the task with the given ID. This diff --git a/tests/test_hub/test_get_archive_file.py b/tests/test_hub/test_get_archive_file.py index 4470c2f..466e31d 100644 --- a/tests/test_hub/test_get_archive_file.py +++ b/tests/test_hub/test_get_archive_file.py @@ -21,23 +21,41 @@ class TestGetArchiveFile(unittest.TestCase): list_archive_files.return_value = FILES rv = kojihub.get_archive_file(1, 'archive1.zip') - list_archive_files.assert_called_with(1) + list_archive_files.assert_called_with(1, strict=False) + self.assertEqual(rv, FILES[0]) + + list_archive_files.reset_mock() + rv = kojihub.get_archive_file(1, 'archive1.zip', strict=True) + list_archive_files.assert_called_with(1, strict=True) self.assertEqual(rv, FILES[0]) @mock.patch('kojihub.list_archive_files') def test_empty_files(self, list_archive_files): list_archive_files.return_value = EMPTY_FILES + rv = kojihub.get_archive_file(1, 'archive1.zip') + list_archive_files.assert_called_with(1, strict=False) + self.assertIsNone(rv) + + list_archive_files.reset_mock() + list_archive_files.side_effect = koji.GenericError('error message') + with self.assertRaises(koji.GenericError) as cm: - kojihub.get_archive_file(1, 'archive1.zip') - list_archive_files.assert_called_with(1) - self.assertEqual(cm.exception.args[0], 'Archive#1 doesn\'t contain any files') + kojihub.get_archive_file(1, 'archive1.zip', strict=True) + list_archive_files.assert_called_with(1, strict=True) + self.assertEqual(cm.exception.args[0], 'error message') @mock.patch('kojihub.list_archive_files') def test_non_existing_file(self, list_archive_files): list_archive_files.return_value = FILES + rv = kojihub.get_archive_file(1, 'archive3.xml') + list_archive_files.assert_called_with(1, strict=False) + self.assertEqual(rv, None) + + list_archive_files.reset_mock() + with self.assertRaises(koji.GenericError) as cm: - kojihub.get_archive_file(1, 'archive3.xml') - list_archive_files.assert_called_with(1) + kojihub.get_archive_file(1, 'archive3.xml', strict=True) + list_archive_files.assert_called_with(1, strict=True) self.assertEqual(cm.exception.args[0], 'No such file: archive3.xml in archive#1') From 497f547c0871b7aa106487b4029308328ef72750 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 22 2018 05:25:59 +0000 Subject: [PATCH 3/8] hub: unittest for `list_archive_files()` --- diff --git a/hub/kojihub.py b/hub/kojihub.py index f425469..8589ed8 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -3958,6 +3958,7 @@ def list_archives(buildID=None, buildrootID=None, componentBuildrootID=None, hos buildroot with that ID. If hostID is not null it will restrict the list to archives built on the host with that ID. If filename, size, and/or checksum are not null it will filter the results to entries matching the provided values. + Returns a list of maps containing the following keys: id: unique id of the archive file (integer) @@ -4316,15 +4317,17 @@ def list_archive_files(archive_id, queryOpts=None, strict=False): archive_info.update(image_archive) file_path = os.path.join(koji.pathinfo.imagebuild(build_info), archive_info['filename']) + else: + # TODO: support other archive btypes + file_path = None result = [] - if archive_type['name'] in ('zip', 'jar'): + if file_path and archive_type['name'] in ('zip', 'jar'): result = _applyQueryOpts(_get_zipfile_list(archive_id, file_path), queryOpts) - elif archive_type['name'] == 'tar': + elif file_path and archive_type['name'] == 'tar': result = _applyQueryOpts(_get_tarball_list(archive_id, file_path), queryOpts) else: - # XXX support other archive types result = _applyQueryOpts(result, queryOpts) if strict and not result: diff --git a/tests/test_hub/test_list_archive_files.py b/tests/test_hub/test_list_archive_files.py new file mode 100644 index 0000000..b1037e8 --- /dev/null +++ b/tests/test_hub/test_list_archive_files.py @@ -0,0 +1,121 @@ +import unittest +import mock + +import koji +import kojihub + +GET_ARCHIVE_RV = {'id': 1, 'build_id': 2, 'type_id': 3, 'filename': 'somearchive.zip'} +GET_ARCHIVE_TYPE_RV = {'id': 3, 'name': 'zip'} +GET_BUILD_RV = {'id': 2, 'name': 'somebuild', 'version': '1.2.3', 'release': '1.el6', 'volume_name': 'archive_vol'} + + +class TestListArchiveFiles(unittest.TestCase): + def setUp(self): + self.mm = mock.MagicMock() + self.mm.get_image_build = mock.patch('kojihub.get_image_build', return_value=None).start() + self.mm.get_win_build = mock.patch('kojihub.get_win_build', return_value=None).start() + self.mm.get_maven_build = mock.patch('kojihub.get_maven_build', return_value=None).start() + self.mm.get_build = mock.patch('kojihub.get_build', return_value=GET_BUILD_RV).start() + self.mm.get_archive_type = mock.patch('kojihub.get_archive_type', return_value=GET_ARCHIVE_TYPE_RV).start() + self.mm.get_archive = mock.patch('kojihub.get_archive', return_value=GET_ARCHIVE_RV).start() + + def tearDown(self): + mock.patch.stopall() + + def test_simple(self): + rv = kojihub.list_archive_files(1) + self.mm.get_archive.assert_called_once_with(1, strict=True) + self.mm.get_archive_type.assert_called_once_with(type_id=3, strict=True) + self.mm.get_build.assert_called_once_with(2, strict=True) + self.mm.get_maven_build.assert_called_once_with(2) + self.mm.get_win_build.assert_called_once_with(2) + self.mm.get_image_build.assert_called_once_with(2) + self.assertListEqual(rv, []) + + def test_simple_strict(self): + with self.assertRaises(koji.GenericError) as cm: + kojihub.list_archive_files(1, strict=True) + self.mm.get_archive.assert_called_once_with(1, strict=True) + self.mm.get_archive_type.assert_called_once_with(type_id=3, strict=True) + self.mm.get_build.assert_called_once_with(2, strict=True) + self.mm.get_maven_build.assert_called_once_with(2) + self.mm.get_win_build.assert_called_once_with(2) + self.mm.get_image_build.assert_called_once_with(2) + self.assertEqual(cm.exception.args[0], 'Archive#1 doesn\'t contain any files') + + @mock.patch('kojihub.get_maven_archive', return_value={'archive_id': 1, + 'group_id': 'gid', + 'artifact_id': 'aid', + 'version': '1.0.0'}) + @mock.patch('kojihub._get_zipfile_list', return_value=[{'archive_id': 1, + 'name': 'file1', + 'size': 4096, + 'mtime': 1000}, + {'archive_id': 1, + 'name': 'file2', + 'size': 512000, + 'mtime': 103420}, + ]) + def test_maven_archive(self, get_zipfile_list, get_maven_archive): + self.mm.get_maven_build.return_value = {'build_id': 2, + 'group_id': 'gid', + 'artifact_id': 'aid', + 'version': '1.0.0'} + rv = kojihub.list_archive_files(1) + get_maven_archive.assert_called_once_with(1, strict=True) + get_zipfile_list.assert_called_once_with(1, + '/mnt/koji/vol/archive_vol/packages' + '/somebuild/1.2.3/1.el6/maven/gid/aid/1.0.0/somearchive.zip') + self.assertListEqual(rv, [{'archive_id': 1, 'mtime': 1000, 'name': 'file1', 'size': 4096}, + {'archive_id': 1, 'mtime': 103420, 'name': 'file2', 'size': 512000}]) + + @mock.patch('kojihub.get_win_archive', return_value={'archive_id': 1, + 'relpath': 'rpath', + 'platform': 'all', + 'version': 'src'}) + @mock.patch('kojihub._get_zipfile_list', return_value=[{'archive_id': 1, + 'name': 'file1', + 'size': 4096, + 'mtime': 1000}, + {'archive_id': 1, + 'name': 'file2', + 'size': 512000, + 'mtime': 103420}, + ]) + def test_win_archive(self, get_zipfile_list, get_win_archive): + self.mm.get_win_build.return_value = {'build_id': 2, + 'platform': 'all'} + rv = kojihub.list_archive_files(1) + get_win_archive.assert_called_once_with(1, strict=True) + get_zipfile_list.assert_called_once_with(1, + '/mnt/koji/vol/archive_vol/packages' + '/somebuild/1.2.3/1.el6/win/rpath/somearchive.zip') + self.assertListEqual(rv, [{'archive_id': 1, 'mtime': 1000, 'name': 'file1', 'size': 4096}, + {'archive_id': 1, 'mtime': 103420, 'name': 'file2', 'size': 512000}]) + + @mock.patch('kojihub.get_image_archive', return_value={'archive_id': 1, + 'arch': 'noarch'}) + @mock.patch('kojihub._get_tarball_list', return_value=[{'archive_id': 1, + 'name': 'file1', + 'size': 4096, + 'mtime': 1000, + 'mode': '0755', + 'user': 1000, + 'group': 1000}, + {'archive_id': 1, + 'name': 'file2', + 'size': 512000, + 'mtime': 103420, + 'mode': '0644', + 'user': 1001, + 'group': 1001} + ]) + def test_image_archive(self, get_tarball_list, get_image_archive): + self.mm.get_archive_type.return_value = {'id': 3, 'name': 'tar'} + self.mm.get_image_build.return_value = {'build_id': 2} + rv = kojihub.list_archive_files(1, queryOpts={'countOnly': True}) + get_image_archive.assert_called_once_with(1, strict=True) + get_tarball_list.assert_called_once_with(1, + '/mnt/koji/vol/archive_vol/packages' + '/somebuild/1.2.3/1.el6/images/somearchive.zip') + self.assertEqual(rv, 2) From 34fdd92123a2fa3c0abd14d970a24b9c8d9d74b3 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 22 2018 05:25:59 +0000 Subject: [PATCH 4/8] simplify strict logic in `get_archive_file` --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 8589ed8..09f6c11 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -4352,11 +4352,9 @@ def get_archive_file(archive_id, filename, strict=False): for file_info in files: if file_info['name'] == filename: return file_info - else: - if strict: - raise koji.GenericError('No such file: %s in archive#%s' % (filename, archive_id)) - else: - return None + if strict: + raise koji.GenericError('No such file: %s in archive#%s' % (filename, archive_id)) + return None def list_task_output(taskID, stat=False, all_volumes=False): From 389e127fa499e051626779bb159126aff6b80da1 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 22 2018 05:25:59 +0000 Subject: [PATCH 5/8] use double quotes instead of escaping --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 09f6c11..c170ea7 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -4331,7 +4331,7 @@ def list_archive_files(archive_id, queryOpts=None, strict=False): result = _applyQueryOpts(result, queryOpts) if strict and not result: - raise koji.GenericError('Archive#%s doesn\'t contain any files' % archive_id) + raise koji.GenericError("Archive#%s doesn't contain any files" % archive_id) return result diff --git a/tests/test_hub/test_list_archive_files.py b/tests/test_hub/test_list_archive_files.py index b1037e8..8b07ccd 100644 --- a/tests/test_hub/test_list_archive_files.py +++ b/tests/test_hub/test_list_archive_files.py @@ -1,23 +1,32 @@ import unittest + import mock import koji import kojihub -GET_ARCHIVE_RV = {'id': 1, 'build_id': 2, 'type_id': 3, 'filename': 'somearchive.zip'} +GET_ARCHIVE_RV = {'id': 1, 'build_id': 2, 'type_id': 3, + 'filename': 'somearchive.zip'} GET_ARCHIVE_TYPE_RV = {'id': 3, 'name': 'zip'} -GET_BUILD_RV = {'id': 2, 'name': 'somebuild', 'version': '1.2.3', 'release': '1.el6', 'volume_name': 'archive_vol'} +GET_BUILD_RV = {'id': 2, 'name': 'somebuild', 'version': '1.2.3', + 'release': '1.el6', 'volume_name': 'archive_vol'} class TestListArchiveFiles(unittest.TestCase): def setUp(self): self.mm = mock.MagicMock() - self.mm.get_image_build = mock.patch('kojihub.get_image_build', return_value=None).start() - self.mm.get_win_build = mock.patch('kojihub.get_win_build', return_value=None).start() - self.mm.get_maven_build = mock.patch('kojihub.get_maven_build', return_value=None).start() - self.mm.get_build = mock.patch('kojihub.get_build', return_value=GET_BUILD_RV).start() - self.mm.get_archive_type = mock.patch('kojihub.get_archive_type', return_value=GET_ARCHIVE_TYPE_RV).start() - self.mm.get_archive = mock.patch('kojihub.get_archive', return_value=GET_ARCHIVE_RV).start() + self.mm.get_image_build = mock.patch('kojihub.get_image_build', + return_value=None).start() + self.mm.get_win_build = mock.patch('kojihub.get_win_build', + return_value=None).start() + self.mm.get_maven_build = mock.patch('kojihub.get_maven_build', + return_value=None).start() + self.mm.get_build = mock.patch('kojihub.get_build', + return_value=GET_BUILD_RV).start() + self.mm.get_archive_type = mock.patch('kojihub.get_archive_type', + return_value=GET_ARCHIVE_TYPE_RV).start() + self.mm.get_archive = mock.patch('kojihub.get_archive', + return_value=GET_ARCHIVE_RV).start() def tearDown(self): mock.patch.stopall() @@ -25,7 +34,8 @@ class TestListArchiveFiles(unittest.TestCase): def test_simple(self): rv = kojihub.list_archive_files(1) self.mm.get_archive.assert_called_once_with(1, strict=True) - self.mm.get_archive_type.assert_called_once_with(type_id=3, strict=True) + self.mm.get_archive_type.assert_called_once_with(type_id=3, + strict=True) self.mm.get_build.assert_called_once_with(2, strict=True) self.mm.get_maven_build.assert_called_once_with(2) self.mm.get_win_build.assert_called_once_with(2) @@ -36,17 +46,20 @@ class TestListArchiveFiles(unittest.TestCase): with self.assertRaises(koji.GenericError) as cm: kojihub.list_archive_files(1, strict=True) self.mm.get_archive.assert_called_once_with(1, strict=True) - self.mm.get_archive_type.assert_called_once_with(type_id=3, strict=True) + self.mm.get_archive_type.assert_called_once_with(type_id=3, + strict=True) self.mm.get_build.assert_called_once_with(2, strict=True) self.mm.get_maven_build.assert_called_once_with(2) self.mm.get_win_build.assert_called_once_with(2) self.mm.get_image_build.assert_called_once_with(2) - self.assertEqual(cm.exception.args[0], 'Archive#1 doesn\'t contain any files') + self.assertEqual(cm.exception.args[0], + "Archive#1 doesn't contain any files") - @mock.patch('kojihub.get_maven_archive', return_value={'archive_id': 1, - 'group_id': 'gid', - 'artifact_id': 'aid', - 'version': '1.0.0'}) + @mock.patch('kojihub.get_maven_archive', + return_value={'archive_id': 1, + 'group_id': 'gid', + 'artifact_id': 'aid', + 'version': '1.0.0'}) @mock.patch('kojihub._get_zipfile_list', return_value=[{'archive_id': 1, 'name': 'file1', 'size': 4096, @@ -66,8 +79,10 @@ class TestListArchiveFiles(unittest.TestCase): get_zipfile_list.assert_called_once_with(1, '/mnt/koji/vol/archive_vol/packages' '/somebuild/1.2.3/1.el6/maven/gid/aid/1.0.0/somearchive.zip') - self.assertListEqual(rv, [{'archive_id': 1, 'mtime': 1000, 'name': 'file1', 'size': 4096}, - {'archive_id': 1, 'mtime': 103420, 'name': 'file2', 'size': 512000}]) + self.assertListEqual(rv, [ + {'archive_id': 1, 'mtime': 1000, 'name': 'file1', 'size': 4096}, + {'archive_id': 1, 'mtime': 103420, 'name': 'file2', + 'size': 512000}]) @mock.patch('kojihub.get_win_archive', return_value={'archive_id': 1, 'relpath': 'rpath', @@ -90,8 +105,10 @@ class TestListArchiveFiles(unittest.TestCase): get_zipfile_list.assert_called_once_with(1, '/mnt/koji/vol/archive_vol/packages' '/somebuild/1.2.3/1.el6/win/rpath/somearchive.zip') - self.assertListEqual(rv, [{'archive_id': 1, 'mtime': 1000, 'name': 'file1', 'size': 4096}, - {'archive_id': 1, 'mtime': 103420, 'name': 'file2', 'size': 512000}]) + self.assertListEqual(rv, [ + {'archive_id': 1, 'mtime': 1000, 'name': 'file1', 'size': 4096}, + {'archive_id': 1, 'mtime': 103420, 'name': 'file2', + 'size': 512000}]) @mock.patch('kojihub.get_image_archive', return_value={'archive_id': 1, 'arch': 'noarch'}) From 24e8a9cf99b17fc202c01f4d329cfb41ef24ef0d Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 22 2018 05:25:59 +0000 Subject: [PATCH 6/8] better fomatted docstr --- diff --git a/hub/kojihub.py b/hub/kojihub.py index c170ea7..15317f9 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -4292,7 +4292,11 @@ def list_archive_files(archive_id, queryOpts=None, strict=False): name: name of the file (string) size: uncompressed size of the file (integer) - if strict is True, it will raise GenericError when there is no files found for specified archive_id. + If strict is True, raise GenericError if: + - there are no files found for the archive + - the archive is not a type we are able to expand + + Regardless of strict, an error will be raised if the archive_id is invalid """ archive_info = get_archive(archive_id, strict=True) @@ -4345,8 +4349,11 @@ def get_archive_file(archive_id, filename, strict=False): name: name of the file (string) size: uncompressed size of the file (integer) - if strict is True, it will raise GenericError when there is no files found for specified archive_id, - else returns None. + If strict is True, raise GenericError if: + - this file is not found in the archive + - the archive is not a type we are able to expand + + Regardless of strict, an error will be raised if the archive_id is invalid """ files = list_archive_files(archive_id, strict=strict) for file_info in files: From 044521a7feb145b3d3861d14a8e3aab71501f8e6 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 22 2018 05:25:59 +0000 Subject: [PATCH 7/8] more spicified errors --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 15317f9..bc0bd5d 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -24,42 +24,42 @@ import base64 import calendar -import urlparse -import koji -import koji.auth -import koji.db -import koji.plugin -import koji.policy import datetime import errno -import logging import fcntl import fnmatch -from koji.util import md5_constructor -from koji.util import sha1_constructor -from koji.util import dslice -from koji.util import multi_fnmatch -from koji.util import safer_move +import functools import json +import logging import os import re -import rpm import shutil import stat import subprocess import sys import tarfile import tempfile -import traceback import time +import traceback +import urlparse import xmlrpclib import zipfile -import functools + +import rpm import six +import koji +import koji.auth +import koji.db +import koji.plugin +import koji.policy import koji.xmlrpcplus from koji.context import context - +from koji.util import dslice +from koji.util import md5_constructor +from koji.util import multi_fnmatch +from koji.util import safer_move +from koji.util import sha1_constructor logger = logging.getLogger('koji.hub') @@ -4322,21 +4322,25 @@ def list_archive_files(archive_id, queryOpts=None, strict=False): file_path = os.path.join(koji.pathinfo.imagebuild(build_info), archive_info['filename']) else: - # TODO: support other archive btypes - file_path = None - - result = [] + # TODO: support other build types + if strict: + raise koji.GenericError("Unsupported build type") + return _applyQueryOpts([], queryOpts) - if file_path and archive_type['name'] in ('zip', 'jar'): - result = _applyQueryOpts(_get_zipfile_list(archive_id, file_path), queryOpts) - elif file_path and archive_type['name'] == 'tar': - result = _applyQueryOpts(_get_tarball_list(archive_id, file_path), queryOpts) + if archive_type['name'] in ('zip', 'jar'): + filelist = _get_zipfile_list(archive_id, file_path) + elif archive_type['name'] == 'tar': + filelist = _get_tarball_list(archive_id, file_path) else: - result = _applyQueryOpts(result, queryOpts) + # TODO: support other archive types + if strict: + raise koji.GenericError( + "Unsupported archive type: %s" % archive_type['name']) + filelist = [] - if strict and not result: + if strict and not filelist: raise koji.GenericError("Archive#%s doesn't contain any files" % archive_id) - return result + return _applyQueryOpts(filelist, queryOpts) def get_archive_file(archive_id, filename, strict=False): From 68c38f03388f61f43f622fc3c539a7d8761a7acc Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 22 2018 06:15:24 +0000 Subject: [PATCH 8/8] do not raise error when archive is empty --- diff --git a/hub/kojihub.py b/hub/kojihub.py index bc0bd5d..1b21a2d 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -4293,8 +4293,8 @@ def list_archive_files(archive_id, queryOpts=None, strict=False): size: uncompressed size of the file (integer) If strict is True, raise GenericError if: - - there are no files found for the archive - - the archive is not a type we are able to expand + - build btype of this archive belong to is not maven, win or image + - archive_type is not that we are able to expand Regardless of strict, an error will be raised if the archive_id is invalid """ @@ -4337,9 +4337,6 @@ def list_archive_files(archive_id, queryOpts=None, strict=False): raise koji.GenericError( "Unsupported archive type: %s" % archive_type['name']) filelist = [] - - if strict and not filelist: - raise koji.GenericError("Archive#%s doesn't contain any files" % archive_id) return _applyQueryOpts(filelist, queryOpts) @@ -4355,7 +4352,8 @@ def get_archive_file(archive_id, filename, strict=False): If strict is True, raise GenericError if: - this file is not found in the archive - - the archive is not a type we are able to expand + - build btype of this archive belong to is not maven, win or image + - archive_type is not that we are able to expand Regardless of strict, an error will be raised if the archive_id is invalid """ diff --git a/tests/test_hub/test_list_archive_files.py b/tests/test_hub/test_list_archive_files.py index 8b07ccd..5f05df1 100644 --- a/tests/test_hub/test_list_archive_files.py +++ b/tests/test_hub/test_list_archive_files.py @@ -42,18 +42,39 @@ class TestListArchiveFiles(unittest.TestCase): self.mm.get_image_build.assert_called_once_with(2) self.assertListEqual(rv, []) - def test_simple_strict(self): + @mock.patch('kojihub.get_maven_archive', + return_value={'archive_id': 1, + 'group_id': 'gid', + 'artifact_id': 'aid', + 'version': '1.0.0'}) + @mock.patch('kojihub._get_zipfile_list', return_value=[]) + def test_simple_strict_empty(self, get_zipfile_list, get_maven_archive): + self.mm.get_maven_build.return_value = {'build_id': 2, + 'group_id': 'gid', + 'artifact_id': 'aid', + 'version': '1.0.0'} + rv = kojihub.list_archive_files(1, strict=True) + self.assertListEqual(rv, []) + + def test_simple_strict_bad_btype(self): with self.assertRaises(koji.GenericError) as cm: kojihub.list_archive_files(1, strict=True) - self.mm.get_archive.assert_called_once_with(1, strict=True) - self.mm.get_archive_type.assert_called_once_with(type_id=3, - strict=True) - self.mm.get_build.assert_called_once_with(2, strict=True) - self.mm.get_maven_build.assert_called_once_with(2) - self.mm.get_win_build.assert_called_once_with(2) - self.mm.get_image_build.assert_called_once_with(2) - self.assertEqual(cm.exception.args[0], - "Archive#1 doesn't contain any files") + self.assertEqual(cm.exception.args[0], "Unsupported build type") + + @mock.patch('kojihub.get_maven_archive', + return_value={'archive_id': 1, + 'group_id': 'gid', + 'artifact_id': 'aid', + 'version': '1.0.0'}) + def test_simple_strict_bad_archive_type(self, get_maven_archive): + self.mm.get_archive_type.return_value = {'id': 9, 'name': 'txt'} + self.mm.get_maven_build.return_value = {'build_id': 2, + 'group_id': 'gid', + 'artifact_id': 'aid', + 'version': '1.0.0'} + with self.assertRaises(koji.GenericError) as cm: + kojihub.list_archive_files(1, strict=True) + self.assertEqual(cm.exception.args[0], "Unsupported archive type: txt") @mock.patch('kojihub.get_maven_archive', return_value={'archive_id': 1,