#3391 Use compression_type in listArchiveFiles
Merged a month ago by tkopecek. Opened 2 months ago by jcupova.
jcupova/koji issue-855  into  master

@@ -0,0 +1,13 @@ 

+ -- upgrade script to migrate the Koji database schema

+ -- from version 1.29 to 1.30

+ 

+ 

+ BEGIN;

+ 

+ ALTER TABLE archivetypes ADD COLUMN compression_type TEXT;

+ 

+ UPDATE archivetypes set compression_type='zip' WHERE name = 'jar';

+ UPDATE archivetypes set compression_type='zip' WHERE name = 'zip';

+ UPDATE archivetypes set compression_type='tar' WHERE name = 'tar';

+ 

+ COMMIT;

file modified
+5 -4
@@ -802,13 +802,14 @@ 

          id SERIAL NOT NULL PRIMARY KEY,

          name TEXT NOT NULL UNIQUE,

          description TEXT NOT NULL,

-         extensions TEXT NOT NULL

+         extensions TEXT NOT NULL,

+         compression_type TEXT

  ) WITHOUT OIDS;

  

- INSERT INTO archivetypes (name, description, extensions) VALUES ('jar', 'Jar file', 'jar war rar ear sar jdocbook jdocbook-style');

- INSERT INTO archivetypes (name, description, extensions) VALUES ('zip', 'Zip file', 'zip');

+ INSERT INTO archivetypes (name, description, extensions, compression_type) VALUES ('jar', 'Jar file', 'jar war rar ear sar jdocbook jdocbook-style', 'zip');

+ INSERT INTO archivetypes (name, description, extensions, compression_type) VALUES ('zip', 'Zip file', 'zip', 'zip');

  INSERT INTO archivetypes (name, description, extensions) VALUES ('pom', 'Maven Project Object Management file', 'pom');

- INSERT INTO archivetypes (name, description, extensions) VALUES ('tar', 'Tar file', 'tar tar.gz tar.bz2 tar.xz tgz');

+ INSERT INTO archivetypes (name, description, extensions, compression_type) VALUES ('tar', 'Tar file', 'tar tar.gz tar.bz2 tar.xz tgz', 'tar');

  INSERT INTO archivetypes (name, description, extensions) VALUES ('xml', 'XML file', 'xml');

  INSERT INTO archivetypes (name, description, extensions) VALUES ('xmlcompressed', 'Compressed XML file', 'xml.gz xml.bz2 xml.xz');

  INSERT INTO archivetypes (name, description, extensions) VALUES ('xsd', 'XML Schema Definition', 'xsd');

file modified
+20 -13
@@ -5033,6 +5033,7 @@ 

                ('archivetypes.name', 'type_name'),

                ('archivetypes.description', 'type_description'),

                ('archivetypes.extensions', 'type_extensions'),

+               ('archivetypes.compression_type', 'compression_type')

                ]

      clauses = []

  
@@ -5339,9 +5340,9 @@ 

          # should not happen

          raise koji.GenericError("Missing build type info for archive %s" % archive_id)

  

-     if archive_type['name'] in ('zip', 'jar'):

+     if archive_type['compression_type'] == 'zip':

          filelist = _get_zipfile_list(archive_id, file_path)

-     elif archive_type['name'] == 'tar':

+     elif archive_type['compression_type'] == 'tar':

          filelist = _get_tarball_list(archive_id, file_path)

      else:

          # TODO: support other archive types
@@ -7392,21 +7393,23 @@ 

  

  def get_archive_types():

      """Return a list of all supported archive types."""

-     select = """SELECT id, name, description, extensions FROM archivetypes

+     select = """SELECT id, name, description, extensions, compression_type FROM archivetypes

      ORDER BY id"""

-     return _multiRow(select, {}, ('id', 'name', 'description', 'extensions'))

+     return _multiRow(select, {}, ('id', 'name', 'description', 'extensions', 'compression_type'))

  

  

  def _get_archive_type_by_name(name, strict=True):

-     select = """SELECT id, name, description, extensions FROM archivetypes

+     select = """SELECT id, name, description, extensions, compression_type FROM archivetypes

      WHERE name = %(name)s"""

-     return _singleRow(select, locals(), ('id', 'name', 'description', 'extensions'), strict)

+     return _singleRow(select, locals(),

+                       ('id', 'name', 'description', 'extensions', 'compression_type'), strict)

  

  

  def _get_archive_type_by_id(type_id, strict=False):

-     select = """SELECT id, name, description, extensions FROM archivetypes

+     select = """SELECT id, name, description, extensions, compression_type FROM archivetypes

      WHERE id = %(type_id)i"""

-     return _singleRow(select, locals(), ('id', 'name', 'description', 'extensions'), strict)

+     return _singleRow(select, locals(),

+                       ('id', 'name', 'description', 'extensions', 'compression_type'), strict)

  

  

  def get_archive_type(filename=None, type_name=None, type_id=None, strict=False):
@@ -7425,7 +7428,7 @@ 

      parts = filename.split('.')

      query = QueryProcessor(

          tables=['archivetypes'],

-         columns=['id', 'name', 'description', 'extensions'],

+         columns=['id', 'name', 'description', 'extensions', 'compression_type'],

          clauses=['extensions ~* %(pattern)s'],

      )

      for start in range(len(parts) - 1, -1, -1):
@@ -7445,7 +7448,7 @@ 

          return None

  

  

- def add_archive_type(name, description, extensions):

+ def add_archive_type(name, description, extensions, compression_type=None):

      """

      Add new archive type.

  
@@ -7460,21 +7463,25 @@ 

      verify_name_internal(name)

      convert_value(description, cast=str, check_only=True)

      convert_value(extensions, cast=str, check_only=True)

+     convert_value(compression_type, cast=str, none_allowed=True, check_only=True)

+     if compression_type not in ['zip', 'tar', None]:

+         raise koji.GenericError(f"Unsupported compression type {compression_type}")

      data = {'name': name,

              'description': description,

              'extensions': extensions,

+             'compression_type': compression_type,

              }

      if get_archive_type(type_name=name):

-         raise koji.GenericError("archivetype %s already exists" % name)

+         raise koji.GenericError(f"archivetype {name} already exists")

      # No invalid or duplicate extensions

      for ext in extensions.split(' '):

          if not ext.replace('.', '').isalnum():

-             raise koji.GenericError('No such %s file extension' % ext)

+             raise koji.GenericError(f'No such {ext} file extension')

          select = r"""SELECT id FROM archivetypes

                        WHERE extensions ~* E'(\\s|^)%s(\\s|$)'""" % ext

          results = _multiRow(select, {}, ('id',))

          if len(results) > 0:

-             raise koji.GenericError('file extension %s already exists' % ext)

+             raise koji.GenericError(f'file extension {ext} already exists')

      insert = InsertProcessor('archivetypes', data=data)

      insert.execute()

  

@@ -34,7 +34,7 @@ 

          self.inserts.append(insert)

          return insert

  

-     def test_add_archive_type_valid(self):

+     def test_add_archive_type_valid_empty_compression_type(self):

          self.verify_name_internal.return_value = None

          self.get_archive_type.return_value = None

          kojihub.add_archive_type('deb', 'Debian package', 'deb')
@@ -44,7 +44,23 @@ 

          self.assertEqual(insert.table, 'archivetypes')

          self.assertEqual(insert.data, {'name': 'deb',

                                         'description': 'Debian package',

-                                        'extensions': 'deb'})

+                                        'extensions': 'deb',

+                                        'compression_type': None})

+         self.assertEqual(insert.rawdata, {})

+         self.context.session.assertPerm.assert_called_with('admin')

+ 

+     def test_add_archive_type_valid_with_compression_type(self):

+         self.verify_name_internal.return_value = None

+         self.get_archive_type.return_value = None

+         kojihub.add_archive_type('jar', 'Jar package', 'jar', 'zip')

+ 

+         self.assertEqual(len(self.inserts), 1)

+         insert = self.inserts[0]

+         self.assertEqual(insert.table, 'archivetypes')

+         self.assertEqual(insert.data, {'name': 'jar',

+                                        'description': 'Jar package',

+                                        'extensions': 'jar',

+                                        'compression_type': 'zip'})

          self.assertEqual(insert.rawdata, {})

          self.context.session.assertPerm.assert_called_with('admin')

  

@@ -6,7 +6,7 @@ 

  

  GET_ARCHIVE_RV = {'id': 1, 'build_id': 2, 'type_id': 3, 'btype': 'btype',

                    'filename': 'somearchive.zip'}

- GET_ARCHIVE_TYPE_RV = {'id': 3, 'name': 'zip'}

+ GET_ARCHIVE_TYPE_RV = {'id': 3, 'name': 'zip', 'compression_type': 'zip'}

  GET_BUILD_RV = {'id': 2, 'name': 'somebuild', 'version': '1.2.3',

                  'release': '1.el6', 'volume_name': 'archive_vol'}

  
@@ -22,119 +22,165 @@ 

                                                return_value=GET_ARCHIVE_TYPE_RV.copy()).start()

          self.mm.get_archive = mock.patch('kojihub.get_archive',

                                           return_value=GET_ARCHIVE_RV.copy()).start()

+         self.mm.get_maven_archive = mock.patch('kojihub.get_maven_archive').start()

+         self.mm.get_win_archive = mock.patch('kojihub.get_win_archive').start()

+         self.mm.get_image_archive = mock.patch('kojihub.get_image_archive').start()

+         self.mm.get_zipfile_list = mock.patch('kojihub._get_zipfile_list').start()

+         self.mm.get_tarball_list = mock.patch('kojihub._get_tarball_list').start()

+         self.maven_archive = {'archive_id': 1, 'group_id': 'gid', 'artifact_id': 'aid',

+                               'version': '1.0.0'}

+         self.win_archive = {'archive_id': 1, 'relpath': 'rpath', 'platform': 'all',

+                             'version': 'src'}

+         self.image_archive = {'archive_id': 1, 'arch': 'noarch'}

+         self.zipfile_list = [

+             {'archive_id': 1, 'name': 'file1', 'size': 4096, 'mtime': 1000},

+             {'archive_id': 1, 'name': 'file2', 'size': 512000, 'mtime': 103420},

+         ]

+         self.tarball_list = [

+             {'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}

+         ]

+         self.archive_id = 1

  

      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_zipfile_list.return_value = []

+         rv = kojihub.list_archive_files(self.archive_id)

+         self.mm.get_archive.assert_called_once_with(self.archive_id, 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_zipfile_list.assert_called_once_with(

+             1,

+             '/mnt/koji/vol/archive_vol/packages/somebuild/1.2.3/1.el6/files/btype/somearchive.zip')

+         self.mm.get_maven_archive.assert_not_called()

+         self.mm.get_win_archive.assert_not_called()

+         self.mm.get_image_archive.assert_not_called()

+         self.mm.get_tarball_list.assert_not_called()

          self.assertListEqual(rv, [])

  

-     @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):

-         rv = kojihub.list_archive_files(1, strict=True)

+     def test_simple_strict_empty(self):

+         self.mm.get_zipfile_list.return_value = []

+         rv = kojihub.list_archive_files(self.archive_id, strict=True)

+         self.mm.get_archive.assert_called_once_with(self.archive_id, 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_zipfile_list.assert_called_once_with(

+             1,

+             '/mnt/koji/vol/archive_vol/packages/somebuild/1.2.3/1.el6/files/btype/somearchive.zip')

+         self.mm.get_maven_archive.assert_not_called()

+         self.mm.get_win_archive.assert_not_called()

+         self.mm.get_image_archive.assert_not_called()

+         self.mm.get_tarball_list.assert_not_called()

          self.assertListEqual(rv, [])

  

      def test_simple_strict_missing_btype(self):

          self.mm.get_archive.return_value['btype'] = None

          with self.assertRaises(koji.GenericError) as cm:

-             kojihub.list_archive_files(1, strict=True)

-         self.assertEqual(cm.exception.args[0][:18], "Missing build type")

+             kojihub.list_archive_files(self.archive_id, strict=True)

+         self.assertEqual(cm.exception.args[0],

+                          f'Missing build type info for archive {self.archive_id}')

+         self.mm.get_archive.assert_called_once_with(self.archive_id, 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_archive.assert_not_called()

+         self.mm.get_win_archive.assert_not_called()

+         self.mm.get_image_archive.assert_not_called()

+         self.mm.get_zipfile_list.assert_not_called()

+         self.mm.get_tarball_list.assert_not_called()

  

-     @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'}

+     def test_simple_strict_bad_archive_type_with_strict(self):

+         self.mm.get_archive_type.return_value = {'id': 9, 'name': 'txt', 'compression_type': ''}

          with self.assertRaises(koji.GenericError) as cm:

-             kojihub.list_archive_files(1, strict=True)

+             kojihub.list_archive_files(self.archive_id, strict=True)

          self.assertEqual(cm.exception.args[0], "Unsupported archive type: txt")

+         self.mm.get_archive.assert_called_once_with(self.archive_id, 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_archive.assert_not_called()

+         self.mm.get_win_archive.assert_not_called()

+         self.mm.get_image_archive.assert_not_called()

+         self.mm.get_zipfile_list.assert_not_called()

+         self.mm.get_tarball_list.assert_not_called()

  

-     @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):

+     def test_simple_strict_bad_archive_type_without_strict(self):

+         self.mm.get_archive_type.return_value = {'id': 9, 'name': 'txt', 'compression_type': ''}

+         rv = kojihub.list_archive_files(1)

+         self.mm.get_archive.assert_called_once_with(self.archive_id, 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_win_archive.assert_not_called()

+         self.mm.get_image_archive.assert_not_called()

+         self.mm.get_zipfile_list.assert_not_called()

+         self.mm.get_tarball_list.assert_not_called()

+         self.mm.get_maven_archive.assert_not_called()

+         self.assertListEqual(rv, [])

+ 

+     def test_maven_archive(self):

+         self.mm.get_maven_archive.return_value = self.maven_archive

+         self.mm.get_zipfile_list.return_value = self.zipfile_list

          self.mm.get_archive.return_value['btype'] = 'maven'

          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.mm.get_maven_archive.assert_called_once_with(1, strict=True)

+         self.mm.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.mm.get_archive.assert_called_once_with(self.archive_id, 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_win_archive.assert_not_called()

+         self.mm.get_image_archive.assert_not_called()

          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):

+     def test_win_archive(self):

+         self.mm.get_win_archive.return_value = self.win_archive

+         self.mm.get_zipfile_list.return_value = self.zipfile_list

          self.mm.get_archive.return_value['btype'] = 'win'

          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.mm.get_win_archive.assert_called_once_with(1, strict=True)

+         self.mm.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.mm.get_archive.assert_called_once_with(self.archive_id, 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_image_archive.assert_not_called()

+         self.mm.get_tarball_list.assert_not_called()

+         self.mm.get_maven_archive.assert_not_called()

          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'}

+     def test_image_archive(self):

+         self.mm.get_image_archive.return_value = self.image_archive

+         self.mm.get_tarball_list.return_value = self.tarball_list

+         self.mm.get_archive_type.return_value = {'id': 3, 'name': 'tar', 'compression_type': 'tar'}

          self.mm.get_archive.return_value['btype'] = 'image'

          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.mm.get_image_archive.assert_called_once_with(1, strict=True)

+         self.mm.get_tarball_list.assert_called_once_with(

+             1,

+             '/mnt/koji/vol/archive_vol/packages/somebuild/1.2.3/1.el6/images/somearchive.zip')

+         self.mm.get_archive.assert_called_once_with(self.archive_id, 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_win_archive.assert_not_called()

+         self.mm.get_zipfile_list.assert_not_called()

+         self.mm.get_maven_archive.assert_not_called()

          self.assertEqual(rv, 2)

to accomplish this functionality, it's better to let addArchiveType support adding archive_type with compression_type.
And the values of compression_type are limited to 'tar', 'zip' by implementation so far, so we could check the values in addArchiveType too. On the other hand, we can just ignore invalid/not-implemented values. What do you think?

rebased onto dcac7e25e3ac6b20b49a45b1cd537bdc917191b6

2 months ago

sure, it make sense to check compression_type in addArchiveType. All fixed in PR now.

I think None is better than '' here, so the default value of compression_type can be None.

And this part could be

    convert_value(compression_type, cast=str, none_allowed=True, check_only=True)
    if compression_type not in ['zip', 'tar', None]:
        ...

rebased onto 512c9fb6866087d54926503ff7161c1474de015e

2 months ago

ok, when we want to use None, yes, we can do it like you wrote...so, updated with None value.

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 months ago

rebased onto eddfbf6

2 months ago

Metadata Update from @mfilip:
- Pull-request tagged with: testing-done

a month ago

Commit 576d04e fixes this pull-request

Pull-Request has been merged by tkopecek

a month ago