PR#1473: move tag/package owners to separate table

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

Fixes: #956
https://pagure.io/koji/issue/956
package ownership changes should not trigger repo regens
This commit is contained in:
Tomas Kopecek 2019-08-29 15:44:28 +02:00
commit 1e35e60c21
4 changed files with 123 additions and 18 deletions

View file

@ -4200,6 +4200,13 @@ def _print_histline(entry, **kwargs):
fmt = "package list entry created: %(package.name)s in %(tag.name)s"
else:
fmt = "package list entry revoked: %(package.name)s in %(tag.name)s"
elif table == 'tag_package_owners':
if edit:
fmt = "package owner changed for %(package.name)s in %(tag.name)s"
elif create:
fmt = "package owner %(owner.name)s set for %(package.name)s in %(tag.name)s"
else:
fmt = "package owner %(owner.name)s revoked for %(package.name)s in %(tag.name)s"
elif table == 'tag_inheritance':
if edit:
fmt = "inheritance line %(tag.name)s->%(parent.name)s updated"
@ -4352,6 +4359,7 @@ _table_keys = {
'tag_external_repos' : ['tag_id', 'external_repo_id'],
'tag_listing' : ['build_id', 'tag_id'],
'tag_packages' : ['package_id', 'tag_id'],
'tag_package_owners' : ['package_id', 'tag_id'],
'group_config' : ['group_id', 'tag_id'],
'group_req_listing' : ['group_id', 'tag_id', 'req_id'],
'group_package_listing' : ['group_id', 'tag_id', 'package'],

View file

@ -4,5 +4,55 @@
BEGIN;
CREATE TABLE tag_package_owners (
package_id INTEGER NOT NULL REFERENCES package(id),
tag_id INTEGER NOT NULL REFERENCES tag (id),
owner INTEGER NOT NULL REFERENCES users(id),
-- versioned - see earlier description of versioning
create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(),
revoke_event INTEGER REFERENCES events(id),
creator_id INTEGER NOT NULL REFERENCES users(id),
revoker_id INTEGER REFERENCES users(id),
active BOOLEAN DEFAULT 'true' CHECK (active),
CONSTRAINT active_revoke_sane CHECK (
(active IS NULL AND revoke_event IS NOT NULL AND revoker_id IS NOT NULL)
OR (active IS NOT NULL AND revoke_event IS NULL AND revoker_id IS NULL)),
PRIMARY KEY (create_event, package_id, tag_id),
UNIQUE (package_id,tag_id,active)
) WITHOUT OIDS;
CREATE OR REPLACE FUNCTION convert_owners() RETURNS SETOF tag_packages AS
$BODY$
DECLARE
r tag_packages%rowtype;
r2 tag_packages%rowtype;
last_owner int;
BEGIN
FOR r IN SELECT package_id, tag_id FROM tag_packages GROUP BY package_id, tag_id ORDER BY package_id, tag_id
LOOP
last_owner := 0;
FOR r2 IN SELECT * FROM tag_packages WHERE package_id = r.package_id AND tag_id = r.tag_id ORDER BY create_event
LOOP
-- always use first and last (active) row
IF last_owner = 0 OR r2.active IS TRUE THEN
last_owner := r2.owner;
RETURN NEXT r2; -- return current row of SELECT
ELSE
-- copy others only if owner changed
IF last_owner <> r2.owner THEN
RETURN NEXT r2;
last_owner := r2.owner;
END IF;
END IF;
END LOOP;
END LOOP;
RETURN;
END
$BODY$
LANGUAGE plpgsql;
INSERT INTO tag_package_owners (SELECT package_id, tag_id, owner create_event revoke_event creator_id revoker_id active FROM convert_owners());
ALTER TABLE tag_packages DROP COLUMN owner;
DROP FUNCTION convert_owners();
COMMIT;

View file

@ -604,6 +604,23 @@ CREATE INDEX tag_packages_create_event ON tag_packages(create_event);
CREATE INDEX tag_packages_revoke_event ON tag_packages(revoke_event);
CREATE INDEX tag_packages_owner ON tag_packages(owner);
CREATE TABLE tag_package_owners (
package_id INTEGER NOT NULL REFERENCES package(id),
tag_id INTEGER NOT NULL REFERENCES tag (id),
owner INTEGER NOT NULL REFERENCES users(id),
-- versioned - see earlier description of versioning
create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(),
revoke_event INTEGER REFERENCES events(id),
creator_id INTEGER NOT NULL REFERENCES users(id),
revoker_id INTEGER REFERENCES users(id),
active BOOLEAN DEFAULT 'true' CHECK (active),
CONSTRAINT active_revoke_sane CHECK (
(active IS NULL AND revoke_event IS NOT NULL AND revoker_id IS NOT NULL)
OR (active IS NOT NULL AND revoke_event IS NULL AND revoker_id IS NULL)),
PRIMARY KEY (create_event, package_id, tag_id),
UNIQUE (package_id,tag_id,active)
) WITHOUT OIDS;
-- package groups (per tag). used for generating comps for the tag repos
CREATE TABLE groups (
id SERIAL NOT NULL PRIMARY KEY,

View file

@ -903,16 +903,32 @@ def _pkglist_remove(tag_id, pkg_id):
update.make_revoke() #XXX user_id?
update.execute()
def _pkglist_owner_remove(tag_id, pkg_id):
clauses = ('package_id=%(pkg_id)i', 'tag_id=%(tag_id)i')
update = UpdateProcessor('tag_package_owners', values=locals(), clauses=clauses)
update.make_revoke() #XXX user_id?
update.execute()
def _pkglist_owner_add(tag_id, pkg_id, owner):
_pkglist_owner_remove(tag_id, pkg_id)
data = {'tag_id': tag_id, 'package_id': pkg_id, 'owner': owner}
insert = InsertProcessor('tag_package_owners', data=data)
insert.make_create() #XXX user_id?
insert.execute()
def _pkglist_add(tag_id, pkg_id, owner, block, extra_arches):
#revoke old entry (if present)
data = dslice(locals(), ('tag_id', 'owner', 'extra_arches'))
data['package_id'] = pkg_id
data['blocked'] = block
data['extra_arches'] = koji.parse_arches(data['extra_arches'], strict=True, allow_none=True)
# revoke old entry (if present)
_pkglist_remove(tag_id, pkg_id)
data = {
'tag_id': tag_id,
'package_id': pkg_id,
'blocked': block,
'extra_arches': koji.parse_arches(extra_arches, strict=True, allow_none=True)
}
insert = InsertProcessor('tag_packages', data=data)
insert.make_create() #XXX user_id?
insert.execute()
_pkglist_owner_add(tag_id, pkg_id, owner)
def pkglist_add(taginfo, pkginfo, owner=None, block=None, extra_arches=None, force=False, update=False):
"""Add to (or update) package list for tag"""
@ -955,6 +971,8 @@ def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force,
# blocked
pkglist = readPackageList(tag_id, pkgID=pkg['id'], inherit=True)
previous = pkglist.get(pkg['id'], None)
changed = False
changed_owner = False
if previous is None:
block = bool(block)
if update and not force:
@ -965,6 +983,7 @@ def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force,
#already there (possibly via inheritance)
if owner is None:
owner = previous['owner_id']
changed_owner = previous['owner_id'] != owner
if block is None:
block = previous['blocked']
else:
@ -972,14 +991,12 @@ def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force,
if extra_arches is None:
extra_arches = previous['extra_arches']
#see if the data is the same
changed = False
for key, value in (('owner_id', owner),
('blocked', block),
('extra_arches', extra_arches)):
for key, value in (('blocked', block),
('extra_arches', extra_arches)):
if previous[key] != value:
changed = True
break
if not changed and not force:
if not changed and not changed_owner and not force:
#no point in adding it again with the same data
return
if previous['blocked'] and not block and not force:
@ -989,7 +1006,10 @@ def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force,
owner = context.session.user_id
else:
raise koji.GenericError("owner not specified")
_pkglist_add(tag_id, pkg['id'], owner, block, extra_arches)
if not previous or changed:
_pkglist_add(tag_id, pkg['id'], owner, block, extra_arches)
elif changed_owner:
_pkglist_owner_add(tag_id, pkg['id'], owner)
koji.plugin.run_callbacks('postPackageListChange', action=action, tag=tag, package=pkg, owner=owner,
block=block, extra_arches=extra_arches, force=force, update=update)
@ -1083,14 +1103,18 @@ def readPackageList(tagID=None, userID=None, pkgID=None, event=None, inherit=Fal
('extra_arches', 'extra_arches'),
('tag_packages.blocked', 'blocked'))
flist = ', '.join([pair[0] for pair in fields])
cond = eventCondition(event)
cond1 = eventCondition(event, table='tag_packages')
cond2 = eventCondition(event, table='tag_package_owners')
q = """
SELECT %(flist)s
FROM tag_packages
JOIN tag on tag.id = tag_packages.tag_id
JOIN package ON package.id = tag_packages.package_id
JOIN users ON users.id = tag_packages.owner
WHERE %(cond)s"""
JOIN tag_package_owners ON
tag_packages.tag_id = tag_package_owners.tag_id AND
tag_packages.package_id = tag_packages.package_id
JOIN users ON users.id = tag_package_owners.owner
WHERE %(cond1)s AND %(cond2)s"""
if tagID != None:
q += """
AND tag.id = %%(tagID)i"""
@ -1205,7 +1229,11 @@ def list_tags(build=None, package=None, queryOpts=None):
joins.append('tag_packages ON tag.id = tag_packages.tag_id')
clauses.append('tag_packages.active = true')
clauses.append('tag_packages.package_id = %(packageID)i')
joins.append('users ON tag_packages.owner = users.id')
joins.append("tag_package_owners ON\n"
" tag_packages.tag_id = tag_package_owners.tag_id AND\n"
" tag_packages.package_id = tag_package_owners.package_id AND\n"
" tag_package_owners.active IS TRUE")
joins.append('users ON tag_package_owners.owner = users.id')
packageID = packageinfo['id']
query = QueryProcessor(columns=fields, aliases=aliases, tables=tables,
@ -3349,6 +3377,7 @@ def _delete_tag(tagInfo):
_tagDelete('build_target_config', tagID, 'dest_tag')
_tagDelete('tag_listing', tagID)
_tagDelete('tag_packages', tagID)
_tagDelete('tag_package_owners', tagID)
_tagDelete('tag_external_repos', tagID)
_tagDelete('group_config', tagID)
_tagDelete('group_req_listing', tagID)
@ -6938,8 +6967,8 @@ def query_history(tables=None, **kwargs):
tables: list of versioned tables to search, no value implies all tables
valid entries: user_perms, user_groups, tag_inheritance, tag_config,
build_target_config, external_repo_config, tag_external_repos,
tag_listing, tag_packages, group_config, group_req_listing,
group_package_listing
tag_listing, tag_packages, tag_package_owners, group_config,
group_req_listing, group_package_listing
- Time options -
times are specified as an integer event or a string timestamp
@ -6998,7 +7027,8 @@ def query_history(tables=None, **kwargs):
'host_channels': ['host_id', 'channel_id'],
'tag_external_repos': ['tag_id', 'external_repo_id', 'priority', 'merge_mode'],
'tag_listing': ['build_id', 'tag_id'],
'tag_packages': ['package_id', 'tag_id', 'owner', 'blocked', 'extra_arches'],
'tag_packages': ['package_id', 'tag_id', 'blocked', 'extra_arches'],
'tag_package_owners': ['package_id', 'tag_id', 'owner'],
'group_config': ['group_id', 'tag_id', 'blocked', 'exported', 'display_name', 'is_default', 'uservisible',
'description', 'langonly', 'biarchonly'],
'group_req_listing': ['group_id', 'tag_id', 'req_id', 'blocked', 'type', 'is_metapkg'],