From ad0a34b492c3d41952ff4648c8bfb7b54c376151 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 25 Feb 2024 14:05:23 +0100 Subject: [PATCH] Add `io.Closer` guidelines (#29387) Co-authored-by: Yarden Shoham --- docs/content/contributing/guidelines-backend.en-us.md | 4 ++++ modules/git/blame.go | 4 ++++ modules/git/repo_base_gogit.go | 7 ++++--- modules/git/repo_base_nogogit.go | 4 ++-- modules/indexer/internal/bleve/indexer.go | 2 +- modules/indexer/internal/meilisearch/indexer.go | 3 --- modules/util/filebuffer/file_backed_buffer.go | 1 + 7 files changed, 16 insertions(+), 9 deletions(-) diff --git a/docs/content/contributing/guidelines-backend.en-us.md b/docs/content/contributing/guidelines-backend.en-us.md index 084b3886e..3159a5ff7 100644 --- a/docs/content/contributing/guidelines-backend.en-us.md +++ b/docs/content/contributing/guidelines-backend.en-us.md @@ -101,6 +101,10 @@ i.e. `services/user`, `models/repository`. Since there are some packages which use the same package name, it is possible that you find packages like `modules/user`, `models/user`, and `services/user`. When these packages are imported in one Go file, it's difficult to know which package we are using and if it's a variable name or an import name. So, we always recommend to use import aliases. To differ from package variables which are commonly in camelCase, just use **snake_case** for import aliases. i.e. `import user_service "code.gitea.io/gitea/services/user"` +### Implementing `io.Closer` + +If a type implements `io.Closer`, calling `Close` multiple times must not fail or `panic` but return an error or `nil`. + ### Important Gotchas - Never write `x.Update(exemplar)` without an explicit `WHERE` clause: diff --git a/modules/git/blame.go b/modules/git/blame.go index 64095a218..69e1b08f9 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -115,6 +115,10 @@ func (r *BlameReader) NextPart() (*BlamePart, error) { // Close BlameReader - don't run NextPart after invoking that func (r *BlameReader) Close() error { + if r.bufferedReader == nil { + return nil + } + err := <-r.done r.bufferedReader = nil _ = r.reader.Close() diff --git a/modules/git/repo_base_gogit.go b/modules/git/repo_base_gogit.go index 9270bb70f..3ca5eb36c 100644 --- a/modules/git/repo_base_gogit.go +++ b/modules/git/repo_base_gogit.go @@ -88,16 +88,17 @@ func OpenRepository(ctx context.Context, repoPath string) (*Repository, error) { } // Close this repository, in particular close the underlying gogitStorage if this is not nil -func (repo *Repository) Close() (err error) { +func (repo *Repository) Close() error { if repo == nil || repo.gogitStorage == nil { - return + return nil } if err := repo.gogitStorage.Close(); err != nil { gitealog.Error("Error closing storage: %v", err) } + repo.gogitStorage = nil repo.LastCommitCache = nil repo.tagCache = nil - return + return nil } // GoGitRepo gets the go-git repo representation diff --git a/modules/git/repo_base_nogogit.go b/modules/git/repo_base_nogogit.go index 8c6eae589..86b6a9356 100644 --- a/modules/git/repo_base_nogogit.go +++ b/modules/git/repo_base_nogogit.go @@ -103,7 +103,7 @@ func (repo *Repository) CatFileBatchCheck(ctx context.Context) (WriteCloserError } } -func (repo *Repository) Close() (err error) { +func (repo *Repository) Close() error { if repo == nil { return nil } @@ -123,5 +123,5 @@ func (repo *Repository) Close() (err error) { } repo.LastCommitCache = nil repo.tagCache = nil - return err + return nil } diff --git a/modules/indexer/internal/bleve/indexer.go b/modules/indexer/internal/bleve/indexer.go index ce06b5afc..01e53ca63 100644 --- a/modules/indexer/internal/bleve/indexer.go +++ b/modules/indexer/internal/bleve/indexer.go @@ -92,7 +92,7 @@ func (i *Indexer) Ping(_ context.Context) error { } func (i *Indexer) Close() { - if i == nil { + if i == nil || i.Indexer == nil { return } diff --git a/modules/indexer/internal/meilisearch/indexer.go b/modules/indexer/internal/meilisearch/indexer.go index b037249d4..f4004849c 100644 --- a/modules/indexer/internal/meilisearch/indexer.go +++ b/modules/indexer/internal/meilisearch/indexer.go @@ -87,8 +87,5 @@ func (i *Indexer) Close() { if i == nil { return } - if i.Client == nil { - return - } i.Client = nil } diff --git a/modules/util/filebuffer/file_backed_buffer.go b/modules/util/filebuffer/file_backed_buffer.go index 6b07bd041..739543e29 100644 --- a/modules/util/filebuffer/file_backed_buffer.go +++ b/modules/util/filebuffer/file_backed_buffer.go @@ -149,6 +149,7 @@ func (b *FileBackedBuffer) Close() error { if b.file != nil { err := b.file.Close() os.Remove(b.file.Name()) + b.file = nil return err } return nil