Add services, finder, and policy for merge request saved views

What does this MR do and why?

Adds CreateService, UpdateService and DeleteService, a finder listing the current user's views ordered by id, and a policy scoping every ability to the owning user.

Create is authorized against the User subject via UserPolicy, since there is no persisted record at create time. This matches the existing create_saved_replies idiom.

The finder scopes to current_user, so the scoping is itself the authorization check - there is no way to ask it for another user's views.

This is the second of four backend merge requests for merge request dashboard saved views:

  1. !247083 (merged) - table, model, JSON schema, factory
  2. This merge request - services, finder, and authorization policy
  3. !247106 (merged) - GraphQL read API (currentUser.mergeRequestSavedViews)
  4. !247119 (merged) - GraphQL mutations (create, update, delete)

Database queries

merge_request_saved_views is a new table (added in !247083 (merged)) and is empty in production. Every query is scoped to a single user_id, and rows per user are capped at 5 (MAX_VIEWS_PER_USER), so no access path is unbounded. Available indexes are merge_request_saved_views_pkey on (id) and the unique index_merge_request_saved_views_on_user_id_and_name on (user_id, name). Plans below are from a local development database.

Finder (SavedViewsFinder#execute)

SELECT "merge_request_saved_views".* FROM "merge_request_saved_views"
WHERE "merge_request_saved_views"."user_id" = 1
ORDER BY "merge_request_saved_views"."id" ASC
Sort  (cost=4.23..4.23 rows=3 width=96)
  Sort Key: id
  ->  Index Scan using index_merge_request_saved_views_on_user_id_and_name  (cost=0.15..4.20 rows=3 width=96)
        Index Cond: (user_id = 1)

Name uniqueness validation (CreateService, and UpdateService with the extra id != term)

SELECT 1 AS one FROM "merge_request_saved_views"
WHERE "merge_request_saved_views"."name" = 'x'
  AND "merge_request_saved_views"."user_id" = 1
LIMIT 1
Limit  (cost=0.15..2.17 rows=1 width=4)
  ->  Index Only Scan using index_merge_request_saved_views_on_user_id_and_name  (cost=0.15..2.17 rows=1 width=4)
        Index Cond: ((user_id = 1) AND (name = 'x'::text))

Per-user limit check (validate_views_limit, create only)

SELECT COUNT(*) FROM "merge_request_saved_views" WHERE "merge_request_saved_views"."user_id" = 1
Aggregate  (cost=4.21..4.22 rows=1 width=8)
  ->  Index Only Scan using index_merge_request_saved_views_on_user_id_and_name  (cost=0.15..4.20 rows=3 width=0)
        Index Cond: (user_id = 1)

Writes

CreateService takes a FOR UPDATE lock on the owning users row so two concurrent creates cannot both pass the limit check, then inserts. UpdateService and DeleteService address a single row by primary key.

INSERT INTO "merge_request_saved_views" ("user_id", "created_at", "updated_at", "name", "filters")
VALUES (1, '...', '...', 'name', '{"state":"opened"}') RETURNING "id"

UPDATE "merge_request_saved_views" SET "updated_at" = '...', "name" = 'renamed'
WHERE "merge_request_saved_views"."id" = 1

DELETE FROM "merge_request_saved_views" WHERE "merge_request_saved_views"."id" = 1
Update on merge_request_saved_views  (cost=0.15..2.17 rows=0 width=0)
  ->  Index Scan using merge_request_saved_views_pkey  (cost=0.15..2.17 rows=1 width=38)
        Index Cond: (id = 1)

Delete on merge_request_saved_views  (cost=0.15..2.17 rows=0 width=0)
  ->  Index Scan using merge_request_saved_views_pkey  (cost=0.15..2.17 rows=1 width=6)
        Index Cond: (id = 1)

How to set up and validate locally

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Phil Hughes

Merge request reports

Loading
Loading