From 46fa6d106c6f821b7e1b1f9ab099a6262635f335 Mon Sep 17 00:00:00 2001 From: "takahiro.tominaga" Date: Sun, 10 Sep 2023 18:44:20 +0900 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20defer=20=E3=81=A7=E5=91=BC=E3=81=B3?= =?UTF-8?q?=E5=87=BA=E3=81=97=E3=81=A6=E3=82=8B=E9=96=A2=E6=95=B0=E3=81=AE?= =?UTF-8?q?=E3=82=A8=E3=83=A9=E3=83=BC=E3=82=92=E3=83=8F=E3=83=B3=E3=83=89?= =?UTF-8?q?=E3=83=AA=E3=83=B3=E3=82=B0=E3=81=99=E3=82=8B=E3=82=88=E3=81=86?= =?UTF-8?q?=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- repository/database/member.go | 6 +++++- usecase/download.go | 39 ++++++++++++++++++++++++++++------- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/repository/database/member.go b/repository/database/member.go index 4cab6c0..ac9ad4c 100644 --- a/repository/database/member.go +++ b/repository/database/member.go @@ -43,7 +43,11 @@ func (d *database) ListMembers(ctx context.Context) ([]*model.Member, error) { if err != nil { return nil, fmt.Errorf("failed to conn.Querytext: %w", err) } - defer rows.Close() + defer func() { + if err := rows.Close(); err != nil { + d.logger.Warnf(ctx, "failed to rows.Close: ", err) + } + }() members := []*model.Member{} diff --git a/usecase/download.go b/usecase/download.go index 7d3ce80..8d96c94 100644 --- a/usecase/download.go +++ b/usecase/download.go @@ -4,6 +4,7 @@ import ( "archive/zip" "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -83,12 +84,18 @@ func (u *usecase) DownloadMembersZip(ctx context.Context, writer io.Writer) erro return nil } -func (u *usecase) writeMembersJSON(members []*model.Member, fullPath string) error { +func (u *usecase) writeMembersJSON(members []*model.Member, fullPath string) (err error) { file, err := os.Create(fullPath) if err != nil { return fmt.Errorf("failed to os.Create: %w", err) } - defer file.Close() + + defer func() { + closeErr := file.Close() + if closeErr != nil { + err = errors.Join(err, fmt.Errorf("failed to close json file: %w", closeErr)) + } + }() bytes, err := json.MarshalIndent(members, "", " ") if err != nil { @@ -103,7 +110,7 @@ func (u *usecase) writeMembersJSON(members []*model.Member, fullPath string) err return nil } -func (u *usecase) downloadImage(ctx context.Context, url, fullPath string) error { +func (u *usecase) downloadImage(ctx context.Context, url, fullPath string) (err error) { reader, closer, err := u.remote.GetImage(ctx, url) if err != nil { return fmt.Errorf("failed to GetImage: %w", err) @@ -116,7 +123,12 @@ func (u *usecase) downloadImage(ctx context.Context, url, fullPath string) error return fmt.Errorf("failed to os.Create: %w", err) } - defer file.Close() + defer func() { + closeErr := file.Close() + if closeErr != nil { + err = errors.Join(err, fmt.Errorf("failed to close img file: %w", closeErr)) + } + }() _, err = io.Copy(file, reader) if err != nil { @@ -155,12 +167,17 @@ func (u *usecase) initializeImgDir(imgDir string) error { } // targetDir 配下のファイルを再帰的に zip 化し、渡された writer に書き込む。 -func (u *usecase) createZip(targetDir string, readWriter io.Writer) error { +func (u *usecase) createZip(targetDir string, readWriter io.Writer) (err error) { zipWriter := zip.NewWriter(readWriter) - defer zipWriter.Close() + defer func() { + closeErr := zipWriter.Close() + if closeErr != nil { + err = errors.Join(err, fmt.Errorf("failed to close img file: %w", closeErr)) + } + }() // ディレクトリを再帰的に探索する。 - err := filepath.Walk(targetDir, func(path string, info os.FileInfo, err error) error { + err = filepath.Walk(targetDir, func(path string, info os.FileInfo, err error) error { if err != nil { return fmt.Errorf("failed to Walk: %w", err) } @@ -182,7 +199,13 @@ func (u *usecase) createZip(targetDir string, readWriter io.Writer) error { if err != nil { return fmt.Errorf("failed to Open: %w", err) } - defer fileToZip.Close() + + defer func() { + closeErr := fileToZip.Close() + if closeErr != nil { + err = errors.Join(err, closeErr) + } + }() fileToZipStat, err := fileToZip.Stat() if err != nil { From 34abc4509344bc271c20b3490c67d930cfe076c4 Mon Sep 17 00:00:00 2001 From: "takahiro.tominaga" Date: Sun, 10 Sep 2023 18:45:07 +0900 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20=E7=8B=AC=E8=87=AA=E3=81=A7=E8=BF=94?= =?UTF-8?q?=E3=81=97=E3=81=A6=E3=81=84=E3=82=8B=20closer=20=E3=81=AE?= =?UTF-8?q?=E3=82=A4=E3=83=B3=E3=82=BF=E3=83=95=E3=82=A7=E3=83=BC=E3=82=B9?= =?UTF-8?q?=E3=81=AE=E8=A6=8B=E7=9B=B4=E3=81=97?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- main.go | 12 ++++++++++-- repository/remote/image.go | 10 ++++------ repository/reoisitory.go | 4 ++-- usecase/download.go | 11 ++++++++--- usecase/download_test.go | 4 ++-- usecase/mock_repository_test.go | 4 ++-- util/logger/file_logger.go | 6 +++--- 7 files changed, 31 insertions(+), 20 deletions(-) diff --git a/main.go b/main.go index 296a030..3585578 100644 --- a/main.go +++ b/main.go @@ -25,14 +25,22 @@ func main() { log.Fatal("failed to initialize logger: ", err) } - defer fileCloser() + defer func() { + if err := fileCloser(); err != nil { + logger.Warnf(context.Background(), "failed to fileCloser: ", err) + } + }() tracer, traceCloser, err := util.NewJaegerTracer(cfg.Service) if err != nil { logger.Errorf(context.Background(), "cannot initialize jaeger tracer: ", err) } - defer traceCloser.Close() + defer func() { + if err := traceCloser.Close(); err != nil { + logger.Warnf(context.Background(), "failed to traceCloser: ", err) + } + }() opentracing.SetGlobalTracer(tracer) database, err := db.New(context.Background(), cfg, logger) diff --git a/repository/remote/image.go b/repository/remote/image.go index fc60371..b8a85cd 100644 --- a/repository/remote/image.go +++ b/repository/remote/image.go @@ -8,21 +8,19 @@ import ( "net/http" ) -func (r *remote) GetImage(ctx context.Context, url string) (io.Reader, func(), error) { +func (r *remote) GetImage(ctx context.Context, url string) (io.ReadCloser, error) { reader := bytes.NewReader([]byte{}) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, reader) if err != nil { - return nil, nil, fmt.Errorf("failed to NewRequestWithContext: %w", err) + return nil, fmt.Errorf("failed to NewRequestWithContext: %w", err) } //nolint:bodyclose resp, err := r.client.Do(req) if err != nil { - return nil, nil, fmt.Errorf("failed to client.Get: %w", err) + return nil, fmt.Errorf("failed to client.Get: %w", err) } - return resp.Body, func() { - resp.Body.Close() - }, nil + return resp.Body, nil } diff --git a/repository/reoisitory.go b/repository/reoisitory.go index eefaca6..3949bea 100644 --- a/repository/reoisitory.go +++ b/repository/reoisitory.go @@ -15,6 +15,6 @@ type Database interface { // HTTP 通信を伴う interface。 type Remote interface { - // 対象 URL の画像を byte 配列で取得する。 - GetImage(ctx context.Context, url string) (io.Reader, func(), error) + // 対象 URL の画像を取得する。 + GetImage(ctx context.Context, url string) (io.ReadCloser, error) } diff --git a/usecase/download.go b/usecase/download.go index 8d96c94..435135d 100644 --- a/usecase/download.go +++ b/usecase/download.go @@ -111,12 +111,17 @@ func (u *usecase) writeMembersJSON(members []*model.Member, fullPath string) (er } func (u *usecase) downloadImage(ctx context.Context, url, fullPath string) (err error) { - reader, closer, err := u.remote.GetImage(ctx, url) + readCloser, err := u.remote.GetImage(ctx, url) if err != nil { return fmt.Errorf("failed to GetImage: %w", err) } - defer closer() + defer func() { + closeErr := readCloser.Close() + if closeErr != nil { + err = errors.Join(err, fmt.Errorf("failed to close readCloser: %w", closeErr)) + } + }() file, err := os.Create(fullPath) if err != nil { @@ -130,7 +135,7 @@ func (u *usecase) downloadImage(ctx context.Context, url, fullPath string) (err } }() - _, err = io.Copy(file, reader) + _, err = io.Copy(file, readCloser) if err != nil { return fmt.Errorf("failed to io.Copy: %w", err) } diff --git a/usecase/download_test.go b/usecase/download_test.go index 7d0d9d4..add029e 100644 --- a/usecase/download_test.go +++ b/usecase/download_test.go @@ -54,12 +54,12 @@ func TestDownloadMembersZip(t *testing.T) { }, }, remote: &mockRemote{ - GetImageFunc: func(ctx context.Context, url string) (io.Reader, func(), error) { + GetImageFunc: func(ctx context.Context, url string) (io.ReadCloser, error) { data := imgBinary reader := bytes.NewReader(data) - return reader, func() {}, nil + return io.NopCloser(reader), nil }, }, }, diff --git a/usecase/mock_repository_test.go b/usecase/mock_repository_test.go index a03fcca..62b9808 100644 --- a/usecase/mock_repository_test.go +++ b/usecase/mock_repository_test.go @@ -16,9 +16,9 @@ func (m *mockDatabase) ListMembers(ctx context.Context) ([]*model.Member, error) } type mockRemote struct { - GetImageFunc func(ctx context.Context, url string) (io.Reader, func(), error) + GetImageFunc func(ctx context.Context, url string) (io.ReadCloser, error) } -func (m *mockRemote) GetImage(ctx context.Context, url string) (io.Reader, func(), error) { +func (m *mockRemote) GetImage(ctx context.Context, url string) (io.ReadCloser, error) { return m.GetImageFunc(ctx, url) } diff --git a/util/logger/file_logger.go b/util/logger/file_logger.go index f49bccd..c53cc5f 100644 --- a/util/logger/file_logger.go +++ b/util/logger/file_logger.go @@ -26,7 +26,7 @@ func NewFileLogger( path string, host string, service string, -) (Logger, func(), error) { +) (Logger, func() error, error) { //nolint:nosnakecase logfile, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o666) if err != nil { @@ -40,8 +40,8 @@ func NewFileLogger( service: service, } - return logger, func() { - logfile.Close() + return logger, func() error { + return logfile.Close() }, nil } From 7dd8c551cc7f4493a13cebb93e177a4d97b64ef8 Mon Sep 17 00:00:00 2001 From: "takahiro.tominaga" Date: Sun, 10 Sep 2023 19:11:38 +0900 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20lint=20=E5=AF=BE=E5=BF=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/main.yml | 2 +- .golangci.yml | 7 +++++++ go.mod | 2 +- repository/database/member.go | 1 + util/logger/file_logger.go | 1 + 5 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e3671f3..3b4c342 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -39,7 +39,7 @@ jobs: - name: Setup Go uses: actions/setup-go@v3 with: - go-version: 1.19 + go-version: 1.21 - name: Run Test run: | diff --git a/.golangci.yml b/.golangci.yml index 6861b64..69cbd56 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -6,6 +6,13 @@ linters-settings: settings: mnd: ignored-numbers: "0o666,0o777" + funlen: + lines: 77 + statements: 50 + cyclop: + skip-tests: true + max-complexity: 12 + package-average: 0 linters: enable-all: true disable: diff --git a/go.mod b/go.mod index 130d544..f1e1440 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/android-project-46group/core-api -go 1.19 +go 1.21 require ( github.com/DATA-DOG/go-sqlmock v1.5.0 diff --git a/repository/database/member.go b/repository/database/member.go index ac9ad4c..5d6afac 100644 --- a/repository/database/member.go +++ b/repository/database/member.go @@ -43,6 +43,7 @@ func (d *database) ListMembers(ctx context.Context) ([]*model.Member, error) { if err != nil { return nil, fmt.Errorf("failed to conn.Querytext: %w", err) } + defer func() { if err := rows.Close(); err != nil { d.logger.Warnf(ctx, "failed to rows.Close: ", err) diff --git a/util/logger/file_logger.go b/util/logger/file_logger.go index c53cc5f..34ec37e 100644 --- a/util/logger/file_logger.go +++ b/util/logger/file_logger.go @@ -41,6 +41,7 @@ func NewFileLogger( } return logger, func() error { + //nolint:wrapcheck return logfile.Close() }, nil }