PR#3484: Use nextval function instead of query 'SELECT nextval'

Merges #3484
https://pagure.io/koji/pull-request/3484

Fixes: #3483
https://pagure.io/koji/issue/3483
use nextval() instead of SQL
This commit is contained in:
Tomas Kopecek 2022-09-05 10:54:03 +02:00
commit 93ca7ff10b
4 changed files with 19 additions and 24 deletions

View file

@ -2481,7 +2481,7 @@ def add_channel(channel_name, description=None):
if dup_check: if dup_check:
raise koji.GenericError("channel %(name)s already exists (id=%(id)i)" % dup_check) raise koji.GenericError("channel %(name)s already exists (id=%(id)i)" % dup_check)
table = 'channels' table = 'channels'
channel_id = _singleValue("SELECT nextval('%s_id_seq')" % table, strict=True) channel_id = nextval(f'{table}_id_seq')
insert = InsertProcessor(table) insert = InsertProcessor(table)
insert.set(id=channel_id, name=channel_name, description=description) insert.set(id=channel_id, name=channel_name, description=description)
insert.execute() insert.execute()
@ -2699,7 +2699,7 @@ def repo_init(tag, task_id=None, with_src=False, with_debuginfo=False, event=Non
if arch in ['src', 'noarch']: if arch in ['src', 'noarch']:
continue continue
repo_arches[arch] = 1 repo_arches[arch] = 1
repo_id = _singleValue("SELECT nextval('repo_id_seq')") repo_id = nextval('repo_id_seq')
if event is None: if event is None:
event_id = _singleValue("SELECT get_event()") event_id = _singleValue("SELECT get_event()")
else: else:
@ -5810,9 +5810,7 @@ def new_package(name, strict=True):
if strict: if strict:
raise koji.GenericError("Package already exists [id %d]" % pkg_id) raise koji.GenericError("Package already exists [id %d]" % pkg_id)
else: else:
q = """SELECT nextval('package_id_seq')""" pkg_id = nextval('package_id_seq')
c.execute(q)
(pkg_id,) = c.fetchone()
q = """INSERT INTO package (id,name) VALUES (%(pkg_id)s,%(name)s)""" q = """INSERT INTO package (id,name) VALUES (%(pkg_id)s,%(name)s)"""
context.commit_pending = True context.commit_pending = True
c.execute(q, locals()) c.execute(q, locals())
@ -6098,7 +6096,7 @@ def new_build(data, strict=False):
'extra']) 'extra'])
if 'cg_id' in data: if 'cg_id' in data:
insert_data['cg_id'] = data['cg_id'] insert_data['cg_id'] = data['cg_id']
data['id'] = insert_data['id'] = _singleValue("SELECT nextval('build_id_seq')") data['id'] = insert_data['id'] = nextval('build_id_seq')
insert = InsertProcessor('build', data=insert_data) insert = InsertProcessor('build', data=insert_data)
insert.execute() insert.execute()
new_binfo = get_build(data['id'], strict=True) new_binfo = get_build(data['id'], strict=True)
@ -6384,7 +6382,7 @@ def import_rpm(fn, buildinfo=None, brootid=None, wrapper=False, fileinfo=None):
new_typed_build(buildinfo, 'rpm') new_typed_build(buildinfo, 'rpm')
# add rpminfo entry # add rpminfo entry
rpminfo['id'] = _singleValue("""SELECT nextval('rpminfo_id_seq')""") rpminfo['id'] = nextval('rpminfo_id_seq')
rpminfo['build_id'] = buildinfo['id'] rpminfo['build_id'] = buildinfo['id']
rpminfo['size'] = os.path.getsize(fn) rpminfo['size'] = os.path.getsize(fn)
rpminfo['payloadhash'] = koji.hex_string(koji.get_header_field(hdr, 'sigmd5')) rpminfo['payloadhash'] = koji.hex_string(koji.get_header_field(hdr, 'sigmd5'))
@ -13527,7 +13525,7 @@ class RootExports(object):
userID = context.session.createUser(hostname, usertype=koji.USERTYPES['HOST'], userID = context.session.createUser(hostname, usertype=koji.USERTYPES['HOST'],
krb_principal=krb_principal) krb_principal=krb_principal)
# host entry # host entry
hostID = _singleValue("SELECT nextval('host_id_seq')", strict=True) hostID = nextval('host_id_seq')
insert = "INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, " \ insert = "INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, " \
"%(hostname)s)" "%(hostname)s)"
_dml(insert, dslice(locals(), ('hostID', 'userID', 'hostname'))) _dml(insert, dslice(locals(), ('hostID', 'userID', 'hostname')))
@ -14226,7 +14224,7 @@ class BuildRoot(object):
def new(self, host, repo, arch, task_id=None, ctype='chroot'): def new(self, host, repo, arch, task_id=None, ctype='chroot'):
arch = koji.parse_arches(arch, strict=True, allow_none=True) arch = koji.parse_arches(arch, strict=True, allow_none=True)
state = koji.BR_STATES['INIT'] state = koji.BR_STATES['INIT']
br_id = _singleValue("SELECT nextval('buildroot_id_seq')", strict=True) br_id = nextval('buildroot_id_seq')
insert = InsertProcessor('buildroot', data={'id': br_id}) insert = InsertProcessor('buildroot', data={'id': br_id})
insert.set(container_arch=arch, container_type=ctype) insert.set(container_arch=arch, container_type=ctype)
insert.set(br_type=koji.BR_TYPES['STANDARD']) insert.set(br_type=koji.BR_TYPES['STANDARD'])
@ -14259,7 +14257,7 @@ class BuildRoot(object):
raise koji.GenericError("Buildroot field %s not specified" % key) raise koji.GenericError("Buildroot field %s not specified" % key)
if data['extra'] is not None: if data['extra'] is not None:
data['extra'] = json.dumps(data['extra']), data['extra'] = json.dumps(data['extra']),
br_id = _singleValue("SELECT nextval('buildroot_id_seq')", strict=True) br_id = nextval('buildroot_id_seq')
insert = InsertProcessor('buildroot') insert = InsertProcessor('buildroot')
insert.set(id=br_id, **data) insert.set(id=br_id, **data)
insert.execute() insert.execute()

View file

@ -24,7 +24,7 @@ class TestAddChannel(unittest.TestCase):
self.insert_execute = mock.MagicMock() self.insert_execute = mock.MagicMock()
self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start() self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start()
self.get_channel = mock.patch('kojihub.get_channel').start() self.get_channel = mock.patch('kojihub.get_channel').start()
self._singleValue = mock.patch('kojihub._singleValue').start() self.nextval = mock.patch('kojihub.nextval').start()
def tearDown(self): def tearDown(self):
mock.patch.stopall() mock.patch.stopall()
@ -41,12 +41,12 @@ class TestAddChannel(unittest.TestCase):
with self.assertRaises(koji.GenericError): with self.assertRaises(koji.GenericError):
self.exports.addChannel(self.channel_name) self.exports.addChannel(self.channel_name)
self.get_channel.assert_called_once_with(self.channel_name, strict=False) self.get_channel.assert_called_once_with(self.channel_name, strict=False)
self._singleValue.assert_not_called() self.nextval.assert_not_called()
self.assertEqual(len(self.inserts), 0) self.assertEqual(len(self.inserts), 0)
def test_add_channel_valid(self): def test_add_channel_valid(self):
self.get_channel.return_value = {} self.get_channel.return_value = {}
self._singleValue.side_effect = [12] self.nextval.side_effect = [12]
self.verify_name_internal.return_value = None self.verify_name_internal.return_value = None
r = self.exports.addChannel(self.channel_name, description=self.description) r = self.exports.addChannel(self.channel_name, description=self.description)
@ -60,10 +60,8 @@ class TestAddChannel(unittest.TestCase):
self.context.session.assertPerm.assert_called_once_with('admin') self.context.session.assertPerm.assert_called_once_with('admin')
self.get_channel.assert_called_once_with(self.channel_name, strict=False) self.get_channel.assert_called_once_with(self.channel_name, strict=False)
self.assertEqual(self._singleValue.call_count, 1) self.assertEqual(self.nextval.call_count, 1)
self._singleValue.assert_has_calls([ self.nextval.assert_called_once_with('channels_id_seq')
mock.call("SELECT nextval('channels_id_seq')", strict=True)
])
def test_add_channel_wrong_name(self): def test_add_channel_wrong_name(self):
# name is longer as expected # name is longer as expected

View file

@ -41,6 +41,7 @@ class TestAddHost(unittest.TestCase):
self._dml = mock.patch('kojihub._dml').start() self._dml = mock.patch('kojihub._dml').start()
self.get_host = mock.patch('kojihub.get_host').start() self.get_host = mock.patch('kojihub.get_host').start()
self._singleValue = mock.patch('kojihub._singleValue').start() self._singleValue = mock.patch('kojihub._singleValue').start()
self.nextval = mock.patch('kojihub.nextval').start()
self.get_user = mock.patch('kojihub.get_user').start() self.get_user = mock.patch('kojihub.get_user').start()
def tearDown(self): def tearDown(self):
@ -58,7 +59,8 @@ class TestAddHost(unittest.TestCase):
def test_add_host_valid(self): def test_add_host_valid(self):
self.verify_host_name.return_value = None self.verify_host_name.return_value = None
self.get_host.return_value = {} self.get_host.return_value = {}
self._singleValue.side_effect = [333, 12] self._singleValue.return_value = 333
self.nextval.return_value = 12
self.context.session.createUser.return_value = 456 self.context.session.createUser.return_value = 456
self.get_user.return_value = None self.get_user.return_value = None
@ -69,11 +71,8 @@ class TestAddHost(unittest.TestCase):
kojihub.get_host.assert_called_once_with('hostname') kojihub.get_host.assert_called_once_with('hostname')
self.context.session.createUser.assert_called_once_with( self.context.session.createUser.assert_called_once_with(
'hostname', usertype=koji.USERTYPES['HOST'], krb_principal='-hostname-') 'hostname', usertype=koji.USERTYPES['HOST'], krb_principal='-hostname-')
self.assertEqual(self._singleValue.call_count, 2) self._singleValue.assert_called_once_with("SELECT id FROM channels WHERE name = 'default'")
self._singleValue.assert_has_calls([ self.nextval.assert_called_once_with('host_id_seq')
mock.call("SELECT id FROM channels WHERE name = 'default'"),
mock.call("SELECT nextval('host_id_seq')", strict=True)
])
self.assertEqual(self._dml.call_count, 1) self.assertEqual(self._dml.call_count, 1)
self._dml.assert_called_once_with("INSERT INTO host (id, user_id, name) " self._dml.assert_called_once_with("INSERT INTO host (id, user_id, name) "
"VALUES (%(hostID)i, %(userID)i, %(hostname)s)", "VALUES (%(hostID)i, %(userID)i, %(hostname)s)",

View file

@ -36,7 +36,7 @@ class TestNewBuild(unittest.TestCase):
def test_valid(self): def test_valid(self):
self.get_build.return_value = None self.get_build.return_value = None
self._singleValue.return_value = 65 # free build id self.nextval.return_value = 65 # free build id
self.new_package.return_value = 54 self.new_package.return_value = 54
self.get_user.return_value = {'id': 123} self.get_user.return_value = {'id': 123}
data = { data = {