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 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
1diff --git a/database/schema/patch-2210-26-1.sql b/database/schema/patch-2210-26-1.sql
2new file mode 100644
3index 0000000..90fdbf3
4--- /dev/null
5+++ b/database/schema/patch-2210-26-1.sql
6@@ -0,0 +1,130 @@
7+-- Copyright 2021 Canonical Ltd. This software is licensed under the
8+-- GNU Affero General Public License version 3 (see the file LICENSE).
9+
10+SET client_min_messages=ERROR;
11+
12+ALTER TABLE Snap
13+ ADD COLUMN information_type integer,
14+ ADD COLUMN project integer REFERENCES product,
15+ ADD COLUMN access_policy integer,
16+ ADD COLUMN access_grants integer[];
17+
18+COMMENT ON COLUMN Snap.private IS
19+ '(DEPRECATED; use Snap.information_type) Whether or not this snap is private.';
20+COMMENT ON COLUMN Snap.project IS
21+ 'The project which is the pillar for this Snap';
22+COMMENT ON COLUMN Snap.information_type IS
23+ '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.';
24+
25+CREATE TABLE SnapSubscription (
26+ id serial PRIMARY KEY,
27+ snap integer NOT NULL REFERENCES Snap(id),
28+ person integer NOT NULL REFERENCES Person(id),
29+ date_created timestamp without time zone DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') NOT NULL,
30+ subscribed_by integer NOT NULL REFERENCES Person(id)
31+);
32+
33+COMMENT ON TABLE SnapSubscription IS 'Person subscription for Snap recipes.';
34+COMMENT ON COLUMN SnapSubscription.person IS
35+ 'The person who subscribed to the Snap.';
36+COMMENT ON COLUMN SnapSubscription.snap IS
37+ 'The Snap recipe to which the person subscribed.';
38+COMMENT ON COLUMN SnapSubscription.date_created IS
39+ 'When the subscription was created.';
40+COMMENT ON COLUMN SnapSubscription.subscribed_by IS
41+ 'The person performing the action of subscribing someone to the Snap.';
42+
43+
44+CREATE UNIQUE INDEX snapsubscription__person_snap__key
45+ ON SnapSubscription(snap, person);
46+
47+CREATE INDEX snapsubscription__person__idx
48+ ON SnapSubscription(person);
49+
50+CREATE INDEX snapsubscription__subscribed_by__idx
51+ ON SnapSubscription(subscribed_by);
52+
53+ALTER TABLE AccessArtifact
54+ ADD COLUMN snap integer REFERENCES snap;
55+
56+
57+ALTER TABLE AccessArtifact DROP CONSTRAINT has_artifact;
58+ALTER TABLE AccessArtifact
59+ ADD CONSTRAINT has_artifact CHECK (
60+ (null_count(ARRAY[bug, branch, gitrepository, snap, specification]) = 4)) NOT VALID;
61+
62+
63+CREATE OR REPLACE FUNCTION snap_denorm_access(snap_id integer)
64+ RETURNS void LANGUAGE plpgsql AS
65+$$
66+DECLARE
67+ info_type integer;
68+BEGIN
69+ -- XXX pappacena 2021-02-12: Once we finish filling "information_type" and
70+ -- deprecate the usage of "public" column at code level, we will be able to
71+ -- drop the "private" column usage here.
72+ SELECT
73+ COALESCE(
74+ snap.information_type,
75+ -- information type: 1 = public; 5 = proprietary
76+ CASE WHEN snap.private THEN 5 ELSE 1 END
77+ )
78+ INTO info_type
79+ FROM snap WHERE id = snap_id;
80+
81+ UPDATE Snap
82+ SET access_policy = policies[1], access_grants = grants
83+ FROM
84+ build_access_cache(
85+ (SELECT id FROM accessartifact WHERE snap = snap_id),
86+ info_type)
87+ AS (policies integer[], grants integer[])
88+ WHERE id = snap_id;
89+END;
90+$$;
91+
92+CREATE OR REPLACE FUNCTION accessartifact_denorm_to_artifacts(artifact_id integer)
93+ RETURNS void
94+ LANGUAGE plpgsql
95+ AS $$
96+DECLARE
97+ artifact_row accessartifact%ROWTYPE;
98+BEGIN
99+ SELECT * INTO artifact_row FROM accessartifact WHERE id = artifact_id;
100+ IF artifact_row.bug IS NOT NULL THEN
101+ PERFORM bug_flatten_access(artifact_row.bug);
102+ END IF;
103+ IF artifact_row.branch IS NOT NULL THEN
104+ PERFORM branch_denorm_access(artifact_row.branch);
105+ END IF;
106+ IF artifact_row.gitrepository IS NOT NULL THEN
107+ PERFORM gitrepository_denorm_access(artifact_row.gitrepository);
108+ END IF;
109+ IF artifact_row.snap IS NOT NULL THEN
110+ PERFORM snap_denorm_access(artifact_row.snap);
111+ END IF;
112+ IF artifact_row.specification IS NOT NULL THEN
113+ PERFORM specification_denorm_access(artifact_row.specification);
114+ END IF;
115+ RETURN;
116+END;
117+$$;
118+
119+COMMENT ON FUNCTION accessartifact_denorm_to_artifacts(artifact_id integer) IS
120+ 'Denormalize the policy access and artifact grants to bugs, branches, git repositories, snaps, and specifications.';
121+
122+-- A trigger to handle snap.{private,information_type,project} changes.
123+CREATE OR REPLACE FUNCTION snap_maintain_access_cache_trig() RETURNS trigger
124+ LANGUAGE plpgsql AS $$
125+BEGIN
126+ PERFORM snap_denorm_access(NEW.id);
127+ RETURN NULL;
128+END;
129+$$;
130+
131+CREATE TRIGGER snap_maintain_access_cache
132+ AFTER INSERT OR UPDATE OF private, information_type, project ON Snap
133+ FOR EACH ROW EXECUTE PROCEDURE snap_maintain_access_cache_trig();
134+
135+
136+INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 26, 1);
137diff --git a/database/schema/security.cfg b/database/schema/security.cfg
138index bf4b81c..e343a5f 100644
139--- a/database/schema/security.cfg
140+++ b/database/schema/security.cfg
141@@ -302,6 +302,7 @@ public.snapbuild = SELECT, INSERT, UPDATE, DELETE
142 public.snapbuildjob = SELECT, INSERT, UPDATE, DELETE
143 public.snapfile = SELECT, INSERT, UPDATE, DELETE
144 public.snapjob = SELECT, INSERT, UPDATE, DELETE
145+public.snapsubscription = SELECT, INSERT, UPDATE, DELETE
146 public.snappydistroseries = SELECT, INSERT, UPDATE, DELETE
147 public.snappyseries = SELECT, INSERT, UPDATE, DELETE
148 public.sourcepackageformatselection = SELECT
149@@ -2246,6 +2247,7 @@ type=user
150
151 [person-merge-job]
152 groups=script
153+public.accesspolicyartifact = SELECT
154 public.accessartifactgrant = SELECT, UPDATE, DELETE
155 public.accesspolicy = SELECT, UPDATE, DELETE
156 public.accesspolicygrant = SELECT, UPDATE, DELETE
157@@ -2363,6 +2365,7 @@ public.signedcodeofconduct = SELECT, UPDATE
158 public.snap = SELECT, UPDATE
159 public.snapbase = SELECT, UPDATE
160 public.snapbuild = SELECT, UPDATE
161+public.snapsubscription = SELECT, UPDATE, DELETE
162 public.snappyseries = SELECT, UPDATE
163 public.sourcepackagename = SELECT
164 public.sourcepackagepublishinghistory = SELECT, UPDATE
165diff --git a/lib/lp/registry/personmerge.py b/lib/lp/registry/personmerge.py
166index 7873a61..50e0f91 100644
167--- a/lib/lp/registry/personmerge.py
168+++ b/lib/lp/registry/personmerge.py
169@@ -917,6 +917,10 @@ def merge_people(from_person, to_person, reviewer, delete=False):
170 _mergeSnap(cur, from_person, to_person)
171 skip.append(('snap', 'owner'))
172
173+ # XXX pappacena 2021-02-18: add tests for this once we have
174+ # SnapSubscription model in place.
175+ skip.append(('snapsubscription', 'person'))
176+
177 _mergeOCIRecipe(cur, from_person, to_person)
178 skip.append(('ocirecipe', 'owner'))
179