From f869c3744c0771bb25eefcb2c578bb29bc2589ca Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Aug 29 2019 08:21:54 +0000 Subject: move tag/package owners to separate table Changing owner is triggering repo regeneration in kojira. As it is something which doesn't have any effect on repodata, let's put it to separate table and let kojira ignore these changes. Fixes: https://pagure.io/koji/issue/956 --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 19058ea..2d54606 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -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'], diff --git a/docs/schema-upgrade-1.18-1.19.sql b/docs/schema-upgrade-1.18-1.19.sql index eccce78..9ca4763 100644 --- a/docs/schema-upgrade-1.18-1.19.sql +++ b/docs/schema-upgrade-1.18-1.19.sql @@ -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; diff --git a/docs/schema.sql b/docs/schema.sql index d8d8f25..d9d46c6 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -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, diff --git a/hub/kojihub.py b/hub/kojihub.py index 96196c9..a0c94a4 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -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'],