#2736 api: createMavenBuild wrong buildinfo/maveninfo
Merged a year ago by tkopecek. Opened a year ago by jcupova.
jcupova/koji issue1104  into  master

file modified
+30 -5
@@ -7132,6 +7132,8 @@ 

      maven_info must contain the 'group_id',

      'artifact_id', and 'version' keys.

      """

+     if not isinstance(maven_info, dict):

+         raise koji.GenericError('Invalid type for maven_info: %s' % type(maven_info))

      maven_info = maven_info.copy()

  

      current_maven_info = get_maven_build(build)
@@ -7142,8 +7144,13 @@ 

                  raise koji.BuildError('%s mismatch (current: %s, new: %s)' %

                                        (field, current_maven_info[field], maven_info[field]))

      else:

+         if maven_info == {}:

+             raise koji.GenericError("Maven info is empty")

          maven_info['build_id'] = build['id']

-         data = dslice(maven_info, ['build_id', 'group_id', 'artifact_id', 'version'])

+         try:

+             data = dslice(maven_info, ['build_id', 'group_id', 'artifact_id', 'version'])

+         except KeyError as cm:

+             raise koji.GenericError("Maven info doesn't have mandatory %s key" % cm)

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

          insert.execute()

          # also add build_types entry
@@ -10724,16 +10731,34 @@ 

  

      def createMavenBuild(self, build_info, maven_info):

          """

-         Associate Maven metadata with an existing build.  The build must

-         not already have associated Maven metadata.  maven_info must be a dict

-         containing group_id, artifact_id, and version entries.

+         Associate Maven metadata with an existing build. When build isn`t existing, creates a

+         build. The build must not already have associated Maven metadata.  maven_info must be

+         a dict containing group_id, artifact_id, and version entries.

+ 

+         :param build_info: a str (build name) if build is existing

+                            or a dict:

+                                - name: build name

+                                - version: build version

+                                - release: build release

+                                - epoch: build epoch

+         :param dict maven_info:

+                                - group_id: Group's ID

+                                - artifact_id: Artifact's ID

+                                - version: version

+         :raises: GenericError if type for build_info is not dict, when build isn`t existing.

+         :raises: GenericError if build info doesn't have mandatory keys.

          """

          context.session.assertPerm('maven-import')

          if not context.opts.get('EnableMaven'):

              raise koji.GenericError("Maven support not enabled")

          build = get_build(build_info)

          if not build:

-             build_id = new_build(dslice(build_info, ('name', 'version', 'release', 'epoch')))

+             if not isinstance(build_info, dict):

+                 raise koji.GenericError('Invalid type for build_info: %s' % type(build_info))

+             try:

+                 build_id = new_build(dslice(build_info, ('name', 'version', 'release', 'epoch')))

+             except KeyError as cm:

+                 raise koji.GenericError("Build info doesn't have mandatory %s key" % cm)

              build = get_build(build_id, strict=True)

          new_maven_build(build, maven_info)

  

@@ -0,0 +1,160 @@ 

+ import unittest

+ 

+ import mock

+ 

+ import koji

+ import kojihub

+ 

+ IP = kojihub.InsertProcessor

+ 

+ 

+ class TestCreateMavenBuild(unittest.TestCase):

+ 

+     def setUp(self):

+         self.get_build = mock.patch('kojihub.get_build').start()

+         self.exports = kojihub.RootExports()

+         self.session = mock.MagicMock()

+         self.context = mock.patch('kojihub.context').start()

+         self.context.session.assertPerm = mock.MagicMock()

+         self.InsertProcessor = mock.patch('kojihub.InsertProcessor',

+                                           side_effect=self.getInsert).start()

+         self.inserts = []

+         self.insert_execute = mock.MagicMock()

+         self.get_build_info = {

+             'id': 100,

+             'name': 'test_name',

+             'version': 'test_version',

+             'release': 'test_release',

+             'epoch': 'test_epoch',

+             'owner': 'test_owner',

+             'build_id': 2,

+         }

+         self.build_info = 'test-build-11-12'

+ 

+     def getInsert(self, *args, **kwargs):

+         insert = IP(*args, **kwargs)

+         insert.execute = self.insert_execute

+         self.inserts.append(insert)

+         return insert

+ 

+     def test_maven_info_empty_dict(self):

+         maven_info = {}

+         self.get_build.return_value = self.get_build_info

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

+             self.exports.createMavenBuild(self.build_info, maven_info)

+         self.assertEqual("Maven info is empty", str(cm.exception))

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

+ 

+     def test_maven_info_without_some_key(self):

+         # maven_info without group_id

+         maven_info = {

+             'artifact_id': '99',

+             'version': '33'

+         }

+         self.get_build.return_value = self.get_build_info

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

+             self.exports.createMavenBuild(self.build_info, maven_info)

+         self.assertEqual("Maven info doesn't have mandatory 'group_id' key", str(cm.exception))

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

+ 

+         # maven_info without artifact_id

+         maven_info = {

+             'group_id': '11',

+             'version': '33'

+         }

+         self.get_build.return_value = self.get_build_info

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

+             self.exports.createMavenBuild(self.build_info, maven_info)

+         self.assertEqual("Maven info doesn't have mandatory 'artifact_id' key", str(cm.exception))

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

+ 

+         # maven_info without version

+         maven_info = {

+             'group_id': '11',

+             'artifact_id': '99',

+         }

+         self.get_build.return_value = self.get_build_info

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

+             self.exports.createMavenBuild(self.build_info, maven_info)

+         self.assertEqual("Maven info doesn't have mandatory 'version' key", str(cm.exception))

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

+ 

+     def test_empty_wrong_format_maven_info(self):

+         maven_info = 'maven-wrong-info'

+         self.get_build.return_value = self.get_build_info

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

+             self.exports.createMavenBuild(self.build_info, maven_info)

+         self.assertEqual('Invalid type for maven_info: %s' % type(maven_info), str(cm.exception))

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

+ 

+     def test_empty_wrong_format_non_exist_build_info(self):

+         maven_info = {

+             'group_id': '11',

+             'artifact_id': '99',

+             'version': '33'

+         }

+         self.get_build.return_value = None

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

+             self.exports.createMavenBuild(self.build_info, maven_info)

+         self.assertEqual(

+             'Invalid type for build_info: %s' % type(self.build_info), str(cm.exception))

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

+ 

+     def test_build_info_without_some_mandatory_key(self):

+         maven_info = {

+             'group_id': '11',

+             'artifact_id': '99',

+             'version': '33'

+         }

+ 

+         # build_info without name

+         get_build_info = {

+             'id': 100,

+             'version': 'test_version',

+             'release': 'test_release',

+             'epoch': 'test_epoch',

+         }

+         self.get_build.return_value = None

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

+             self.exports.createMavenBuild(get_build_info, maven_info)

+         self.assertEqual("Build info doesn't have mandatory 'name' key", str(cm.exception))

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

+ 

+         # build_info without version

+         get_build_info = {

+             'id': 100,

+             'name': 'test_name',

+             'release': 'test_release',

+             'epoch': 'test_epoch',

+         }

+         self.get_build.return_value = None

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

+             self.exports.createMavenBuild(get_build_info, maven_info)

+         self.assertEqual("Build info doesn't have mandatory 'version' key", str(cm.exception))

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

+ 

+         # build_info without release

+         get_build_info = {

+             'id': 100,

+             'name': 'test_name',

+             'version': 'test_version',

+             'epoch': 'test_epoch',

+         }

+         self.get_build.return_value = None

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

+             self.exports.createMavenBuild(get_build_info, maven_info)

+         self.assertEqual("Build info doesn't have mandatory 'release' key", str(cm.exception))

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

+ 

+         # build_info without epoch

+         get_build_info = {

+             'id': 100,

+             'name': 'test_name',

+             'version': 'test_version',

+             'release': 'test_release',

+         }

+         self.get_build.return_value = None

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

+             self.exports.createMavenBuild(get_build_info, maven_info)

+         self.assertEqual("Build info doesn't have mandatory 'epoch' key", str(cm.exception))

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

@@ -0,0 +1,78 @@ 

+ import unittest

+ 

+ import mock

+ 

+ import koji

+ import kojihub

+ 

+ IP = kojihub.InsertProcessor

+ 

+ 

+ class TestNewMavenBuild(unittest.TestCase):

+     def setUp(self):

+         self.InsertProcessor = mock.patch('kojihub.InsertProcessor',

+                                           side_effect=self.getInsert).start()

+         self.inserts = []

+         self.insert_execute = mock.MagicMock()

+         self.get_maven_build = mock.patch('kojihub.get_maven_build').start()

+         self.get_maven_build.return_value = None

+         self.build_info = {

+             'id': 100,

+             'name': 'test_name',

+             'version': 'test_version',

+             'release': 'test_release',

+             'epoch': 'test_epoch',

+             'owner': 'test_owner',

+             'build_id': 2,

+         }

+ 

+     def getInsert(self, *args, **kwargs):

+         insert = IP(*args, **kwargs)

+         insert.execute = self.insert_execute

+         self.inserts.append(insert)

+         return insert

+ 

+     def test_empty_maven_info(self):

+         maven_info = {}

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

+             kojihub.new_maven_build(self.build_info, maven_info)

+         self.assertEqual("Maven info is empty", str(cm.exception))

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

+ 

+     def test_maven_info_without_some_key(self):

+         # maven_info without group_id

+         maven_info = {

+             'artifact_id': '99',

+             'version': '33'

+         }

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

+             kojihub.new_maven_build(self.build_info, maven_info)

+         self.assertEqual("Maven info doesn't have mandatory 'group_id' key", str(cm.exception))

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

+ 

+         # maven_info without artifact_id

+         maven_info = {

+             'group_id': '11',

+             'version': '33'

+         }

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

+             kojihub.new_maven_build(self.build_info, maven_info)

+         self.assertEqual("Maven info doesn't have mandatory 'artifact_id' key", str(cm.exception))

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

+ 

+         # maven_info without version

+         maven_info = {

+             'group_id': '11',

+             'artifact_id': '99',

+         }

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

+             kojihub.new_maven_build(self.build_info, maven_info)

+         self.assertEqual("Maven info doesn't have mandatory 'version' key", str(cm.exception))

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

+ 

+     def test_wrong_format_maven_info(self):

+         maven_info = 'test-maven-info'

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

+             kojihub.new_maven_build(self.build_info, maven_info)

+         self.assertEqual('Invalid type for maven_info: %s' % type(maven_info), str(cm.exception))

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

This could be simplied to:

try:
   data = dslice...
except KeyError:
  raise koji.GenericError

Little clarification - your code almost duplicates what dslice(strict=True) (which is True by default) does.

rebased onto 9796f782b333284e15295d6b4a011f166cb8c82f

a year ago

rebased onto f7d1aa8b7b1d0e1487d9eca031bb303502c30025

a year ago

@tkopecek fixed and added the same try-catch for dslice in the createMavenBuild for build info

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

a year ago

rebased onto ee656631cca84e318eeca575ba0722ee210f7610

a year ago

rebased onto 748eadea9502ae5eda2e80ffa2b520811e2d430e

a year ago

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

a year ago

Simple typo here preventing merge (trailing whitespace)

rebased onto 330f85a

a year ago

Commit f6ca9aa fixes this pull-request

Pull-Request has been merged by tkopecek

a year ago