Merge ~pappacena/launchpad:snap-pillar-db into launchpad:db-devel

Proposed by Thiago F. Pappacena
Status: Merged
Approved by: Thiago F. Pappacena
Approved revision: a44145f5d98afb821a19f0802b5464bbea37b677
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pappacena/launchpad:snap-pillar-db
Merge into: launchpad:db-devel
Diff against target: 178 lines (+137/-0)
3 files modified
database/schema/patch-2210-26-1.sql (+130/-0)
database/schema/security.cfg (+3/-0)
lib/lp/registry/personmerge.py (+4/-0)
Reviewer Review Type Date Requested Status
William Grant db Approve
Colin Watson (community) db Approve
Review via email: mp+397459@code.launchpad.net

Commit message

Adding Snap.project for private Snaps' optional pillar

Description of the change

Related code changes here: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/397458.
Some columns created here have index creation and CHECK constraint validation in this other MP: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/398361

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

I'm OK with this, but I wonder if it's worth folding the necessary AccessArtifact-related changes into this patch as well? See database/schema/archive/patch-2209-61-0.sql for an example from the last time we extended this sort of thing.

Adding William for a second DB review.

review: Approve (db)
~pappacena/launchpad:snap-pillar-db updated
f2abd3c... by Thiago F. Pappacena

Adding access_policy and access_grants caches to Snap table

Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

I've pushed the changes to AccessArtifact similar to those ones used for GitRepository, with a couple of assumptions:

- Assuming information_type 1 (PUBLIC) when snap.private is false; and 5 (PROPRIETARY) when it's true
- Updating Snap.access_policy and Snap.access_grants when either "private" or "product" columns changes

~pappacena/launchpad:snap-pillar-db updated
9c49e1b... by Thiago F. Pappacena

Adding SnapSubscription model

Revision history for this message
Colin Watson (cjwatson) :
~pappacena/launchpad:snap-pillar-db updated
08fb5e5... by Thiago F. Pappacena

Adding Snap.information_type column

Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

Pushed the requested changes

~pappacena/launchpad:snap-pillar-db updated
0510967... by Thiago F. Pappacena

Fixing ambiguity in snap_denorm_access

Revision history for this message
Colin Watson (cjwatson) :
review: Approve (db)
Revision history for this message
William Grant (wgrant) wrote :

Thanks, just a couple of small things.

review: Approve (db)
Revision history for this message
Colin Watson (cjwatson) :
~pappacena/launchpad:snap-pillar-db updated
bcdb89b... by Thiago F. Pappacena

Minor adjustments, and moving large index/constraint creation to another patch

a44145f... by Thiago F. Pappacena

Skipping personmerge sanity checks on SnapSubscription (while we dont have this entity in place)

Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

I think all comments were addressed here.

I'll open another MP with index creation and CHECK constraints validations on existing tables, so we run it separately (and with CONCURRENT on index creation).

Revision history for this message
William Grant (wgrant) :
review: Approve (db)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/database/schema/patch-2210-26-1.sql b/database/schema/patch-2210-26-1.sql
0new file mode 1006440new file mode 100644
index 0000000..90fdbf3
--- /dev/null
+++ b/database/schema/patch-2210-26-1.sql
@@ -0,0 +1,130 @@
1-- Copyright 2021 Canonical Ltd. This software is licensed under the
2-- GNU Affero General Public License version 3 (see the file LICENSE).
3
4SET client_min_messages=ERROR;
5
6ALTER TABLE Snap
7 ADD COLUMN information_type integer,
8 ADD COLUMN project integer REFERENCES product,
9 ADD COLUMN access_policy integer,
10 ADD COLUMN access_grants integer[];
11
12COMMENT ON COLUMN Snap.private IS
13 '(DEPRECATED; use Snap.information_type) Whether or not this snap is private.';
14COMMENT ON COLUMN Snap.project IS
15 'The project which is the pillar for this Snap';
16COMMENT ON COLUMN Snap.information_type IS
17 'Enum describing what type of information is stored, such as type of private or security related data, and used to determine to apply an access policy.';
18
19CREATE TABLE SnapSubscription (
20 id serial PRIMARY KEY,
21 snap integer NOT NULL REFERENCES Snap(id),
22 person integer NOT NULL REFERENCES Person(id),
23 date_created timestamp without time zone DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') NOT NULL,
24 subscribed_by integer NOT NULL REFERENCES Person(id)
25);
26
27COMMENT ON TABLE SnapSubscription IS 'Person subscription for Snap recipes.';
28COMMENT ON COLUMN SnapSubscription.person IS
29 'The person who subscribed to the Snap.';
30COMMENT ON COLUMN SnapSubscription.snap IS
31 'The Snap recipe to which the person subscribed.';
32COMMENT ON COLUMN SnapSubscription.date_created IS
33 'When the subscription was created.';
34COMMENT ON COLUMN SnapSubscription.subscribed_by IS
35 'The person performing the action of subscribing someone to the Snap.';
36
37
38CREATE UNIQUE INDEX snapsubscription__person_snap__key
39 ON SnapSubscription(snap, person);
40
41CREATE INDEX snapsubscription__person__idx
42 ON SnapSubscription(person);
43
44CREATE INDEX snapsubscription__subscribed_by__idx
45 ON SnapSubscription(subscribed_by);
46
47ALTER TABLE AccessArtifact
48 ADD COLUMN snap integer REFERENCES snap;
49
50
51ALTER TABLE AccessArtifact DROP CONSTRAINT has_artifact;
52ALTER TABLE AccessArtifact
53 ADD CONSTRAINT has_artifact CHECK (
54 (null_count(ARRAY[bug, branch, gitrepository, snap, specification]) = 4)) NOT VALID;
55
56
57CREATE OR REPLACE FUNCTION snap_denorm_access(snap_id integer)
58 RETURNS void LANGUAGE plpgsql AS
59$$
60DECLARE
61 info_type integer;
62BEGIN
63 -- XXX pappacena 2021-02-12: Once we finish filling "information_type" and
64 -- deprecate the usage of "public" column at code level, we will be able to
65 -- drop the "private" column usage here.
66 SELECT
67 COALESCE(
68 snap.information_type,
69 -- information type: 1 = public; 5 = proprietary
70 CASE WHEN snap.private THEN 5 ELSE 1 END
71 )
72 INTO info_type
73 FROM snap WHERE id = snap_id;
74
75 UPDATE Snap
76 SET access_policy = policies[1], access_grants = grants
77 FROM
78 build_access_cache(
79 (SELECT id FROM accessartifact WHERE snap = snap_id),
80 info_type)
81 AS (policies integer[], grants integer[])
82 WHERE id = snap_id;
83END;
84$$;
85
86CREATE OR REPLACE FUNCTION accessartifact_denorm_to_artifacts(artifact_id integer)
87 RETURNS void
88 LANGUAGE plpgsql
89 AS $$
90DECLARE
91 artifact_row accessartifact%ROWTYPE;
92BEGIN
93 SELECT * INTO artifact_row FROM accessartifact WHERE id = artifact_id;
94 IF artifact_row.bug IS NOT NULL THEN
95 PERFORM bug_flatten_access(artifact_row.bug);
96 END IF;
97 IF artifact_row.branch IS NOT NULL THEN
98 PERFORM branch_denorm_access(artifact_row.branch);
99 END IF;
100 IF artifact_row.gitrepository IS NOT NULL THEN
101 PERFORM gitrepository_denorm_access(artifact_row.gitrepository);
102 END IF;
103 IF artifact_row.snap IS NOT NULL THEN
104 PERFORM snap_denorm_access(artifact_row.snap);
105 END IF;
106 IF artifact_row.specification IS NOT NULL THEN
107 PERFORM specification_denorm_access(artifact_row.specification);
108 END IF;
109 RETURN;
110END;
111$$;
112
113COMMENT ON FUNCTION accessartifact_denorm_to_artifacts(artifact_id integer) IS
114 'Denormalize the policy access and artifact grants to bugs, branches, git repositories, snaps, and specifications.';
115
116-- A trigger to handle snap.{private,information_type,project} changes.
117CREATE OR REPLACE FUNCTION snap_maintain_access_cache_trig() RETURNS trigger
118 LANGUAGE plpgsql AS $$
119BEGIN
120 PERFORM snap_denorm_access(NEW.id);
121 RETURN NULL;
122END;
123$$;
124
125CREATE TRIGGER snap_maintain_access_cache
126 AFTER INSERT OR UPDATE OF private, information_type, project ON Snap
127 FOR EACH ROW EXECUTE PROCEDURE snap_maintain_access_cache_trig();
128
129
130INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 26, 1);
diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index bf4b81c..e343a5f 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -302,6 +302,7 @@ public.snapbuild = SELECT, INSERT, UPDATE, DELETE
302public.snapbuildjob = SELECT, INSERT, UPDATE, DELETE302public.snapbuildjob = SELECT, INSERT, UPDATE, DELETE
303public.snapfile = SELECT, INSERT, UPDATE, DELETE303public.snapfile = SELECT, INSERT, UPDATE, DELETE
304public.snapjob = SELECT, INSERT, UPDATE, DELETE304public.snapjob = SELECT, INSERT, UPDATE, DELETE
305public.snapsubscription = SELECT, INSERT, UPDATE, DELETE
305public.snappydistroseries = SELECT, INSERT, UPDATE, DELETE306public.snappydistroseries = SELECT, INSERT, UPDATE, DELETE
306public.snappyseries = SELECT, INSERT, UPDATE, DELETE307public.snappyseries = SELECT, INSERT, UPDATE, DELETE
307public.sourcepackageformatselection = SELECT308public.sourcepackageformatselection = SELECT
@@ -2246,6 +2247,7 @@ type=user
22462247
2247[person-merge-job]2248[person-merge-job]
2248groups=script2249groups=script
2250public.accesspolicyartifact = SELECT
2249public.accessartifactgrant = SELECT, UPDATE, DELETE2251public.accessartifactgrant = SELECT, UPDATE, DELETE
2250public.accesspolicy = SELECT, UPDATE, DELETE2252public.accesspolicy = SELECT, UPDATE, DELETE
2251public.accesspolicygrant = SELECT, UPDATE, DELETE2253public.accesspolicygrant = SELECT, UPDATE, DELETE
@@ -2363,6 +2365,7 @@ public.signedcodeofconduct = SELECT, UPDATE
2363public.snap = SELECT, UPDATE2365public.snap = SELECT, UPDATE
2364public.snapbase = SELECT, UPDATE2366public.snapbase = SELECT, UPDATE
2365public.snapbuild = SELECT, UPDATE2367public.snapbuild = SELECT, UPDATE
2368public.snapsubscription = SELECT, UPDATE, DELETE
2366public.snappyseries = SELECT, UPDATE2369public.snappyseries = SELECT, UPDATE
2367public.sourcepackagename = SELECT2370public.sourcepackagename = SELECT
2368public.sourcepackagepublishinghistory = SELECT, UPDATE2371public.sourcepackagepublishinghistory = SELECT, UPDATE
diff --git a/lib/lp/registry/personmerge.py b/lib/lp/registry/personmerge.py
index 7873a61..50e0f91 100644
--- a/lib/lp/registry/personmerge.py
+++ b/lib/lp/registry/personmerge.py
@@ -917,6 +917,10 @@ def merge_people(from_person, to_person, reviewer, delete=False):
917 _mergeSnap(cur, from_person, to_person)917 _mergeSnap(cur, from_person, to_person)
918 skip.append(('snap', 'owner'))918 skip.append(('snap', 'owner'))
919919
920 # XXX pappacena 2021-02-18: add tests for this once we have
921 # SnapSubscription model in place.
922 skip.append(('snapsubscription', 'person'))
923
920 _mergeOCIRecipe(cur, from_person, to_person)924 _mergeOCIRecipe(cur, from_person, to_person)
921 skip.append(('ocirecipe', 'owner'))925 skip.append(('ocirecipe', 'owner'))
922926