Preserve offset pagination marker through merge and unscope

What does this MR do and why?

Gitlab::Graphql::Pagination::OffsetPaginatedRelation is a marker class. The schema uses it to select OffsetActiveRecordRelationConnection instead of the keyset connection (lib/gitlab/graphql/pagination/connections.rb:9-16). It re-wrapped itself for preload and includes, but not for merge, so a merge on a marked relation returned a bare ActiveRecord::Relation and the marker was lost.

Resolvers::MergeRequestPipelinesResolver#query_for does exactly that:

resolve_pipelines(mr.source_project, args.merge(merge_request_event_first: true))
  .merge(mr.all_pipelines)

resolve_pipelines returns offset_pagination(pipelines) for the branches and tags scopes (app/graphql/resolvers/concerns/resolves_pipelines.rb:79-83). The .merge on the next line drops the marker.

What the marker loss does, and what it does not do

The defect is in Gitlab::Graphql::Pagination::OffsetPaginatedRelation. When merge or unscope is called on a marked relation, the result is a plain ActiveRecord::Relation. It is no longer marked. Gitlab::Graphql::Pagination::Connections then selects a keyset connection for that relation instead of an offset connection.

No current caller reaches a keyset connection through this path. Resolvers::MergeRequestPipelinesResolver#query_for is the only place in the codebase that calls merge on a marked relation. That resolver includes CachingArrayResolver, which resolves to an Array, not a relation. The connection built for the pipelines field is a GraphQL::Pagination::ArrayConnection, so Gitlab::Graphql::Pagination::Keyset::Connection is never used there.

The fix is preventative, for the same reason the unscope override is preventative. It keeps the marker across relation methods that return a new relation, so a caller added later cannot silently get keyset pagination.

Why unscope is included

unscope has no caller on a marked relation today. Every .unscope( in app/, lib/ and ee/ runs on a plain relation inside a model or finder, before any GraphQL wrapping. Ci::PipelinesForMergeRequestFinder used to combine merge and unscope, but it was refactored in 51df5a072802 and dd2cf0655259 and no longer does.

It is added for symmetry with preload, includes and merge. The class exists so the marker survives relation chaining, and shipping only the method that happens to be called today would let the next chained call reintroduce the same defect. The cost is four lines and one spec.

How to set up and validate locally

In rails console:

marked = Gitlab::Graphql::Pagination::OffsetPaginatedRelation.new(Ci::Pipeline.merge_request_event_first)
merged = marked.merge(Ci::Pipeline.merge_request_event_first)

merged.class
Gitlab::Graphql::Pagination::Keyset::Connection.new(merged, max_page_size: 20, first: 2).nodes

On master, merged.class is Ci::Pipeline::ActiveRecord_Relation and the connection raises Gitlab::Pagination::Keyset::UnsupportedScopeOrder. With this change, merged.class is OffsetPaginatedRelation and the relation resolves through OffsetActiveRecordRelationConnection.

References

Closes #613643 (closed)

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.