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

koji

#1473 move tag/package owners to separate table

Merged 4 years ago by tkopecek. Opened 4 years ago by tkopecek.

tkopecek/koji issue956 into master

move tag/package owners to separate table

Tomas Kopecek • 4 years ago

f869c37

cli/koji_cli/commands.py

file modified

		`@@ -4200,6 +4200,13 @@`
		`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 @@`
		`'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'],`

docs/schema-upgrade-1.18-1.19.sql

file modified

+50

		`@@ -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;`

docs/schema.sql

file modified

+17

		`@@ -604,6 +604,23 @@`
		`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,`

hub/kojihub.py

file modified

+48 -18

		`@@ -903,16 +903,32 @@`
		`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 @@`
		`# 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 @@`
		`#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 @@`
		`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 @@`
		`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 @@`
		`('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 @@`
		`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 @@`
		`_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 @@`
		`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 @@`
		`'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'],`

tkopecek commented 4 years ago

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

rebased onto 07a15f6f098bdf1d987fcc17453b0d9319f3d86a

4 years ago