#1473 move tag/package owners to separate table
Merged 2 years ago by tkopecek. Opened 2 years ago by tkopecek.
tkopecek/koji issue956  into  master

@@ -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'],

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

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,

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'],

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

2 years ago

Why was the 'base64' import removed?

It is not used anymore. base64encode is used instead now in this file.

Needs rebase but otherwise :thumbsup:

rebased onto f869c37

2 years ago

Commit 1e35e60 fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago

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

2 years ago