Skip to content

Commit 3cc1f10

Browse files
wxiaoguangwxiaoguang
committed
fix merge
1 parent bf8db10 commit 3cc1f10

File tree

3 files changed

+60
-45
lines changed

3 files changed

+60
-45
lines changed

‎modules/web/handler.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ var argTypeProvider = map[reflect.Type]func(req *http.Request) ResponseStatusPro
2626
reflect.TypeOf(&context.PrivateContext{}): func(req *http.Request) ResponseStatusProvider { return context.GetPrivateContext(req) },
2727
}
2828

29+
func RegisterHandleTypeProvider[T any](fn func(req *http.Request) ResponseStatusProvider) {
30+
argTypeProvider[reflect.TypeOf((*T)(nil)).Elem()] = fn
31+
}
32+
2933
// responseWriter is a wrapper of http.ResponseWriter, to check whether the response has been written
3034
type responseWriter struct {
3135
respWriter http.ResponseWriter

‎routers/api/actions/artifacts.go

Lines changed: 55 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
package actions
55

6-
// Github Actions Artifacts API Simple Description
6+
// GitHub Actions Artifacts API Simple Description
77
//
88
// 1. Upload artifact
99
// 1.1. Post upload url
@@ -63,7 +63,6 @@ package actions
6363

6464
import (
6565
"compress/gzip"
66-
gocontext "context"
6766
"crypto/md5"
6867
"encoding/base64"
6968
"errors"
@@ -92,9 +91,25 @@ const (
9291

9392
const artifactRouteBase = "/_apis/pipelines/workflows/{run_id}/artifacts"
9493

95-
func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route {
94+
type artifactContextKeyType struct{}
95+
96+
var artifactContextKey = artifactContextKeyType{}
97+
98+
type ArtifactContext struct {
99+
*context.Base
100+
101+
ActionTask *actions.ActionTask
102+
}
103+
104+
func init() {
105+
web.RegisterHandleTypeProvider[*ArtifactContext](func(req *http.Request) web.ResponseStatusProvider {
106+
return req.Context().Value(artifactContextKey).(*ArtifactContext)
107+
})
108+
}
109+
110+
func ArtifactsRoutes(prefix string) *web.Route {
96111
m := web.NewRoute()
97-
m.Use(withContexter(goctx))
112+
m.Use(ArtifactContexter())
98113

99114
r := artifactRoutes{
100115
prefix: prefix,
@@ -115,15 +130,14 @@ func ArtifactsRoutes(goctx gocontext.Context, prefix string) *web.Route {
115130
return m
116131
}
117132

118-
// withContexter initializes a package context for a request.
119-
func withContexter(goctx gocontext.Context) func(next http.Handler) http.Handler {
133+
func ArtifactContexter() func(next http.Handler) http.Handler {
120134
return func(next http.Handler) http.Handler {
121135
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
122-
ctx := context.Context{
123-
Resp: context.NewResponse(resp),
124-
Data: map[string]interface{}{},
125-
}
126-
defer ctx.Close()
136+
base, baseCleanUp := context.NewBaseContext(resp, req)
137+
defer baseCleanUp()
138+
139+
ctx := &ArtifactContext{Base: base}
140+
ctx.AppendContextValue(artifactContextKey, ctx)
127141

128142
// action task call server api with Bearer ACTIONS_RUNTIME_TOKEN
129143
// we should verify the ACTIONS_RUNTIME_TOKEN
@@ -132,23 +146,22 @@ func withContexter(goctx gocontext.Context) func(next http.Handler) http.Handler
132146
ctx.Error(http.StatusUnauthorized, "Bad authorization header")
133147
return
134148
}
149+
135150
authToken := strings.TrimPrefix(authHeader, "Bearer ")
136151
task, err := actions.GetRunningTaskByToken(req.Context(), authToken)
137152
if err != nil {
138153
log.Error("Error runner api getting task: %v", err)
139154
ctx.Error(http.StatusInternalServerError, "Error runner api getting task")
140155
return
141156
}
142-
ctx.Data["task"] = task
143157

144-
if err := task.LoadJob(goctx); err != nil {
158+
if err := task.LoadJob(req.Context()); err != nil {
145159
log.Error("Error runner api getting job: %v", err)
146160
ctx.Error(http.StatusInternalServerError, "Error runner api getting job")
147161
return
148162
}
149163

150-
ctx.Req = context.WithContext(req, &ctx)
151-
164+
ctx.ActionTask = task
152165
next.ServeHTTP(ctx.Resp, ctx.Req)
153166
})
154167
}
@@ -175,13 +188,8 @@ type getUploadArtifactResponse struct {
175188
FileContainerResourceURL string `json:"fileContainerResourceUrl"`
176189
}
177190

178-
func (ar artifactRoutes) validateRunID(ctx *context.Context) (*actions.ActionTask, int64, bool) {
179-
task, ok := ctx.Data["task"].(*actions.ActionTask)
180-
if !ok {
181-
log.Error("Error getting task in context")
182-
ctx.Error(http.StatusInternalServerError, "Error getting task in context")
183-
return nil, 0, false
184-
}
191+
func (ar artifactRoutes) validateRunID(ctx *ArtifactContext) (*actions.ActionTask, int64, bool) {
192+
task := ctx.ActionTask
185193
runID := ctx.ParamsInt64("run_id")
186194
if task.Job.RunID != runID {
187195
log.Error("Error runID not match")
@@ -192,7 +200,7 @@ func (ar artifactRoutes) validateRunID(ctx *context.Context) (*actions.ActionTas
192200
}
193201

194202
// getUploadArtifactURL generates a URL for uploading an artifact
195-
func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) {
203+
func (ar artifactRoutes) getUploadArtifactURL(ctx *ArtifactContext) {
196204
task, runID, ok := ar.validateRunID(ctx)
197205
if !ok {
198206
return
@@ -220,7 +228,7 @@ func (ar artifactRoutes) getUploadArtifactURL(ctx *context.Context) {
220228

221229
// getUploadFileSize returns the size of the file to be uploaded.
222230
// The raw size is the size of the file as reported by the header X-TFS-FileLength.
223-
func (ar artifactRoutes) getUploadFileSize(ctx *context.Context) (int64, int64, error) {
231+
func (ar artifactRoutes) getUploadFileSize(ctx *ArtifactContext) (int64, int64, error) {
224232
contentLength := ctx.Req.ContentLength
225233
xTfsLength, _ := strconv.ParseInt(ctx.Req.Header.Get(artifactXTfsFileLengthHeader), 10, 64)
226234
if xTfsLength > 0 {
@@ -229,7 +237,7 @@ func (ar artifactRoutes) getUploadFileSize(ctx *context.Context) (int64, int64,
229237
return contentLength, contentLength, nil
230238
}
231239

232-
func (ar artifactRoutes) saveUploadChunk(ctx *context.Context,
240+
func (ar artifactRoutes) saveUploadChunk(ctx *ArtifactContext,
233241
artifact *actions.ActionArtifact,
234242
contentSize, runID int64,
235243
) (int64, error) {
@@ -273,7 +281,7 @@ func (ar artifactRoutes) saveUploadChunk(ctx *context.Context,
273281
// The rules are from https://github.com/actions/toolkit/blob/main/packages/artifact/src/internal/path-and-artifact-name-validation.ts#L32
274282
var invalidArtifactNameChars = strings.Join([]string{"\\", "/", "\"", ":", "<", ">", "|", "*", "?", "\r", "\n"}, "")
275283

276-
func (ar artifactRoutes) uploadArtifact(ctx *context.Context) {
284+
func (ar artifactRoutes) uploadArtifact(ctx *ArtifactContext) {
277285
_, runID, ok := ar.validateRunID(ctx)
278286
if !ok {
279287
return
@@ -341,7 +349,7 @@ func (ar artifactRoutes) uploadArtifact(ctx *context.Context) {
341349

342350
// comfirmUploadArtifact comfirm upload artifact.
343351
// if all chunks are uploaded, merge them to one file.
344-
func (ar artifactRoutes) comfirmUploadArtifact(ctx *context.Context) {
352+
func (ar artifactRoutes) comfirmUploadArtifact(ctx *ArtifactContext) {
345353
_, runID, ok := ar.validateRunID(ctx)
346354
if !ok {
347355
return
@@ -364,7 +372,7 @@ type chunkItem struct {
364372
Path string
365373
}
366374

367-
func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64) error {
375+
func (ar artifactRoutes) mergeArtifactChunks(ctx *ArtifactContext, runID int64) error {
368376
storageDir := fmt.Sprintf("tmp%d", runID)
369377
var chunks []*chunkItem
370378
if err := ar.fs.IterateObjects(storageDir, func(path string, obj storage.Object) error {
@@ -415,14 +423,20 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64)
415423

416424
// use multiReader
417425
readers := make([]io.Reader, 0, len(allChunks))
418-
readerClosers := make([]io.Closer, 0, len(allChunks))
426+
closeReaders := func() {
427+
for _, r := range readers {
428+
_ = r.(io.Closer).Close() // it guarantees to be io.Closer by the following loop's Open function
429+
}
430+
readers = nil
431+
}
432+
defer closeReaders()
433+
419434
for _, c := range allChunks {
420-
reader, err := ar.fs.Open(c.Path)
421-
if err != nil {
435+
var readCloser io.ReadCloser
436+
if readCloser, err = ar.fs.Open(c.Path); err != nil {
422437
return fmt.Errorf("open chunk error: %v, %s", err, c.Path)
423438
}
424-
readers = append(readers, reader)
425-
readerClosers = append(readerClosers, reader)
439+
readers = append(readers, readCloser)
426440
}
427441
mergedReader := io.MultiReader(readers...)
428442

@@ -445,11 +459,6 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64)
445459
return fmt.Errorf("merged file size is not equal to chunk length")
446460
}
447461

448-
// close readers
449-
for _, r := range readerClosers {
450-
r.Close()
451-
}
452-
453462
// save storage path to artifact
454463
log.Debug("[artifact] merge chunks to artifact: %d, %s", artifact.ID, storagePath)
455464
artifact.StoragePath = storagePath
@@ -458,6 +467,8 @@ func (ar artifactRoutes) mergeArtifactChunks(ctx *context.Context, runID int64)
458467
return fmt.Errorf("update artifact error: %v", err)
459468
}
460469

470+
closeReaders() // close before delete
471+
461472
// drop chunks
462473
for _, c := range cs {
463474
if err := ar.fs.Delete(c.Path); err != nil {
@@ -479,21 +490,21 @@ type (
479490
}
480491
)
481492

482-
func (ar artifactRoutes) listArtifacts(ctx *context.Context) {
493+
func (ar artifactRoutes) listArtifacts(ctx *ArtifactContext) {
483494
_, runID, ok := ar.validateRunID(ctx)
484495
if !ok {
485496
return
486497
}
487498

488-
artficats, err := actions.ListArtifactsByRunID(ctx, runID)
499+
artifacts, err := actions.ListArtifactsByRunID(ctx, runID)
489500
if err != nil {
490501
log.Error("Error getting artifacts: %v", err)
491502
ctx.Error(http.StatusInternalServerError, err.Error())
492503
return
493504
}
494505

495-
artficatsData := make([]listArtifactsResponseItem, 0, len(artficats))
496-
for _, a := range artficats {
506+
artficatsData := make([]listArtifactsResponseItem, 0, len(artifacts))
507+
for _, a := range artifacts {
497508
artficatsData = append(artficatsData, listArtifactsResponseItem{
498509
Name: a.ArtifactName,
499510
FileContainerResourceURL: ar.buildArtifactURL(runID, a.ID, "path"),
@@ -517,7 +528,7 @@ type (
517528
}
518529
)
519530

520-
func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) {
531+
func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) {
521532
_, runID, ok := ar.validateRunID(ctx)
522533
if !ok {
523534
return
@@ -546,7 +557,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *context.Context) {
546557
ctx.JSON(http.StatusOK, respData)
547558
}
548559

549-
func (ar artifactRoutes) downloadArtifact(ctx *context.Context) {
560+
func (ar artifactRoutes) downloadArtifact(ctx *ArtifactContext) {
550561
_, runID, ok := ar.validateRunID(ctx)
551562
if !ok {
552563
return

‎routers/init.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ func NormalRoutes(ctx context.Context) *web.Route {
198198
// In Github, it uses ACTIONS_RUNTIME_URL=https://pipelines.actions.githubusercontent.com/fLgcSHkPGySXeIFrg8W8OBSfeg3b5Fls1A1CwX566g8PayEGlg/
199199
// TODO: this prefix should be generated with a token string with runner ?
200200
prefix = "/api/actions_pipeline"
201-
r.Mount(prefix, actions_router.ArtifactsRoutes(ctx, prefix))
201+
r.Mount(prefix, actions_router.ArtifactsRoutes(prefix))
202202
}
203203

204204
return r

0 commit comments

Comments
 (0)