Skip to content

Commit 6b0fe79

Browse files
committed
refactor
1 parent 83fe746 commit 6b0fe79

4 files changed

Lines changed: 59 additions & 49 deletions

File tree

routers/api/actions/artifacts.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ func (ar artifactRoutes) uploadArtifact(ctx *ArtifactContext) {
241241
}
242242

243243
// get upload file size
244-
fileRealTotalSize, contentLength := getUploadFileSize(ctx)
244+
fileRealTotalSize := getUploadFileSize(ctx)
245245

246246
// get artifact retention days
247247
expiredDays := setting.Actions.ArtifactRetentionDays
@@ -265,17 +265,17 @@ func (ar artifactRoutes) uploadArtifact(ctx *ArtifactContext) {
265265
return
266266
}
267267

268-
// save chunk to storage, if success, return chunk stotal size
268+
// save chunk to storage, if success, return chunks total size
269269
// if artifact is not gzip when uploading, chunksTotalSize == fileRealTotalSize
270270
// if artifact is gzip when uploading, chunksTotalSize < fileRealTotalSize
271-
chunksTotalSize, err := saveUploadChunk(ar.fs, ctx, artifact, contentLength, runID)
271+
chunksTotalSize, err := saveUploadChunkGetTotalSize(ar.fs, ctx, artifact, runID)
272272
if err != nil {
273273
log.Error("Error save upload chunk: %v", err)
274274
ctx.HTTPError(http.StatusInternalServerError, "Error save upload chunk")
275275
return
276276
}
277277

278-
// update artifact size if zero or not match, over write artifact size
278+
// update artifact size if zero or not match, overwrite artifact size
279279
if artifact.FileSize == 0 ||
280280
artifact.FileCompressedSize == 0 ||
281281
artifact.FileSize != fileRealTotalSize ||

routers/api/actions/artifacts_chunks.go

Lines changed: 51 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,29 @@ import (
2121
"code.gitea.io/gitea/models/db"
2222
"code.gitea.io/gitea/modules/log"
2323
"code.gitea.io/gitea/modules/storage"
24-
"code.gitea.io/gitea/modules/util"
2524
)
2625

27-
func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext,
28-
artifact *actions.ActionArtifact,
29-
contentSize, runID, start, end, length int64, checkMd5 bool,
30-
) (int64, error) {
26+
type saveUploadChunkOptions struct {
27+
start int64
28+
end *int64
29+
checkMd5 bool
30+
}
31+
32+
func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact,
33+
runID int64, opts saveUploadChunkOptions,
34+
) (writtenSize int64, retErr error) {
3135
// build chunk store path
32-
storagePath := fmt.Sprintf("tmp%d/%d-%d-%d-%d.chunk", runID, runID, artifact.ID, start, end)
36+
storagePath := fmt.Sprintf("tmp%d/%d-%d-%d.chunk", runID, runID, artifact.ID, opts.start)
37+
38+
// "end" is optional, so "contentSize=-1" means read until EOF
39+
contentSize := int64(-1)
40+
if opts.end != nil {
41+
contentSize = *opts.end - opts.start + 1
42+
}
43+
3344
var r io.Reader = ctx.Req.Body
3445
var hasher hash.Hash
35-
if checkMd5 {
46+
if opts.checkMd5 {
3647
// use io.TeeReader to avoid reading all body to md5 sum.
3748
// it writes data to hasher after reading end
3849
// if hash is not matched, delete the read-end result
@@ -42,55 +53,55 @@ func saveUploadChunkBase(st storage.ObjectStorage, ctx *ArtifactContext,
4253
// save chunk to storage
4354
writtenSize, err := st.Save(storagePath, r, contentSize)
4455
if err != nil {
45-
return -1, fmt.Errorf("save chunk to storage error: %v", err)
56+
return 0, fmt.Errorf("save chunk to storage error: %v", err)
57+
}
58+
59+
defer func() {
60+
if retErr != nil {
61+
if err := st.Delete(storagePath); err != nil {
62+
log.Error("Error deleting chunk: %s, %v", storagePath, err)
63+
}
64+
}
65+
}()
66+
67+
if contentSize != -1 && writtenSize != contentSize {
68+
return writtenSize, fmt.Errorf("writtenSize %d does not match contentSize %d", writtenSize, contentSize)
4669
}
47-
var checkErr error
48-
if checkMd5 {
70+
if opts.checkMd5 {
4971
// check md5
5072
reqMd5String := ctx.Req.Header.Get(artifactXActionsResultsMD5Header)
5173
chunkMd5String := base64.StdEncoding.EncodeToString(hasher.Sum(nil))
52-
log.Info("[artifact] check chunk md5, sum: %s, header: %s", chunkMd5String, reqMd5String)
74+
log.Debug("[artifact] check chunk md5, sum: %s, header: %s", chunkMd5String, reqMd5String)
5375
// if md5 not match, delete the chunk
5476
if reqMd5String != chunkMd5String {
55-
checkErr = errors.New("md5 not match")
77+
return writtenSize, errors.New("md5 not match")
5678
}
5779
}
58-
if writtenSize != contentSize && contentSize != -1 {
59-
checkErr = errors.Join(checkErr, fmt.Errorf("writtenSize %d not match contentSize %d", writtenSize, contentSize))
60-
}
61-
if checkErr != nil {
62-
if err := st.Delete(storagePath); err != nil {
63-
log.Error("Error deleting chunk: %s, %v", storagePath, err)
64-
}
65-
return -1, checkErr
66-
}
67-
log.Info("[artifact] save chunk %s, size: %d, artifact id: %d, start: %d, end: %d",
68-
storagePath, writtenSize, artifact.ID, start, end)
69-
// return chunk total size if length is provided, otherwise return writtenSize
70-
return util.Iif(length != -1, length, writtenSize), nil
80+
log.Debug("[artifact] save chunk %s, size: %d, artifact id: %d, start: %d, size: %d", storagePath, writtenSize, artifact.ID, opts.start, contentSize)
81+
return writtenSize, nil
7182
}
7283

73-
func saveUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext,
74-
artifact *actions.ActionArtifact,
75-
contentSize, runID int64,
76-
) (int64, error) {
84+
func saveUploadChunkGetTotalSize(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID int64) (totalSize int64, _ error) {
7785
// parse content-range header, format: bytes 0-1023/146515
7886
contentRange := ctx.Req.Header.Get("Content-Range")
79-
start, end, length := int64(0), int64(0), int64(0)
80-
if _, err := fmt.Sscanf(contentRange, "bytes %d-%d/%d", &start, &end, &length); err != nil {
81-
log.Warn("parse content range error: %v, content-range: %s", err, contentRange)
82-
return -1, fmt.Errorf("parse content range error: %v", err)
87+
var start, end int64
88+
if _, err := fmt.Sscanf(contentRange, "bytes %d-%d/%d", &start, &end, &totalSize); err != nil {
89+
return 0, fmt.Errorf("parse content range error: %v", err)
90+
}
91+
_, err := saveUploadChunkBase(st, ctx, artifact, runID, saveUploadChunkOptions{start: start, end: new(end), checkMd5: true})
92+
if err != nil {
93+
return 0, err
8394
}
84-
return saveUploadChunkBase(st, ctx, artifact, contentSize, runID, start, end, length, true)
95+
return totalSize, nil
8596
}
8697

8798
// Returns uploaded length
88-
func appendUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext,
89-
artifact *actions.ActionArtifact,
90-
start, contentSize, runID int64,
91-
) (int64, error) {
92-
end := start + contentSize - 1
93-
return saveUploadChunkBase(st, ctx, artifact, contentSize, runID, start, end, -1, false)
99+
func appendUploadChunk(st storage.ObjectStorage, ctx *ArtifactContext, artifact *actions.ActionArtifact, runID, start int64) (int64, error) {
100+
opts := saveUploadChunkOptions{start: start, checkMd5: true}
101+
if ctx.Req.ContentLength > 0 {
102+
opts.end = new(start + ctx.Req.ContentLength - 1)
103+
}
104+
return saveUploadChunkBase(st, ctx, artifact, runID, opts)
94105
}
95106

96107
type chunkFileItem struct {

routers/api/actions/artifacts_utils.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,10 @@ func parseArtifactItemPath(ctx *ArtifactContext) (string, string, bool) {
8484

8585
// getUploadFileSize returns the size of the file to be uploaded.
8686
// The raw size is the size of the file as reported by the header X-TFS-FileLength.
87-
func getUploadFileSize(ctx *ArtifactContext) (int64, int64) {
88-
contentLength := ctx.Req.ContentLength
87+
func getUploadFileSize(ctx *ArtifactContext) int64 {
8988
xTfsLength, _ := strconv.ParseInt(ctx.Req.Header.Get(artifactXTfsFileLengthHeader), 10, 64)
9089
if xTfsLength > 0 {
91-
return xTfsLength, contentLength
90+
return xTfsLength
9291
}
93-
return contentLength, contentLength
92+
return ctx.Req.ContentLength
9493
}

routers/api/actions/artifactsv4.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ func (r *artifactV4Routes) uploadArtifact(ctx *ArtifactContext) {
319319
}
320320
blockid := ctx.Req.URL.Query().Get("blockid")
321321
if blockid == "" {
322-
uploadedLength, err := appendUploadChunk(r.fs, ctx, artifact, artifact.FileSize, ctx.Req.ContentLength, artifact.RunID)
322+
uploadedLength, err := appendUploadChunk(r.fs, ctx, artifact, artifact.RunID, artifact.FileSize)
323323
if err != nil {
324324
log.Error("Error appending Chunk %v", err)
325325
ctx.HTTPError(http.StatusInternalServerError, "Error appending Chunk")

0 commit comments

Comments
 (0)