Merge lp:~wgrant/launchpad/observer-db into lp:launchpad/db-devel

[William Grant]

[r=lifeless,stub][bug=890931] First part of the AccessPolicy DB model.

This is the initial DB patch for Disclosure's access policies. The migration process will be long and tortuous, but this is a start.

A private artifact (currently a bug or a branch) will be linked to one of its target's access policies, and attempts to access the artifact will be checked against the policy's permissions. This will shortly supplant the existing subscription-and-other-stuff-based checks.

Permissions can be either policy-global or artifact-specific. In the first case APP.artifact is left unset, letting the permission holder see all artifacts under the policy. For the second case both policy and artifact are set, restricting the access to a specific artifact under a specific policy.

The identification of access policies isn't well-defined yet, but this schema will do for now. The intent is that, at least for the initial pass, projects will have preconfigured and immutable "Private" and "Security" policies, with the existing bug privacy/security checkboxes altered to map onto these policies. So we either have to use an enum, or just treat these as well-known names until a later pass.

[Robert Collins (lifeless)]

I think s/Permission/Grant/ will avoid some confusion.

This looks like what we discussed. Cool. Worth nothing I think is some more intent.
E.g. for artifacts mention that more types should be added there rather than in parallel tables.

Stuart: the idea here is that reporting is simple: one table supplies all the grants made to a private project/distro. Rather than a constantly evolving set of unions, this provides a single (indirect) level in the DB : *and* lets us query and report grants without accessing the granted-on objects [at least at this point]).

+1 from me but please get Stuarts ok before landing.

review: Approve

[William Grant (wgrant)]

The schema has been revised significantly. Key changes:

 - AccessPolicy now has a type enum column (Private, Security, etc.) instead of names. This is hopefully sufficiently inflexible that people won't do stupid things, makes bug and branch retargeting easier, and makes the table smaller.
 - AccessPolicyArtifact.policy now exists, so AccessPolicyGrant.{policy,artifact} are mutually exclusive.
 - AccessPolicyGrant has gained grantor/date_created columns, because unauditable grants are bad.

Bug/Branch retain their FKs directly to AP, rather than just having a policy type and inferring the target from the object's context. Multi-task bugs make determining that context non-trivial, and we have to have the AP FK denormed onto APA anyway.

Public bugs can have any number of targets, so it's not really possible to link them to a particular policy in any meaningful way. This, along with performance concerns around the expense that would be imposed on queries for public bugs, lead me to not have an AccessPolicyType.PUBLIC, and hence no AccessPolicy for public artifacts.

[Stuart Bishop (stub)]

We need an index on grantor to keep person merging happy:

CREATE INDEX accesspolicygrant__grantor__idx ON AccessPolicyGrant(grantor);

Otherwise all good!

review: Approve (db)

[Robert Collins (lifeless)]

This is great. I'd love an indicative result on prod size data for bug search & branch search - adapted current queries. Just as a safety belt. Doing that anytime before we go live with this is fine, but the earlier the easier from a fdt deploys perspective.

review: Approve

[Robert Collins (lifeless)]

Oh, and
+COMMENT ON COLUMN AccessPolicyArtifact.policy IS 'An optional policy that controls access to this artifact. Otherwise the artifact is public.';

Do we want APA rows for public artifacts at all? Or are we treating bug/branch.policy as a denormalisation [put another way - which side is authoritative if we are doing data repair in the future]. I would have expected 'public bugs have no APA rows'.

=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql	2011-11-09 19:55:49 +0000
+++ database/schema/comments.sql	2011-11-16 03:17:30 +0000
@@ -6,6 +6,29 @@
      GNU Affero General Public License version 3 (see the file LICENSE).
 */
 
+-- AccessPolicy
+
+COMMENT ON TABLE AccessPolicy IS 'An access policy used to manage a project or distribution\'s artifacts.';
+COMMENT ON COLUMN AccessPolicy.product IS 'The product that this policy is used on.';
+COMMENT ON COLUMN AccessPolicy.distribution IS 'The distribution that this policy is used on.';
+COMMENT ON COLUMN AccessPolicy.type IS 'The type of policy (an enum value). Private, Security, etc.';
+
+-- AccessPolicyArtifact
+
+COMMENT ON TABLE AccessPolicyArtifact IS 'An artifact that an access grant can apply to. Additional private artifacts should be handled by adding new columns here, rather than new tables or columns on AccessPolicyGrant.';
+COMMENT ON COLUMN AccessPolicyArtifact.bug IS 'The bug that this abstract artifact represents.';
+COMMENT ON COLUMN AccessPolicyArtifact.branch IS 'The branch that this abstract artifact represents.';
+COMMENT ON COLUMN AccessPolicyArtifact.policy IS 'An optional policy that controls access to this artifact. Otherwise the artifact is public.';
+
+-- AccessPolicyGrant
+
+COMMENT ON TABLE AccessPolicyGrant IS 'A grant for a person to access a specific artifact or all artifacts controlled by a particular policy.';
+COMMENT ON COLUMN AccessPolicyGrant.grantee IS 'The person to whom access is granted.';
+COMMENT ON COLUMN AccessPolicyGrant.grantor IS 'The person who granted the access.';
+COMMENT ON COLUMN AccessPolicyGrant.date_created IS 'The date the access was granted.';
+COMMENT ON COLUMN AccessPolicyGrant.policy IS 'The policy on which access is granted.';
+COMMENT ON COLUMN AccessPolicyGrant.artifact IS 'The artifact on which access is granted.';
+
 -- Announcement
 
 COMMENT ON TABLE Announcement IS 'A project announcement. This is a single item of news or information that the project is communicating. Announcements can be attached to a Project, a Product or a Distribution.';

=== added file 'database/schema/patch-2208-93-1.sql'
--- database/schema/patch-2208-93-1.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-93-1.sql	2011-11-16 03:17:30 +0000
@@ -0,0 +1,59 @@
+-- Copyright 2011 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+SET client_min_messages=ERROR;
+
+CREATE TABLE AccessPolicy (
+    id serial PRIMARY KEY,
+    product integer REFERENCES Product,
+    distribution integer REFERENCES Distribution,
+    type integer NOT NULL,
+    CONSTRAINT has_target CHECK (product IS NULL != distribution IS NULL)
+);
+
+CREATE UNIQUE INDEX accesspolicy__product__type__key
+    ON AccessPolicy(product, type) WHERE product IS NOT NULL;
+CREATE UNIQUE INDEX accesspolicy__distribution__type__key
+    ON AccessPolicy(distribution, type) WHERE distribution IS NOT NULL;
+
+CREATE TABLE AccessPolicyArtifact (
+    id serial PRIMARY KEY,
+    bug integer REFERENCES Bug,
+    branch integer, -- FK to be added later.
+    policy integer REFERENCES AccessPolicy,
+    CONSTRAINT has_artifact CHECK (bug IS NULL != branch IS NULL)
+);
+
+CREATE UNIQUE INDEX accesspolicyartifact__bug__key
+    ON AccessPolicyArtifact(bug) WHERE bug IS NOT NULL;
+CREATE UNIQUE INDEX accesspolicyartifact__branch__key
+    ON AccessPolicyArtifact(branch) WHERE branch IS NOT NULL;
+CREATE INDEX accesspolicyartifact__policy__key
+    ON AccessPolicyArtifact(policy);
+
+CREATE TABLE AccessPolicyGrant (
+    id serial PRIMARY KEY,
+    grantee integer NOT NULL, -- FK to be added later.
+    grantor integer NOT NULL, -- FK to be added later.
+    date_created timestamp without time zone
+        DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') NOT NULL,
+    policy integer REFERENCES AccessPolicy,
+    artifact integer REFERENCES AccessPolicyArtifact,
+    CONSTRAINT has_target CHECK (policy IS NULL != artifact IS NULL)
+);
+
+CREATE UNIQUE INDEX accesspolicygrant__policy__grantee__key
+    ON AccessPolicyGrant(policy, grantee) WHERE policy IS NOT NULL;
+CREATE UNIQUE INDEX accessartifactgrant__artifact__grantee__key
+    ON AccessPolicyGrant(artifact, grantee) WHERE artifact IS NOT NULL;
+CREATE INDEX accesspolicygrant__grantee__idx ON AccessPolicyGrant(grantee);
+CREATE INDEX accesspolicygrant__grantor__idx ON AccessPolicyGrant(grantor);
+
+ALTER TABLE bug
+    ADD COLUMN access_policy integer REFERENCES AccessPolicy;
+CREATE INDEX bug__access_policy__idx ON bug(access_policy);
+
+ALTER TABLE branch
+    ADD COLUMN access_policy integer REFERENCES AccessPolicy;
+CREATE INDEX branch__access_policy__idx ON branch(access_policy);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 93, 1);