Refactor build artifact serialization
Summary
The legacy code for representing build artifacts assumes that users will only interact (download, browse, and preserve) directly with archive artifacts. This paradigm is changing: #23847 (closed)
As a result of !32811 (merged), BuildArtifactEntity
has become out of date. Previously its object
was a Ci::Build
. It is now a Ci::JobArtifact
.
This change makes this entity the wrong place to represent the keep_path
and browse_path
attributes, since they both act on all of a build's artifacts.
Context
The following discussion from !32811 (merged) should be addressed:
-
@dosuken123 started a discussion: (+2 comments) This path only works for
archive
-type artifact, right? Does it make sense to expose it for the other type?Maybe
expose :keep_path, if: -> (*) { artifact.archive? && artifact.expiring? }
? This would be more explicit.
In the future, we should support keep action for any artifact types.
Possible solutions
- Remove
keep_path
andbrowse_path
fromBuildArtifactEntity
so all of its attributes are only associated with the single artifact that it's representing. Create aBuildArtifactsEntity
that has aCi::Build
as itsobject
and represents the collection of the build's artifacts.keep_path
andbrowse_path
will become attributes onBuildArtifactsEntity
. - Since updating the entities will require both frontend and backend changes, this may be a good time to migrate the features that use
BuildArtifactEntity
to GraphQL.
Removal Proposal (current leading candidate)
These paths seem to be already exposed in the BuildDetailsEntity. Being that the action calls methods defined on the Ci::Build class and the path helpers accept project
and job
params:
# /*namespace_id/:project_id/-/jobs/:job_id/artifacts/keep(.:format)
def fast_keep_project_job_artifacts_path(project, job)
expose_fast_artifacts_path(project, job, :keep)
end
# /*namespace_id/:project_id/-/jobs/:job_id/artifacts/browse(/*path)
def fast_browse_project_job_artifacts_path(project, job)
expose_fast_artifacts_path(project, job, :browse)
end
The BuildDetailsEntity
is a fine place for those fields to be exposed, and we can avoid introducing a new entity. Simply removing these fields from BuildArtifactEntity
and refactoring the GitLab frontend (this is internal API only) to find the field on BuildDetails instead of on each BuildArtifact achieves what we want here.
Potential follow-up: BuildArtifactEntity
uses some performance-optimized path helpers that we might use in BuildDetailsEntity