From 832bfda89e285197e9e55f243b0ad4bb552ccc9a Mon Sep 17 00:00:00 2001 From: motoki317 Date: Thu, 18 May 2023 09:28:34 +0900 Subject: [PATCH 1/3] fix: save artifact correctly --- pkg/usecase/builder_service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/usecase/builder_service.go b/pkg/usecase/builder_service.go index 5ea14ee89..ec353c3f8 100644 --- a/pkg/usecase/builder_service.go +++ b/pkg/usecase/builder_service.go @@ -444,7 +444,7 @@ func (s *builderService) saveArtifact(ctx context.Context, st *state) error { return errors.Wrap(err, "opening artifact") } defer file.Close() - err = domain.SaveArtifact(s.storage, filename, file) + err = domain.SaveArtifact(s.storage, artifact.ID, file) if err != nil { return errors.Wrap(err, "saving artifact") } From becef750d62d1fb8ffc9c9d47dc30471045d7abe Mon Sep 17 00:00:00 2001 From: motoki317 Date: Thu, 18 May 2023 09:36:57 +0900 Subject: [PATCH 2/3] refactor: return filename from domain --- pkg/domain/storage.go | 16 ++++++++++------ pkg/infrastructure/grpc/api_app_build_service.go | 4 ++-- pkg/usecase/api_app_build_service.go | 2 +- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/domain/storage.go b/pkg/domain/storage.go index c83647cfc..e9ea8d389 100644 --- a/pkg/domain/storage.go +++ b/pkg/domain/storage.go @@ -51,9 +51,12 @@ func DeleteBuildLog(s Storage, buildID string) error { return nil } -func artifactPath(id string) string { - const artifactDirectory = "artifacts" - return filepath.Join(artifactDirectory, id+".tar") +func artifactPath(artifactID string) string { + return filepath.Join("artifacts", artifactFilename(artifactID)) +} + +func artifactFilename(artifactID string) string { + return artifactID + ".tar" } // SaveArtifact Artifactをtar形式で保存する @@ -64,13 +67,14 @@ func SaveArtifact(s Storage, artifactID string, src io.Reader) error { return nil } -func GetArtifact(s Storage, artifactID string) ([]byte, error) { +func GetArtifact(s Storage, artifactID string) (filename string, b []byte, err error) { r, err := s.Open(artifactPath(artifactID)) if err != nil { - return nil, errors.Wrap(err, "failed to open artifact") + return "", nil, errors.Wrap(err, "failed to open artifact") } defer r.Close() - return io.ReadAll(r) + b, err = io.ReadAll(r) + return artifactFilename(artifactID), b, err } func DeleteArtifact(s Storage, artifactID string) error { diff --git a/pkg/infrastructure/grpc/api_app_build_service.go b/pkg/infrastructure/grpc/api_app_build_service.go index 5078440cb..4f7611884 100644 --- a/pkg/infrastructure/grpc/api_app_build_service.go +++ b/pkg/infrastructure/grpc/api_app_build_service.go @@ -78,12 +78,12 @@ func (s *APIService) GetBuildLogStream(ctx context.Context, req *connect.Request } func (s *APIService) GetBuildArtifact(ctx context.Context, req *connect.Request[pb.ArtifactIdRequest]) (*connect.Response[pb.ArtifactContent], error) { - content, err := s.svc.GetArtifact(ctx, req.Msg.ArtifactId) + filename, content, err := s.svc.GetArtifact(ctx, req.Msg.ArtifactId) if err != nil { return nil, handleUseCaseError(err) } res := connect.NewResponse(&pb.ArtifactContent{ - Filename: req.Msg.ArtifactId + ".tar", + Filename: filename, Content: content, }) return res, nil diff --git a/pkg/usecase/api_app_build_service.go b/pkg/usecase/api_app_build_service.go index b4a091768..2387d1f04 100644 --- a/pkg/usecase/api_app_build_service.go +++ b/pkg/usecase/api_app_build_service.go @@ -79,6 +79,6 @@ func (s *APIServerService) GetBuildLogStream(ctx context.Context, buildID string return ch, nil } -func (s *APIServerService) GetArtifact(_ context.Context, artifactID string) ([]byte, error) { +func (s *APIServerService) GetArtifact(_ context.Context, artifactID string) (filename string, b []byte, err error) { return domain.GetArtifact(s.storage, artifactID) } From 08ae2cc414a0fb87a37d71bd524936486889c073 Mon Sep 17 00:00:00 2001 From: motoki317 Date: Thu, 18 May 2023 10:05:44 +0900 Subject: [PATCH 3/3] use gzip for artifacts --- pkg/domain/storage.go | 23 +++------------- .../grpc/api_app_build_service.go | 8 +++++- pkg/infrastructure/staticserver/builtin.go | 26 ++++++++++++++++--- pkg/usecase/api_app_build_service.go | 3 ++- pkg/usecase/builder_service.go | 22 +++++++++++++++- 5 files changed, 57 insertions(+), 25 deletions(-) diff --git a/pkg/domain/storage.go b/pkg/domain/storage.go index e9ea8d389..180f0329c 100644 --- a/pkg/domain/storage.go +++ b/pkg/domain/storage.go @@ -5,8 +5,6 @@ import ( "path/filepath" "github.com/friendsofgo/errors" - - "github.com/traPtitech/neoshowcase/pkg/util/tarfs" ) var ( @@ -56,7 +54,7 @@ func artifactPath(artifactID string) string { } func artifactFilename(artifactID string) string { - return artifactID + ".tar" + return artifactID + ".tar.gz" } // SaveArtifact Artifactをtar形式で保存する @@ -67,14 +65,12 @@ func SaveArtifact(s Storage, artifactID string, src io.Reader) error { return nil } -func GetArtifact(s Storage, artifactID string) (filename string, b []byte, err error) { - r, err := s.Open(artifactPath(artifactID)) +func GetArtifact(s Storage, artifactID string) (filename string, r io.ReadCloser, err error) { + r, err = s.Open(artifactPath(artifactID)) if err != nil { return "", nil, errors.Wrap(err, "failed to open artifact") } - defer r.Close() - b, err = io.ReadAll(r) - return artifactFilename(artifactID), b, err + return artifactFilename(artifactID), r, nil } func DeleteArtifact(s Storage, artifactID string) error { @@ -85,17 +81,6 @@ func DeleteArtifact(s Storage, artifactID string) error { return nil } -// ExtractTarToDir tarファイルをディレクトリに展開する -func ExtractTarToDir(s Storage, artifactID string, destPath string) error { - inputFile, err := s.Open(artifactPath(artifactID)) - if err != nil { - return errors.Wrap(err, "couldn't open source file") - } - defer inputFile.Close() - - return tarfs.Extract(inputFile, destPath) -} - type StorageConfig struct { Type string `mapstructure:"type" yaml:"type"` Local struct { diff --git a/pkg/infrastructure/grpc/api_app_build_service.go b/pkg/infrastructure/grpc/api_app_build_service.go index 4f7611884..18c963184 100644 --- a/pkg/infrastructure/grpc/api_app_build_service.go +++ b/pkg/infrastructure/grpc/api_app_build_service.go @@ -2,6 +2,7 @@ package grpc import ( "context" + "io" "github.com/bufbuild/connect-go" "github.com/friendsofgo/errors" @@ -78,10 +79,15 @@ func (s *APIService) GetBuildLogStream(ctx context.Context, req *connect.Request } func (s *APIService) GetBuildArtifact(ctx context.Context, req *connect.Request[pb.ArtifactIdRequest]) (*connect.Response[pb.ArtifactContent], error) { - filename, content, err := s.svc.GetArtifact(ctx, req.Msg.ArtifactId) + filename, r, err := s.svc.GetArtifact(ctx, req.Msg.ArtifactId) if err != nil { return nil, handleUseCaseError(err) } + defer r.Close() + content, err := io.ReadAll(r) + if err != nil { + return nil, err + } res := connect.NewResponse(&pb.ArtifactContent{ Filename: filename, Content: content, diff --git a/pkg/infrastructure/staticserver/builtin.go b/pkg/infrastructure/staticserver/builtin.go index 713330441..07fb12cb2 100644 --- a/pkg/infrastructure/staticserver/builtin.go +++ b/pkg/infrastructure/staticserver/builtin.go @@ -1,6 +1,7 @@ package staticserver import ( + "compress/gzip" "context" "fmt" "os" @@ -14,6 +15,7 @@ import ( "github.com/traPtitech/neoshowcase/pkg/domain" "github.com/traPtitech/neoshowcase/pkg/domain/web" + "github.com/traPtitech/neoshowcase/pkg/util/tarfs" ) type BuiltIn struct { @@ -121,10 +123,9 @@ func (b *BuiltIn) syncArtifacts(sites []*domain.StaticSite) error { if _, ok := currentArtifacts[artifactID]; ok { continue } - artifactDir := filepath.Join(b.docsRoot, artifactID) - err = domain.ExtractTarToDir(b.storage, artifactID, artifactDir) + err = b.extractArtifact(artifactID) if err != nil { - return errors.Wrap(err, "failed to extract artifact tar") + return err } } @@ -141,3 +142,22 @@ func (b *BuiltIn) syncArtifacts(sites []*domain.StaticSite) error { return nil } + +func (b *BuiltIn) extractArtifact(artifactID string) error { + destDir := filepath.Join(b.docsRoot, artifactID) + _, r, err := domain.GetArtifact(b.storage, artifactID) + if err != nil { + return errors.Wrap(err, "getting artifact") + } + defer r.Close() + tarReader, err := gzip.NewReader(r) + if err != nil { + return errors.Wrap(err, "preparing gzip reader") + } + defer tarReader.Close() + err = tarfs.Extract(tarReader, destDir) + if err != nil { + return errors.Wrap(err, "failed to extract artifact tar") + } + return nil +} diff --git a/pkg/usecase/api_app_build_service.go b/pkg/usecase/api_app_build_service.go index 2387d1f04..90380eb6d 100644 --- a/pkg/usecase/api_app_build_service.go +++ b/pkg/usecase/api_app_build_service.go @@ -2,6 +2,7 @@ package usecase import ( "context" + "io" "github.com/friendsofgo/errors" @@ -79,6 +80,6 @@ func (s *APIServerService) GetBuildLogStream(ctx context.Context, buildID string return ch, nil } -func (s *APIServerService) GetArtifact(_ context.Context, artifactID string) (filename string, b []byte, err error) { +func (s *APIServerService) GetArtifact(_ context.Context, artifactID string) (filename string, r io.ReadCloser, err error) { return domain.GetArtifact(s.storage, artifactID) } diff --git a/pkg/usecase/builder_service.go b/pkg/usecase/builder_service.go index ec353c3f8..c51b6eed6 100644 --- a/pkg/usecase/builder_service.go +++ b/pkg/usecase/builder_service.go @@ -2,6 +2,7 @@ package usecase import ( "bytes" + "compress/gzip" "context" "fmt" "io" @@ -444,7 +445,26 @@ func (s *builderService) saveArtifact(ctx context.Context, st *state) error { return errors.Wrap(err, "opening artifact") } defer file.Close() - err = domain.SaveArtifact(s.storage, artifact.ID, file) + + pr, pw := io.Pipe() + gzipWriter := gzip.NewWriter(pw) + if err != nil { + return errors.Wrap(err, "creating gzip stream") + } + go func() { + defer pw.Close() + _, err := io.Copy(gzipWriter, file) + if err != nil { + _ = pw.CloseWithError(errors.Wrap(err, "copying file to pipe writer")) + return + } + err = gzipWriter.Close() + if err != nil { + _ = pw.CloseWithError(errors.Wrap(err, "flushing gzip writer")) + return + } + }() + err = domain.SaveArtifact(s.storage, artifact.ID, pr) if err != nil { return errors.Wrap(err, "saving artifact") }