Add filter by ref to project jobs resolver

What does this MR do and why?

For #367294

This MR modifies the parameters that you can provide when getting the list of jobs for a project via GraphQL: it adds a ref filter that should allow you to get only the jobs that ran for that ref.

Screenshots or screen recordings

Screenshots from GraphiQL

jobs query without ref	jobs query with ref

How to set up and validate locally

1. git checkout 367294-project-jobs-resolver-filter-by-ref
2. find/make/import a project with a pipeline with a job that generates artifacts

you can use this CI yaml to generate some sample artifacts:

# .gitlab-ci.yml

potato:
  stage: build
  script:
    - echo 'potato' >> potato.txt
  artifacts:
    expose_as: 'potato'
    paths: ['potato.txt']

tomato:
  stage: build
  script:
    - echo 'tomato' >> tomato.txt
  artifacts:
    expose_as: 'tomato'
    paths: ['tomato.txt']

pineapple:
  stage: build
  script:
    - echo 'pineapple' >> pineapple.txt
  artifacts:
    expose_as: 'pineapple'
    paths: ['pineapple.txt']

apple:
  stage: build
  script:
    - echo 'apple' >> apple.txt
  artifacts:
    expose_as: 'apple'
    paths: ['apple.txt']

toblerone:
  stage: build
  script:
    - echo 'toblerone' >> toblerone.txt
  artifacts:
    expose_as: 'toblerone'
    paths: ['toblerone.txt']

3. run a pipeline for one branch in that project - for this you'll need to set up a local runner for GDK or enable runners in gitpod
4. run a pipeline for another branch
5. using the GraphQL explorer (http://gdk.test:3000/-/graphql-explorer), query for a project's jobs by ref:

query getJobArtifacts {
  project(fullPath: "path/to/project") {  # substitute the full path of your project
    jobs(ref: "main") {                   # substitute the names of your branches
      nodes {
        id
        status
        name
        refName
      }
    }
  }
}

6. when a ref is provided, only jobs that ran for that ref should be returned
7. when a ref is not provided, jobs for any ref should be returned

Database details

Raw SQL

If I use this as the execute method:

    def execute
      builds = init_collection.order_id_desc
      builds = filter_by_with_artifacts(builds)
      builds = filter_by_ref(builds)
      builds = filter_by_scope(builds)
      puts builds.to_sql
      builds
    rescue Gitlab::Access::AccessDeniedError
      type.none
    end

... and run bundle exec rspec spec/finders/ci/jobs_finder_spec.rb:85 (and format the SQL it outputs with pgFormatter), then I get:

SELECT
    "ci_builds".*
FROM
    "ci_builds"
WHERE
    "ci_builds"."type" = 'Ci::Build'
    AND "ci_builds"."project_id" = 23
    AND ("ci_builds"."status" NOT IN ('created'))
    AND "ci_builds"."ref" = 'dev'
ORDER BY
    "ci_builds"."id" DESC

... and as a control, if I comment out builds = filter_by_ref(builds) in the execute method and run it again, I get:

SELECT
    "ci_builds".*
FROM
    "ci_builds"
WHERE
    "ci_builds"."type" = 'Ci::Build'
    AND "ci_builds"."project_id" = 29
    AND ("ci_builds"."status" NOT IN ('created'))
ORDER BY
    "ci_builds"."id" DESC

Query plans

Adding in the gitlab-org/gitlab project id (278964) and the ref name master for testing gives us these two (with and without "ci_builds"."ref" = 'master'):

query	link to plan
SELECT "ci_builds".* FROM "ci_builds" WHERE "ci_builds"."type" = 'Ci::Build' AND "ci_builds"."project_id" = 278964 AND ("ci_builds"."status" NOT IN ('created')) AND "ci_builds"."ref" = 'master' ORDER BY "ci_builds"."id" DESC	https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/13900/commands/48651
SELECT "ci_builds".* FROM "ci_builds" WHERE "ci_builds"."type" = 'Ci::Build' AND "ci_builds"."project_id" = 278964 AND ("ci_builds"."status" NOT IN ('created')) ORDER BY "ci_builds"."id" DESC	https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/13900/commands/48652

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

• I have evaluated the MR acceptance checklist for this MR.

changed milestone to %15.6

added Category:Build Artifacts Deliverable GitLab Free UX VerifyP1 backend devopsverify direction documentation featureenhancement frontend grouppipeline security sectionops typefeature workflowin dev labels

assigned to @mfluharty

Please wait for Reviewer Roulette to suggest a designer for UX review, and then assign them as Reviewer. This helps evenly distribute reviews across UX.

This message was generated automatically. You're welcome to improve it.

added database databasereview pending labels

	2 Messages
	This merge request adds or changes files that require a review from the Database team.
	This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge.

This merge request requires a database review. To make sure these changes are reviewed, take the following steps:

1. Ensure the merge request has database and databasereview pending labels. If the merge request modifies database files, Danger will do this for you.
2. Prepare your MR for database review according to the docs.
3. Assign and mention the database reviewer suggested by Reviewer Roulette.

The following files require a review from the Database team:

• app/finders/ci/jobs_finder.rb

Documentation review

The following files require a review from a technical writer:

• doc/api/graphql/reference/index.md (Link to current live version)

The review does not need to block merging this merge request. See the:

• Metadata for the *.md files that you've changed. The first few lines of each *.md file identify the stage and group most closely associated with your docs change.
• The Technical Writer assigned for that stage and group.
• Documentation workflows for information on when to assign a merge request for review.

Reviewer roulette

Changes that require review have been detected!

Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

Category	Reviewer	Maintainer
backend	Alina Mihaila (@alinamihaila) (UTC+2, 9 hours ahead of @mfluharty)	Douglas Barbosa Alexandre (@dbalexandre) (UTC+0, 7 hours ahead of @mfluharty)
database	Jon Jenkins (@jon_jenkins) (UTC-6, 1 hour ahead of @mfluharty)	Mayra Cabrera (@mayra-cabrera) (UTC-6, 1 hour ahead of @mfluharty)
UX	Veethika M (@v_mishra) (UTC+0, 7 hours ahead of @mfluharty)	Maintainer review is optional for UX

To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

If needed, you can retry the danger-review job that generated this comment.

Generated by Danger

marked this merge request as draft

changed milestone to %15.7

added missed-deliverable missed:15.6 labels

changed the description

added 2285 commits

• eac6dfa2...38fb2dd9 - 2284 commits from branch master
• 264e6093 - Add filter by ref to project jobs resolver

Compare with previous version

added 1154 commits

• 264e6093...8ba06140 - 1153 commits from branch master
• 9d67cf6d - Add filter by ref to project jobs resolver

Compare with previous version

changed the description

@mfluharty could you try creating a scope Ci::Build.with_ref(ref) and test that scope in isolation? Maybe it'll help to unearth the problem.

Sure - scope created in a new commit

added 2 commits

• d599fabf - Set ref on job to fix test
• 5df4e71e - Move where clause to a with_ref scope

Compare with previous version

added 1 commit

• e71e4e6d - Apply 1 suggestion(s) to 1 file(s)

Compare with previous version

Squashing/rebasing this and adding a changelog entry now that it's working...

added 489 commits

• e71e4e6d...10956828 - 488 commits from branch master
• 44412a20 - Add filter by ref to project jobs resolver

Compare with previous version

marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed

marked this merge request as ready

@l.rosa Could you do the database review for this please?

added workflowin review label and removed workflowin dev label

requested review from @l.rosa

@alinamihaila Would you review the backend here?

Thank you @mfluharty! LGTM

@dbalexandre could you please take the maintainer review when you have the chance? Thank you

Thanks, @mfluharty! I have nothing to add besides the @l.rosa notes.

requested review from @alinamihaila

mentioned in merge request !106499 (closed)

mentioned in issue #367294

requested review from @dbalexandre and removed review request for @alinamihaila

I liked the overall approach. I left some comments on the MR that follows the DB review guidelines.

PS: We could split the Ci::JobsFinder into separated classes, maybe using the strategy pattern (based on each type of filtering), but that is out of the scope of this MR.

removed review request for @l.rosa

removed review request for @dbalexandre

changed the description

added 798 commits

• 44412a20...74eefb77 - 796 commits from branch master
• a9d5307f - Add filter by ref to project jobs resolver
• 83e53a23 - Changes from review feedback

Compare with previous version

requested review from @l.rosa

approved this merge request

@l.rosa, thanks for approving this merge request.

This is the first time the merge request is approved. To ensure full test coverage, a new pipeline will be started shortly.

For more info, please refer to the following links:

• rspec minimal jobs
• jest minimal jobs
• merging a merge request.

added pipeline:mr-approved label

removed databasereview pending label

added databaseapproved label

removed review request for @l.rosa

requested review from @krasio

removed databaseapproved label

added databasereviewed label

@mfluharty code looks pretty good!

It follows the DB guidelines and the queries performance are excelent.

Raw SQL	Query Plans	Code	Tests
!104191 (closed)	!104191 (closed)

/cc @krasio I'm assigning you to do the final approval here ~"database::approved"

@mfluharty @l.rosa Seems like this is not the final SQL that will be generated, as the GraphQL API should add pagination, and we do not have LIMIT in this queries. To get the actual SQL it's usually better to execute a request against our local app, and then find the query in the logs. Once we have it, we can replace parameters like project_id and check the plan in DB Lab.

As is, the above queries will load all rows matching the filters, which is probably not what we want, and may hit the timeout if there are too many, or it's too slow to find them. This is not obvious from the execution plans we have, as they return 0 rows, and the reason for this is that these plans are from the main database (named gitlab-production-tunnel-pg12 in postgres.ai), and not ci (which is called gitlab-production-ci here). CI related tables on main were truncated, hence we get no results.

Can you please try again with the actual SQL and using the ci database, and go through review with Leo once again. Assign back for maintainer review when ready.

Nice catch @krasio! @mfluharty could you please, take a look?