Add a trigger_type scope to Govern::Policy

Marcos Rocharequested to merge
604367-govern-policy-trigger-type-scope into master

What does this MR do and why?

Govern::Policy gains a for_trigger_type scope, naming the trigger filter that .evaluation_candidates already applied inline. It exists because more than one read needs it: the evaluation read composes it with for_organization and active, and the management read composes it with for_organization alone, so without a scope each caller rewrites the same where.

Split out of the ActiveRecord policy repository MR, where the composed read that uses it lives, so this can be reviewed on its own and that one gets smaller.

Queries

No new query shape, so the plans reviewed in !249130 (merged) still hold. .evaluation_candidates produces byte-identical SQL before and after, since replacing where(trigger_type: trigger_type) with for_trigger_type changes only where the predicate is written:

SELECT "govern_policies".* FROM "govern_policies"
WHERE "govern_policies"."organization_id" = 1
  AND "govern_policies"."lifecycle_state" = 0
  AND "govern_policies"."trigger_type" = 0
ORDER BY "govern_policies"."id" ASC LIMIT 101

That read is served entirely by the composite index index_govern_policies_on_org_trigger_lifecycle_and_id with no sort step, because the index ends in id. The plan is under Plans in !249130 (merged), which added both the index and for_organization.

The one read that composes the scope without a lifecycle_state predicate is the management list, which arrives with the repository adapter rather than here:

SELECT "govern_policies".* FROM "govern_policies"
WHERE "govern_policies"."organization_id" = 1
  AND "govern_policies"."trigger_type" = 0
ORDER BY "govern_policies"."id" ASC

(organization_id, trigger_type) is a prefix of the same composite index, so the filter is an index scan, but a sort step is expected: lifecycle_state sits between trigger_type and id in the index, and with no predicate on it the index is not ordered by id within the range. That matches the for_organization plan in !249130 (merged), which sorts for the same reason.

The scope is not usable on its own at production scale, because the index leads with organization_id, so for_trigger_type alone has no index to use and would scan the table. Nothing calls it that way: every caller composes it with for_organization first, and the only bare uses are in the model spec. Worth knowing before someone reaches for it as a public read.

How to set up and validate locally

Assumes the branch is checked out. No migration, licence tier, or feature flag is needed, since the index this composes against is already on master.

  1. Confirm the scope composes into the same evaluation read, on the rails console
Govern::Policy.evaluation_candidates(
  organization_id: 1, trigger_type: :deployment_requested).to_sql

Verify the predicates are organization_id, lifecycle_state, and trigger_type, ordered by id with LIMIT 101. Checking this out against master and running the same line returns the identical string, which is the point: the refactor is invisible to the query.

  1. Create two policies on one trigger and one on another
organization = Organizations::Organization.first
group = Group.find_by(organization_id: organization.id)

requested = Govern::Policy.create!(organization: organization, namespace: group,
  name: 'Deployment requested policy', trigger_type: :deployment_requested)
promoted = Govern::Policy.create!(organization: organization, namespace: group,
  name: 'Deployment promoted policy', trigger_type: :deployment_promoted)
disabled = Govern::Policy.create!(organization: organization, namespace: group,
  name: 'Disabled policy', trigger_type: :deployment_requested, lifecycle_state: :disabled)
  1. Verify the scope selects by trigger and ignores lifecycle state
scoped = Govern::Policy.for_organization(organization.id)
  .for_trigger_type(:deployment_requested).map(&:name)

scoped.include?('Deployment requested policy') # => true
scoped.include?('Disabled policy')             # => true
scoped.include?('Deployment promoted policy')  # => false

Verify the disabled policy is present, because filtering by lifecycle state belongs to the evaluation read rather than to this scope, and a management surface has to show a policy the user has switched off.

  1. Verify the evaluation read still excludes it, so the two reads remain different
Govern::Policy.evaluation_candidates(
  organization_id: organization.id, trigger_type: :deployment_requested)
  .map(&:name).include?('Disabled policy')
# => false
  1. Ask for the other trigger and confirm only its policy comes back
Govern::Policy.for_organization(organization.id)
  .for_trigger_type(:deployment_promoted).map(&:name)
# => ["Deployment promoted policy"]

References

Edited by Marcos Rocha

Merge request reports