diff --git a/models/actions/artifact.go b/models/actions/artifact.go index 95c4bead8f..492e3f6cea 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -108,6 +108,7 @@ func UpdateArtifactByID(ctx context.Context, id int64, art *ActionArtifact) erro type FindArtifactsOptions struct { db.ListOptions + ID int64 RepoID int64 RunID int64 ArtifactName string @@ -116,6 +117,9 @@ type FindArtifactsOptions struct { func (opts FindArtifactsOptions) ToConds() builder.Cond { cond := builder.NewCond() + if opts.ID > 0 { + cond = cond.And(builder.Eq{"id": opts.ID}) + } if opts.RepoID > 0 { cond = cond.And(builder.Eq{"repo_id": opts.RepoID}) } diff --git a/models/fixtures/action_artifact.yml b/models/fixtures/action_artifact.yml index 2c51c11ebd..a591719aee 100644 --- a/models/fixtures/action_artifact.yml +++ b/models/fixtures/action_artifact.yml @@ -57,7 +57,7 @@ run_id: 792 runner_id: 1 repo_id: 4 - owner_id: 1 + owner_id: 5 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 storage_path: "27/5/1730330775594233150.chunk" file_size: 1024 diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 260468f207..a325c51afe 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -668,9 +668,51 @@ func ArtifactsDeleteView(ctx *context_module.Context) { ctx.JSON(http.StatusOK, struct{}{}) } +func artifactsFind(ctx *context_module.Context, opts actions_model.FindArtifactsOptions) []*actions_model.ActionArtifact { + artifacts, err := db.Find[actions_model.ActionArtifact](ctx, opts) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return nil + } + if len(artifacts) == 0 { + return nil + } + return artifacts +} + +func artifactsFindByNameOrID(ctx *context_module.Context, runID int64, nameOrID string) []*actions_model.ActionArtifact { + artifacts := artifactsFind(ctx, actions_model.FindArtifactsOptions{ + RunID: runID, + ArtifactName: nameOrID, + }) + if ctx.Written() { + return nil + } + // if lookup by name found nothing, maybe it is an ID + if len(artifacts) == 0 { + id, err := strconv.ParseInt(nameOrID, 10, 64) + if err != nil || id == 0 { + ctx.Error(http.StatusNotFound, fmt.Sprintf("runID %d: artifact name not found: %v", runID, nameOrID)) + return nil + } + artifacts = artifactsFind(ctx, actions_model.FindArtifactsOptions{ + RunID: runID, + ID: id, + }) + if ctx.Written() { + return nil + } + if len(artifacts) == 0 { + ctx.Error(http.StatusNotFound, fmt.Sprintf("runID %d: artifact ID not found: %v", runID, nameOrID)) + return nil + } + } + return artifacts +} + func ArtifactsDownloadView(ctx *context_module.Context) { runIndex := ctx.ParamsInt64("run") - artifactName := ctx.Params("artifact_name") + artifactNameOrID := ctx.Params("artifact_name_or_id") run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex) if err != nil { @@ -682,16 +724,8 @@ func ArtifactsDownloadView(ctx *context_module.Context) { return } - artifacts, err := db.Find[actions_model.ActionArtifact](ctx, actions_model.FindArtifactsOptions{ - RunID: run.ID, - ArtifactName: artifactName, - }) - if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error()) - return - } - if len(artifacts) == 0 { - ctx.Error(http.StatusNotFound, "artifact not found") + artifacts := artifactsFindByNameOrID(ctx, run.ID, artifactNameOrID) + if ctx.Written() { return } @@ -720,12 +754,14 @@ func ArtifactsDownloadView(ctx *context_module.Context) { ctx.Error(http.StatusInternalServerError, err.Error()) return } - common.ServeContentByReadSeeker(ctx.Base, artifactName, util.ToPointer(art.UpdatedUnix.AsTime()), f) + common.ServeContentByReadSeeker(ctx.Base, artifacts[0].ArtifactName+".zip", util.ToPointer(art.UpdatedUnix.AsTime()), f) return } // Artifacts using the v1-v3 backend are stored as multiple individual files per artifact on the backend // Those need to be zipped for download + artifactName := artifacts[0].ArtifactName + ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(artifactName), artifactName)) writer := zip.NewWriter(ctx.Resp) defer writer.Close() diff --git a/routers/web/repo/actions/view_test.go b/routers/web/repo/actions/view_test.go new file mode 100644 index 0000000000..d6a390befc --- /dev/null +++ b/routers/web/repo/actions/view_test.go @@ -0,0 +1,95 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "fmt" + "testing" + + actions_model "forgejo.org/models/actions" + unittest "forgejo.org/models/unittest" + "forgejo.org/services/contexttest" + + "github.com/stretchr/testify/assert" +) + +func Test_artifactsFind(t *testing.T) { + unittest.PrepareTestEnv(t) + + for _, testCase := range []struct { + name string + artifactName string + count int + }{ + { + name: "Found", + artifactName: "artifact-v4-download", + count: 1, + }, + { + name: "NotFound", + artifactName: "notexist", + count: 0, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + runID := int64(792) + ctx, _ := contexttest.MockContext(t, fmt.Sprintf("user5/repo4/actions/runs/%v/artifacts/%v", runID, testCase.artifactName)) + artifacts := artifactsFind(ctx, actions_model.FindArtifactsOptions{ + RunID: runID, + ArtifactName: testCase.artifactName, + }) + assert.False(t, ctx.Written()) + assert.Len(t, artifacts, testCase.count) + }) + } +} + +func Test_artifactsFindByNameOrID(t *testing.T) { + unittest.PrepareTestEnv(t) + + for _, testCase := range []struct { + name string + nameOrID string + err string + }{ + { + name: "NameFound", + nameOrID: "artifact-v4-download", + }, + { + name: "NameNotFound", + nameOrID: "notexist", + err: "artifact name not found", + }, + { + name: "IDFound", + nameOrID: "22", + }, + { + name: "IDNotFound", + nameOrID: "666", + err: "artifact ID not found", + }, + { + name: "IDZeroNotFound", + nameOrID: "0", + err: "artifact name not found", + }, + } { + t.Run(testCase.name, func(t *testing.T) { + runID := int64(792) + ctx, resp := contexttest.MockContext(t, fmt.Sprintf("user5/repo4/actions/runs/%v/artifacts/%v", runID, testCase.nameOrID)) + artifacts := artifactsFindByNameOrID(ctx, runID, testCase.nameOrID) + if testCase.err == "" { + assert.NotEmpty(t, artifacts) + assert.False(t, ctx.Written(), resp.Body.String()) + } else { + assert.Empty(t, artifacts) + assert.True(t, ctx.Written()) + assert.Contains(t, resp.Body.String(), testCase.err) + } + }) + } +} diff --git a/routers/web/web.go b/routers/web/web.go index f1cbc6b7ad..a47ce2bff7 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1455,7 +1455,7 @@ func registerRoutes(m *web.Route) { m.Post("/cancel", reqRepoActionsWriter, actions.Cancel) m.Post("/approve", reqRepoActionsWriter, actions.Approve) m.Get("/artifacts", actions.ArtifactsView) - m.Get("/artifacts/{artifact_name}", actions.ArtifactsDownloadView) + m.Get("/artifacts/{artifact_name_or_id}", actions.ArtifactsDownloadView) m.Delete("/artifacts/{artifact_name}", reqRepoActionsWriter, actions.ArtifactsDeleteView) m.Post("/rerun", reqRepoActionsWriter, actions.Rerun) }) diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index 1526ae3585..4ecbbeb92c 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -267,6 +267,11 @@ func TestActionsArtifactDownloadMultiFiles(t *testing.T) { resp = MakeRequest(t, req, http.StatusOK) assert.Equal(t, strings.Repeat(bodyChar, 1024), resp.Body.String()) } + + // Download artifact via user-facing URL + req = NewRequest(t, "GET", "/user5/repo4/actions/runs/187/artifacts/multi-file-download") + resp = MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Header().Get("content-disposition"), "multi-file-download.zip") } func TestActionsArtifactUploadWithRetentionDays(t *testing.T) { diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index af0dcb98a9..716d9e1abe 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -344,6 +344,7 @@ func TestActionsArtifactV4DownloadSingle(t *testing.T) { req = NewRequest(t, "GET", "/user5/repo4/actions/runs/188/artifacts/artifact-v4-download") resp = MakeRequest(t, req, http.StatusOK) assert.Equal(t, "bytes", resp.Header().Get("accept-ranges")) + assert.Contains(t, resp.Header().Get("content-disposition"), "artifact-v4-download.zip") assert.Equal(t, body, resp.Body.String()) // Partial artifact download