diff --git a/routers/web/devtest/mock_actions.go b/routers/web/devtest/mock_actions.go index f5daf70f3f..89dd25ca29 100644 --- a/routers/web/devtest/mock_actions.go +++ b/routers/web/devtest/mock_actions.go @@ -17,6 +17,7 @@ import ( "time" actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" @@ -36,6 +37,106 @@ var mockActionsArtifactFiles = map[string][]mockArtifactFile{ Content: "artifact-b report", }, }, + "artifact-lcov-coverage": { + { + Path: "coverage/index.html", + Content: ` + + + + Coverage Report + + + +

LCOV Coverage Report

+

Generated from mock fixture artifact.

+ + + + + + + + + + + + + + + + + + + + + + + +
FileLinesBranchesFunctions
web_src/js/components/RepoActionView.vue100.0% (7/7)75.0% (3/4)100.0% (3/3)
web_src/js/features/repo-actions.ts100.0% (5/5)n/a100.0% (2/2)
+

Open raw lcov.info

+ +`, + }, + { + Path: "coverage/lcov.info", + Content: `TN: +SF:web_src/js/components/RepoActionView.vue +FN:33,artifactBaseURL +FN:37,artifactPreviewURL +FN:41,deleteArtifact +FNF:3 +FNH:3 +FNDA:9,artifactBaseURL +FNDA:8,artifactPreviewURL +FNDA:2,deleteArtifact +DA:33,9 +DA:34,9 +DA:37,8 +DA:38,8 +DA:41,2 +DA:42,2 +DA:43,2 +LF:7 +LH:7 +BRDA:131,0,0,1 +BRDA:131,0,1,3 +BRDA:140,1,0,1 +BRDA:140,1,1,0 +BRF:4 +BRH:3 +end_of_record +TN: +SF:web_src/js/features/repo-actions.ts +FN:12,loadRunView +FN:61,fetchRunArtifacts +FNF:2 +FNH:2 +FNDA:5,loadRunView +FNDA:5,fetchRunArtifacts +DA:12,5 +DA:13,5 +DA:14,5 +DA:61,5 +DA:62,5 +LF:5 +LH:5 +BRF:0 +BRH:0 +end_of_record +`, + }, + { + Path: "coverage/summary.txt", + Content: "HTML coverage fixture with linked lcov.info and realistic function/line/branch records.", + }, + }, "artifact-really-loooooooooooooooooooooooooooooooooooooooooooooooooooooooong": { { Path: "index.html", @@ -160,6 +261,11 @@ func MockActionsRunsJobs(ctx *context.Context) { Size: 1024 * 1024, Status: "completed", }) + resp.Artifacts = append(resp.Artifacts, &actions.ArtifactsViewItem{ + Name: "artifact-lcov-coverage", + Size: 256 * 1024, + Status: "completed", + }) resp.Artifacts = append(resp.Artifacts, &actions.ArtifactsViewItem{ Name: "artifact-very-loooooooooooooooooooooooooooooooooooooooooooooooooooooooong", Size: 100 * 1024, @@ -291,7 +397,7 @@ func MockActionsArtifactDownload(ctx *context.Context) { return } - ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(artifactName), artifactName)) + ctx.Resp.Header().Set("Content-Disposition", httplib.EncodeContentDispositionAttachment(artifactName+".zip")) writer := zip.NewWriter(ctx.Resp) defer writer.Close() for _, file := range files { diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 378b0caec5..c39f7c0e38 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -135,6 +135,24 @@ type ArtifactPreviewFile struct { Selected bool } +const ( + artifactPreviewV4ZipListCacheTTL = 10 * time.Minute + artifactPreviewV4ZipListCacheMaxEntries = 128 +) + +type artifactPreviewV4ZipListCacheEntry struct { + paths []string + expiresAt time.Time +} + +var artifactPreviewV4ZipListCache = struct { + mu sync.Mutex + entries map[string]artifactPreviewV4ZipListCacheEntry + order []string +}{ + entries: map[string]artifactPreviewV4ZipListCacheEntry{}, +} + type readAtBySeeker struct { rs io.ReadSeeker mu sync.Mutex @@ -841,19 +859,84 @@ func listArtifactV4ZipFiles(reader *zip.Reader) ([]string, map[string]*zip.File) } func listPreviewPathsForV4Artifact(artifact *actions_model.ActionArtifact) ([]string, error) { + if paths, ok := getArtifactPreviewV4ZipListFromCache(artifact); ok { + return paths, nil + } + obj, reader, err := openArtifactV4ZipReader(artifact) if err != nil { if errors.Is(err, zip.ErrFormat) { - return []string{artifactPreviewFallbackPath(artifact)}, nil + paths := []string{artifactPreviewFallbackPath(artifact)} + setArtifactPreviewV4ZipListCache(artifact, paths) + return paths, nil } return nil, err } defer obj.Close() paths, _ := listArtifactV4ZipFiles(reader) + setArtifactPreviewV4ZipListCache(artifact, paths) return paths, nil } +func artifactPreviewV4ZipListCacheKey(artifact *actions_model.ActionArtifact) string { + return strconv.FormatInt(artifact.ID, 10) + ":" + strconv.FormatInt(int64(artifact.UpdatedUnix), 10) + ":" + artifact.StoragePath +} + +func removeArtifactPreviewV4ZipListCacheOrderKey(order []string, key string) []string { + for i, current := range order { + if current != key { + continue + } + return append(order[:i], order[i+1:]...) + } + return order +} + +func getArtifactPreviewV4ZipListFromCache(artifact *actions_model.ActionArtifact) ([]string, bool) { + key := artifactPreviewV4ZipListCacheKey(artifact) + + artifactPreviewV4ZipListCache.mu.Lock() + entry, ok := artifactPreviewV4ZipListCache.entries[key] + if !ok { + artifactPreviewV4ZipListCache.mu.Unlock() + return nil, false + } + if time.Now().After(entry.expiresAt) { + delete(artifactPreviewV4ZipListCache.entries, key) + artifactPreviewV4ZipListCache.order = removeArtifactPreviewV4ZipListCacheOrderKey(artifactPreviewV4ZipListCache.order, key) + artifactPreviewV4ZipListCache.mu.Unlock() + return nil, false + } + paths := append([]string(nil), entry.paths...) + artifactPreviewV4ZipListCache.mu.Unlock() + return paths, true +} + +func setArtifactPreviewV4ZipListCache(artifact *actions_model.ActionArtifact, paths []string) { + key := artifactPreviewV4ZipListCacheKey(artifact) + + artifactPreviewV4ZipListCache.mu.Lock() + defer artifactPreviewV4ZipListCache.mu.Unlock() + + if _, ok := artifactPreviewV4ZipListCache.entries[key]; !ok { + artifactPreviewV4ZipListCache.order = append(artifactPreviewV4ZipListCache.order, key) + } + artifactPreviewV4ZipListCache.entries[key] = artifactPreviewV4ZipListCacheEntry{ + paths: append([]string(nil), paths...), + expiresAt: time.Now().Add(artifactPreviewV4ZipListCacheTTL), + } + + for len(artifactPreviewV4ZipListCache.entries) > artifactPreviewV4ZipListCacheMaxEntries && len(artifactPreviewV4ZipListCache.order) > 0 { + oldestKey := artifactPreviewV4ZipListCache.order[0] + artifactPreviewV4ZipListCache.order = artifactPreviewV4ZipListCache.order[1:] + if _, ok := artifactPreviewV4ZipListCache.entries[oldestKey]; !ok { + continue + } + delete(artifactPreviewV4ZipListCache.entries, oldestKey) + } +} + func listPreviewPaths(artifacts []*actions_model.ActionArtifact) ([]string, error) { if len(artifacts) == 1 && actions.IsArtifactV4(artifacts[0]) { return listPreviewPathsForV4Artifact(artifacts[0]) @@ -1081,7 +1164,7 @@ func ArtifactsDownloadView(ctx *context_module.Context) { return } - ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(artifactName), artifactName)) + ctx.Resp.Header().Set("Content-Disposition", httplib.EncodeContentDispositionAttachment(artifactName+".zip")) if len(artifacts) == 1 && actions.IsArtifactV4(artifacts[0]) { err := actions.DownloadArtifactV4(ctx.Base, artifacts[0]) if err != nil { @@ -1093,7 +1176,6 @@ func ArtifactsDownloadView(ctx *context_module.Context) { // Artifacts using the v1-v3 backend are stored as multiple individual files per artifact on the backend // Those need to be zipped for download - ctx.Resp.Header().Set("Content-Disposition", httplib.EncodeContentDispositionAttachment(artifactName+".zip")) zipWriter := zip.NewWriter(ctx.Resp) defer zipWriter.Close() diff --git a/routers/web/repo/actions/view_test.go b/routers/web/repo/actions/view_test.go index 7296ea6849..3455e88a30 100644 --- a/routers/web/repo/actions/view_test.go +++ b/routers/web/repo/actions/view_test.go @@ -5,6 +5,7 @@ package actions import ( "testing" + "time" actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/modules/timeutil" @@ -45,3 +46,90 @@ func TestConvertToViewModel(t *testing.T) { } assert.Equal(t, expectedViewJobs, viewJobSteps) } + +func resetArtifactPreviewV4ZipListCacheForTest() { + artifactPreviewV4ZipListCache.mu.Lock() + defer artifactPreviewV4ZipListCache.mu.Unlock() + artifactPreviewV4ZipListCache.entries = map[string]artifactPreviewV4ZipListCacheEntry{} + artifactPreviewV4ZipListCache.order = nil +} + +func TestArtifactPreviewV4ZipListCacheSetGet(t *testing.T) { + resetArtifactPreviewV4ZipListCacheForTest() + + artifact := &actions_model.ActionArtifact{ + ID: 1, + UpdatedUnix: timeutil.TimeStamp(2), + StoragePath: "artifact/path.zip", + } + paths := []string{"index.html", "logs/output.txt"} + setArtifactPreviewV4ZipListCache(artifact, paths) + + paths[0] = "changed" + got, ok := getArtifactPreviewV4ZipListFromCache(artifact) + require.True(t, ok) + assert.Equal(t, []string{"index.html", "logs/output.txt"}, got) + + got[0] = "changed-again" + got2, ok := getArtifactPreviewV4ZipListFromCache(artifact) + require.True(t, ok) + assert.Equal(t, []string{"index.html", "logs/output.txt"}, got2) +} + +func TestArtifactPreviewV4ZipListCacheExpires(t *testing.T) { + resetArtifactPreviewV4ZipListCacheForTest() + + artifact := &actions_model.ActionArtifact{ + ID: 2, + UpdatedUnix: timeutil.TimeStamp(3), + StoragePath: "artifact/expired.zip", + } + key := artifactPreviewV4ZipListCacheKey(artifact) + + artifactPreviewV4ZipListCache.mu.Lock() + artifactPreviewV4ZipListCache.entries[key] = artifactPreviewV4ZipListCacheEntry{ + paths: []string{"expired.txt"}, + expiresAt: time.Now().Add(-time.Second), + } + artifactPreviewV4ZipListCache.order = []string{key} + artifactPreviewV4ZipListCache.mu.Unlock() + + _, ok := getArtifactPreviewV4ZipListFromCache(artifact) + require.False(t, ok) + + artifactPreviewV4ZipListCache.mu.Lock() + _, exists := artifactPreviewV4ZipListCache.entries[key] + order := append([]string(nil), artifactPreviewV4ZipListCache.order...) + artifactPreviewV4ZipListCache.mu.Unlock() + assert.False(t, exists) + assert.NotContains(t, order, key) +} + +func TestArtifactPreviewV4ZipListCacheEvictsOldest(t *testing.T) { + resetArtifactPreviewV4ZipListCacheForTest() + + for i := range artifactPreviewV4ZipListCacheMaxEntries + 1 { + artifact := &actions_model.ActionArtifact{ + ID: int64(i + 1), + UpdatedUnix: timeutil.TimeStamp(i + 1), + StoragePath: "artifact/cache-entry.zip", + } + setArtifactPreviewV4ZipListCache(artifact, []string{"file.txt"}) + } + + oldest := &actions_model.ActionArtifact{ + ID: 1, + UpdatedUnix: timeutil.TimeStamp(1), + StoragePath: "artifact/cache-entry.zip", + } + _, ok := getArtifactPreviewV4ZipListFromCache(oldest) + assert.False(t, ok) + + newest := &actions_model.ActionArtifact{ + ID: artifactPreviewV4ZipListCacheMaxEntries + 1, + UpdatedUnix: timeutil.TimeStamp(artifactPreviewV4ZipListCacheMaxEntries + 1), + StoragePath: "artifact/cache-entry.zip", + } + _, ok = getArtifactPreviewV4ZipListFromCache(newest) + assert.True(t, ok) +} diff --git a/templates/shared/actions/artifact_preview_content.tmpl b/templates/shared/actions/artifact_preview_content.tmpl index dd16648574..1fc13fca2e 100644 --- a/templates/shared/actions/artifact_preview_content.tmpl +++ b/templates/shared/actions/artifact_preview_content.tmpl @@ -7,10 +7,6 @@ min-width: 0; } -.artifact-preview-subtitle { - color: var(--color-text-light-3); -} - .artifact-preview-frame { width: 100%; min-height: 70vh; @@ -22,14 +18,17 @@
{{ctx.Locale.Tr "preview"}}: {{.ArtifactName}} -
- {{ctx.Locale.Tr "go_back"}} -
- - {{svg "octicon-download"}} - {{ctx.Locale.Tr "repo.download_file"}} - +
+ + {{svg "octicon-arrow-left"}} + {{ctx.Locale.Tr "go_back"}} + + + {{svg "octicon-download"}} + {{ctx.Locale.Tr "repo.download_file"}} + +
diff --git a/tests/integration/actions_artifact_preview_test.go b/tests/integration/actions_artifact_preview_test.go index 7df2dc708d..def760463c 100644 --- a/tests/integration/actions_artifact_preview_test.go +++ b/tests/integration/actions_artifact_preview_test.go @@ -36,6 +36,8 @@ func TestActionsArtifactPreviewSingleFile(t *testing.T) { resp := session.MakeRequest(t, req, http.StatusOK) assert.Contains(t, resp.Body.String(), "abc.txt") assert.Contains(t, resp.Body.String(), "/preview/raw/abc.txt") + assert.Contains(t, resp.Body.String(), "sandbox=\"allow-scripts\"") + assert.Contains(t, resp.Body.String(), "referrerpolicy=\"no-referrer\"") req = NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/artifact-download/preview/raw", repo.FullName()) resp = session.MakeRequest(t, req, http.StatusOK) @@ -54,7 +56,11 @@ func TestActionsArtifactPreviewMultiFile(t *testing.T) { assert.Contains(t, resp.Body.String(), "abc.txt") assert.Contains(t, resp.Body.String(), "xyz/def.txt") - req = NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/multi-file-download/preview/raw?path=%s", repo.FullName(), url.QueryEscape("xyz/def.txt")) + req = NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/multi-file-download/preview?path=%s", repo.FullName(), url.QueryEscape("xyz/def.txt")) + resp = session.MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), "/preview/raw/xyz/def.txt") + + req = NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/multi-file-download/preview/raw/xyz/def.txt", repo.FullName()) resp = session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, strings.Repeat("C", 1024), resp.Body.String()) }