diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index 6c77c51..206345f 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -1,6 +1,11 @@ package artifact -import "context" +import ( + "context" + "errors" +) + +var ErrNotFound = errors.New("artifact not found") type Key struct { Module string @@ -21,7 +26,7 @@ type Artifact struct { } type Store interface { - Get(ctx context.Context, key Key) (Artifact, bool, error) + Get(ctx context.Context, key Key) (Artifact, error) Put(ctx context.Context, key Key, artifact Artifact) (Artifact, error) Delete(ctx context.Context, key Key) error } diff --git a/internal/artifact/gorm.go b/internal/artifact/gorm.go index 7d12b50..effef96 100644 --- a/internal/artifact/gorm.go +++ b/internal/artifact/gorm.go @@ -38,7 +38,7 @@ func NewGormStore(db *gorm.DB) (*GormStore, error) { return &GormStore{db: db}, nil } -func (s *GormStore) Get(ctx context.Context, key Key) (Artifact, bool, error) { +func (s *GormStore) Get(ctx context.Context, key Key) (Artifact, error) { return s.get(ctx, key) } @@ -83,18 +83,18 @@ func (s *GormStore) Delete(ctx context.Context, key Key) error { return nil } -func (s *GormStore) get(ctx context.Context, key Key) (Artifact, bool, error) { +func (s *GormStore) get(ctx context.Context, key Key) (Artifact, error) { var record artifactRecord err := s.db.WithContext(ctx). Where("module = ? AND version = ? AND matrix_str = ?", key.Module, key.Version, key.MatrixStr). First(&record).Error if errors.Is(err, gorm.ErrRecordNotFound) { - return Artifact{}, false, nil + return Artifact{}, ErrNotFound } if err != nil { - return Artifact{}, false, fmt.Errorf("get artifact: %w", err) + return Artifact{}, fmt.Errorf("get artifact: %w", err) } - return record.artifact(), true, nil + return record.artifact(), nil } func (r artifactRecord) artifact() Artifact { diff --git a/internal/artifact/gorm_test.go b/internal/artifact/gorm_test.go index 032be94..cdeda90 100644 --- a/internal/artifact/gorm_test.go +++ b/internal/artifact/gorm_test.go @@ -3,6 +3,7 @@ package artifact import ( "context" "database/sql" + "errors" "strings" "sync" "testing" @@ -25,12 +26,8 @@ func TestGormStoreGetMissAndPut(t *testing.T) { Checksum: "abc", } - got, ok, err := store.Get(ctx, key) - if err != nil { - t.Fatalf("Get miss: %v", err) - } - if ok { - t.Fatalf("Get miss ok = true, artifact = %+v", got) + if got, err := store.Get(ctx, key); !errors.Is(err, ErrNotFound) { + t.Fatalf("Get miss = %+v, %v; want ErrNotFound", got, err) } inserted, err := store.Put(ctx, key, want) @@ -41,13 +38,10 @@ func TestGormStoreGetMissAndPut(t *testing.T) { t.Fatalf("Put = %+v, want %+v", inserted, want) } - got, ok, err = store.Get(ctx, key) + got, err := store.Get(ctx, key) if err != nil { t.Fatalf("Get hit: %v", err) } - if !ok { - t.Fatalf("Get hit ok = false") - } if got != want { t.Fatalf("Get = %+v, want %+v", got, want) } @@ -149,10 +143,8 @@ func TestGormStoreDelete(t *testing.T) { if err := store.Delete(ctx, key); err != nil { t.Fatalf("Delete: %v", err) } - if got, ok, err := store.Get(ctx, key); err != nil { - t.Fatalf("Get after Delete: %v", err) - } else if ok { - t.Fatalf("Get after Delete ok = true, artifact = %+v", got) + if got, err := store.Get(ctx, key); !errors.Is(err, ErrNotFound) { + t.Fatalf("Get after Delete = %+v, %v; want ErrNotFound", got, err) } } @@ -183,7 +175,7 @@ func TestGormStoreDatabaseErrors(t *testing.T) { Metadata: "-lz", Checksum: "abc", } - if _, _, err := store.Get(ctx, key); err == nil { + if _, err := store.Get(ctx, key); err == nil { t.Fatal("Get with closed database = nil, want error") } if _, err := store.Put(ctx, key, value); err == nil { diff --git a/internal/artifact/kodo.go b/internal/artifact/kodo.go index b4170eb..87832a0 100644 --- a/internal/artifact/kodo.go +++ b/internal/artifact/kodo.go @@ -5,7 +5,6 @@ import ( "encoding/base64" "encoding/json" "errors" - "fmt" "strings" qiniuclient "github.com/qiniu/go-sdk/v7/client" @@ -43,20 +42,20 @@ func NewKodoArtifact(cfg KodoArtifactConfig) Store { } } -func (s *kodoArtifact) Get(ctx context.Context, key Key) (Artifact, bool, error) { +func (s *kodoArtifact) Get(ctx context.Context, key Key) (Artifact, error) { objectName := s.objectName(key) object, err := s.objects.Bucket(s.bucket).Object(objectName).Stat().Call(ctx) if err != nil { if kodoArtifactObjectNotFound(err) { - return Artifact{}, false, nil + return Artifact{}, ErrNotFound } - return Artifact{}, false, err + return Artifact{}, err } got, ok := kodoArtifactFromMetadata(object.Metadata) if !ok { - return Artifact{}, false, fmt.Errorf("read kodo artifact metadata for %s", objectName) + return Artifact{}, ErrNotFound } - return got, true, nil + return got, nil } func (s *kodoArtifact) Put(ctx context.Context, key Key, art Artifact) (Artifact, error) { diff --git a/internal/artifact/kodo_test.go b/internal/artifact/kodo_test.go index b33390f..bc1fccf 100644 --- a/internal/artifact/kodo_test.go +++ b/internal/artifact/kodo_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "fmt" "net/http" "net/http/httptest" @@ -102,13 +103,10 @@ func TestKodoArtifactGetPutDeleteWithFakeKodo(t *testing.T) { t.Fatalf("Put = %+v, want %+v", got, want) } - got, ok, err := store.Get(context.Background(), key) + got, err = store.Get(context.Background(), key) if err != nil { t.Fatalf("Get: %v", err) } - if !ok { - t.Fatal("Get missed object") - } if got != want { t.Fatalf("Get = %+v, want %+v", got, want) } @@ -126,12 +124,8 @@ func TestKodoArtifactGetErrors(t *testing.T) { defer server.Close() store := newFakeKodoArtifactStore(server.URL, "bucket", "") - got, ok, err := store.Get(context.Background(), key) - if err != nil { - t.Fatalf("Get: %v", err) - } - if ok { - t.Fatalf("Get = %+v, true; want miss", got) + if got, err := store.Get(context.Background(), key); !errors.Is(err, ErrNotFound) { + t.Fatalf("Get = %+v, %v; want ErrNotFound", got, err) } }) @@ -141,8 +135,8 @@ func TestKodoArtifactGetErrors(t *testing.T) { defer server.Close() store := newFakeKodoArtifactStore(server.URL, "bucket", "") - if got, ok, err := store.Get(context.Background(), key); err == nil { - t.Fatalf("Get = %+v, %v, nil; want metadata error", got, ok) + if got, err := store.Get(context.Background(), key); !errors.Is(err, ErrNotFound) { + t.Fatalf("Get = %+v, %v; want ErrNotFound", got, err) } }) } @@ -235,13 +229,10 @@ func TestKodoArtifactE2E(t *testing.T) { t.Fatalf("Put = %+v, want %+v", got, want) } - got, ok, err := store.Get(ctx, key) + got, err = store.Get(ctx, key) if err != nil { t.Fatalf("Get: %v", err) } - if !ok { - t.Fatal("Get missed uploaded object") - } if got != want { t.Fatalf("Get = %+v, want %+v", got, want) }