diff --git a/batching.go b/batching.go index 6026c49..a567093 100644 --- a/batching.go +++ b/batching.go @@ -117,10 +117,13 @@ func (db *BatchingGitHubDB) RunReadwriteTransaction(ctx context.Context, f dal.R return nil } -// Compile-time check: BatchingGitHubDB satisfies dal.DB. The embedded +// Compile-time check: BatchingGitHubDB satisfies dal.Backend. The embedded // *githubDB supplies every method except the overridden -// RunReadwriteTransaction. -var _ dal.DB = (*BatchingGitHubDB)(nil) +// RunReadwriteTransaction. BatchingGitHubDB is itself a Backend composed over +// another Backend (githubDB), not a decorator over a sealed dal.DB — callers +// that need a sealed dal.DB wrap the result with dal.NewDB themselves, the +// same as they would for githubDB. +var _ dal.Backend = (*BatchingGitHubDB)(nil) // batchingTx implements dal.ReadwriteTransaction by buffering all writes. // @@ -155,9 +158,9 @@ func (t *batchingTx) Set(ctx context.Context, record dalrecord.Record) error { } recordPath := resolveRecordPath(colDef, recordKey) record.SetError(nil) - data, ok := record.Data().(map[string]any) - if !ok { - return fmt.Errorf("record data is not map[string]any") + data, err := dalrecord.DataToMap(record.Data()) + if err != nil { + return err } switch colDef.RecordFile.RecordType { case ingitdb.MapOfRecords: @@ -183,9 +186,9 @@ func (t *batchingTx) Insert(ctx context.Context, record dalrecord.Record, opts . return err } recordPath := resolveRecordPath(colDef, recordKey) - data, ok := record.Data().(map[string]any) - if !ok { - return fmt.Errorf("record data is not map[string]any") + data, err := dalrecord.DataToMap(record.Data()) + if err != nil { + return err } switch colDef.RecordFile.RecordType { case ingitdb.MapOfRecords: diff --git a/conformance_test.go b/conformance_test.go new file mode 100644 index 0000000..6b9bb08 --- /dev/null +++ b/conformance_test.go @@ -0,0 +1,65 @@ +package dalgo2ghingitdb + +import ( + "context" + "testing" + + "github.com/dal-go/dalgo/dal" + "github.com/dal-go/dalgo/dalgotest" +) + +// skippedConformanceCheck names a dalgotest.Checks() check this adapter +// cannot pass for a documented, pre-existing architectural reason (not +// something the dal.DB-sealing migration introduced), plus that reason. +const skippedConformanceCheck = "accepts a valid record on UpdateRecord" + +// TestConformance runs the shared dalgotest.RunConformance suite against a +// githubDB backed by the in-package httptest Contents API fake (no live +// GitHub repository or credentials required — see newGitHubContentsServer). +// +// The write path (readwriteTx.Set / Insert) converts record.Data() via +// record.DataToMap, matching the dalgo2ingitdb sibling: a no-op for the +// map[string]any this adapter always wrote before, and a JSON round-trip for +// the typed dalgotest.Record / dalgotest.Plain fixtures the suite writes. +// Before that conversion was in place, every write of a suite fixture failed +// "record data is not map[string]any", valid or invalid alike — confirmed by +// actually running the suite against the unconverted adapter. +// +// One check is skipped rather than run: see skippedConformanceCheck below. +// Every other check runs for real and must pass. +func TestConformance(t *testing.T) { + def := buildSingleRecordDef(dalgotest.DefaultCollection, "data/"+dalgotest.DefaultCollection, "{key}.yaml") + newDB := func(t *testing.T) (dal.DB, func()) { + server := newGitHubContentsServer(t, nil) + cfg := Config{Owner: "ingitdb", Repo: "ingitdb-cli", APIBaseURL: server.URL + "/"} + db, err := NewGitHubDBWithDef(cfg, def) + if err != nil { + t.Fatalf("NewGitHubDBWithDef: %v", err) + } + return db, server.Close + } + for _, check := range dalgotest.Checks() { + if check.Name == skippedConformanceCheck { + t.Run(check.Name, func(t *testing.T) { + t.Skip("the plain githubDB reads via the eventually-consistent GitHub Contents API " + + "(Repositories.GetContents) and does not guarantee read-your-writes within a " + + "transaction — see the ReadFile/consistent field doc in file_reader.go and the " + + "BatchingGitHubDB doc in batching.go, both of which call this out by name for " + + "exactly this reason. This check does a Set followed immediately by a Get (via " + + "UpdateRecord) in the same transaction, which only BatchingGitHubDB satisfies. " + + "Pointing this factory at BatchingGitHubDB would need a fuller Git Data API mock " + + "(tree/blob/commit, as batching_test.go builds inline) than is warranted here.") + }) + continue + } + t.Run(check.Name, func(t *testing.T) { + db, cleanup := newDB(t) + if cleanup != nil { + defer cleanup() + } + if err := check.Run(context.Background(), db); err != nil { + t.Error(err) + } + }) + } +} diff --git a/coverage_final_test.go b/coverage_final_test.go index 1c0401a..e3e6615 100644 --- a/coverage_final_test.go +++ b/coverage_final_test.go @@ -161,8 +161,8 @@ func TestBatchingTx_Set_InvalidRecordData(t *testing.T) { if err == nil { t.Fatal("Set: expected error for non-map data, got nil") } - if err.Error() != "record data is not map[string]any" { - t.Errorf("Set error = %q, want 'record data is not map[string]any'", err.Error()) + if !strings.Contains(err.Error(), "convert data of type string to map") { + t.Errorf("Set error = %q, want a record.DataToMap conversion error", err.Error()) } } @@ -190,8 +190,8 @@ func TestBatchingTx_Insert_InvalidRecordData(t *testing.T) { if err == nil { t.Fatal("Insert: expected error for non-map data, got nil") } - if err.Error() != "record data is not map[string]any" { - t.Errorf("Insert error = %q, want 'record data is not map[string]any'", err.Error()) + if !strings.Contains(err.Error(), "convert data of type string to map") { + t.Errorf("Insert error = %q, want a record.DataToMap conversion error", err.Error()) } } @@ -627,7 +627,7 @@ func TestReadwriteTx_Delete_MapOfRecords_EncodeError(t *testing.T) { if err != nil { t.Fatalf("NewGitHubDBWithDef: %v", err) } - concreteDB := db.(*githubDB) + concreteDB := dal.BackendOf(db).(*githubDB) ctx := context.Background() tx := readwriteTx{ diff --git a/db_github.go b/db_github.go index 16a791a..c87fb7a 100644 --- a/db_github.go +++ b/db_github.go @@ -15,7 +15,7 @@ import ( "github.com/ingitdb/ingitdb-go/ingitdb" ) -var _ dal.DB = (*githubDB)(nil) +var _ dal.Backend = (*githubDB)(nil) // NewGitHubDB creates a GitHub repository adapter. // Note: Definition is required for most operations, so prefer NewGitHubDBWithDef. @@ -28,7 +28,7 @@ func NewGitHubDB(cfg Config) (dal.DB, error) { cfg: cfg, fileReader: reader.(*githubFileReader), } - return db, nil + return dal.NewDB(db), nil } func NewGitHubDBWithDef(cfg Config, def *ingitdb.Definition) (dal.DB, error) { @@ -44,7 +44,7 @@ func NewGitHubDBWithDef(cfg Config, def *ingitdb.Definition) (dal.DB, error) { def: def, fileReader: reader.(*githubFileReader), } - return db, nil + return dal.NewDB(db), nil } type githubDB struct { diff --git a/error_paths_test.go b/error_paths_test.go index 9554754..6c5a173 100644 --- a/error_paths_test.go +++ b/error_paths_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "net/http/httptest" + "strings" "testing" "github.com/dal-go/dalgo/dal" @@ -49,9 +50,8 @@ func TestReadwriteTx_Set_InvalidRecordData(t *testing.T) { if err == nil { t.Fatal("Set() expected error for invalid record data, got nil") } - expectedMsg := "record data is not map[string]any" - if err.Error() != expectedMsg { - t.Errorf("Set() error = %q, want %q", err.Error(), expectedMsg) + if !strings.Contains(err.Error(), "convert data of type string to map") { + t.Errorf("Set() error = %q, want a record.DataToMap conversion error", err.Error()) } } @@ -92,9 +92,8 @@ func TestReadwriteTx_Set_MapOfRecords_InvalidRecordData(t *testing.T) { if err == nil { t.Fatal("Set() expected error for invalid record data, got nil") } - expectedMsg := "record data is not map[string]any" - if err.Error() != expectedMsg { - t.Errorf("Set() error = %q, want %q", err.Error(), expectedMsg) + if !strings.Contains(err.Error(), "convert data of type string to map") { + t.Errorf("Set() error = %q, want a record.DataToMap conversion error", err.Error()) } } @@ -131,9 +130,8 @@ func TestReadwriteTx_Insert_InvalidRecordData(t *testing.T) { if err == nil { t.Fatal("Insert() expected error for invalid record data, got nil") } - expectedMsg := "record data is not map[string]any" - if err.Error() != expectedMsg { - t.Errorf("Insert() error = %q, want %q", err.Error(), expectedMsg) + if !strings.Contains(err.Error(), "convert data of type string to map") { + t.Errorf("Insert() error = %q, want a record.DataToMap conversion error", err.Error()) } } @@ -174,9 +172,8 @@ func TestReadwriteTx_Insert_MapOfRecords_InvalidRecordData(t *testing.T) { if err == nil { t.Fatal("Insert() expected error for invalid record data, got nil") } - expectedMsg := "record data is not map[string]any" - if err.Error() != expectedMsg { - t.Errorf("Insert() error = %q, want %q", err.Error(), expectedMsg) + if !strings.Contains(err.Error(), "convert data of type string to map") { + t.Errorf("Insert() error = %q, want a record.DataToMap conversion error", err.Error()) } } diff --git a/go.mod b/go.mod index f43cfcf..11e310f 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module github.com/ingitdb/dalgo2ingitdb4github go 1.26.0 require ( - github.com/dal-go/dalgo v0.63.1 - github.com/dal-go/record v0.1.0 + github.com/dal-go/dalgo v0.64.2 + github.com/dal-go/record v0.1.1 github.com/google/go-github/v88 v88.0.0 github.com/ingitdb/ingitdb-go/ingitdb v0.5.2 github.com/pelletier/go-toml/v2 v2.4.3 @@ -12,7 +12,7 @@ require ( ) require ( - github.com/RoaringBitmap/roaring/v2 v2.22.0 // indirect + github.com/RoaringBitmap/roaring/v2 v2.24.0 // indirect github.com/bits-and-blooms/bitset v1.24.6 // indirect github.com/google/go-querystring v1.2.0 // indirect github.com/ingr-io/ingr-go v0.0.2 // indirect diff --git a/go.sum b/go.sum index c4a5260..dec31e2 100644 --- a/go.sum +++ b/go.sum @@ -1,12 +1,12 @@ -github.com/RoaringBitmap/roaring/v2 v2.22.0 h1:aGqjvTSkJSTP7W6q518EHiK9RRRb5gJbCaaciCFr/Lg= -github.com/RoaringBitmap/roaring/v2 v2.22.0/go.mod h1:SfT3of9nYh3vis1dIbCj4Yw6KQGujTN+f345nrN/0JA= +github.com/RoaringBitmap/roaring/v2 v2.24.0 h1:zQkkBZtG3WRP4j+P3A5DO221SvL1Br88TJkhyqEQRZo= +github.com/RoaringBitmap/roaring/v2 v2.24.0/go.mod h1:SfT3of9nYh3vis1dIbCj4Yw6KQGujTN+f345nrN/0JA= github.com/bits-and-blooms/bitset v1.24.6 h1:qcrftZUVBIwfs+m+nhoCBAPT+ZPZZjti8SbHbDQQkZ4= github.com/bits-and-blooms/bitset v1.24.6/go.mod h1:7hO7Gc7Pp1vODcmWvKMRA9BNmbv6a/7QIWpPxHddWR8= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/dal-go/dalgo v0.63.1 h1:GEJAGlNH5xGLdFasSIRrYdqGfd0+4A9DGQ843qJQbyA= -github.com/dal-go/dalgo v0.63.1/go.mod h1:LtD5XVzb1kAdXaRcWVNy4F2ROC4fqR4jqD3q/GD4fJQ= -github.com/dal-go/record v0.1.0 h1:hA4143oZwIgtBH/1BRTUEZMCpDfXQr3ONlVXX8USCNg= -github.com/dal-go/record v0.1.0/go.mod h1:quwsVJTT0f6y3Mhx+yHpTobY7luX1M6kyO6fdJ/AFYE= +github.com/dal-go/dalgo v0.64.2 h1:uWCRISCMTpuwjq+VKlymq3kBtdr0c09gcsr75zmeqrw= +github.com/dal-go/dalgo v0.64.2/go.mod h1:PZGzE0AqnaJgPEDTSo7ayfKZaoglhFQ9FxHecF3aQAc= +github.com/dal-go/record v0.1.1 h1:N2WVDBnm2tOb83h5DJqFyINB5kH0vLjKlYFAgCJWv2g= +github.com/dal-go/record v0.1.1/go.mod h1:quwsVJTT0f6y3Mhx+yHpTobY7luX1M6kyO6fdJ/AFYE= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= diff --git a/tx_readwrite.go b/tx_readwrite.go index cd46719..2c7ee76 100644 --- a/tx_readwrite.go +++ b/tx_readwrite.go @@ -49,9 +49,9 @@ func (r readwriteTx) Set(ctx context.Context, record dalrecord.Record) error { return parseErr } } - data, ok := record.Data().(map[string]any) - if !ok { - return fmt.Errorf("record data is not map[string]any") + data, convErr := dalrecord.DataToMap(record.Data()) + if convErr != nil { + return convErr } allRecords[recordKey] = ingitdb.ApplyLocaleToWrite(data, colDef.Columns) encoded, encodeErr := ingitdb.EncodeMapOfRecordsContent( @@ -69,9 +69,9 @@ func (r readwriteTx) Set(ctx context.Context, record dalrecord.Record) error { if readErr != nil { return readErr } - data, ok := record.Data().(map[string]any) - if !ok { - return fmt.Errorf("record data is not map[string]any") + data, convErr := dalrecord.DataToMap(record.Data()) + if convErr != nil { + return convErr } encoded, encodeErr := encodeRecordContent(data, colDef.RecordFile.Format) if encodeErr != nil { @@ -116,9 +116,9 @@ func (r readwriteTx) Insert(ctx context.Context, record dalrecord.Record, opts . return fmt.Errorf("record already exists: %s/%s", colDef.ID, recordKey) } record.SetError(nil) - data, ok := record.Data().(map[string]any) - if !ok { - return fmt.Errorf("record data is not map[string]any") + data, convErr := dalrecord.DataToMap(record.Data()) + if convErr != nil { + return convErr } allRecords[recordKey] = ingitdb.ApplyLocaleToWrite(data, colDef.Columns) encoded, encodeErr := ingitdb.EncodeMapOfRecordsContent( @@ -140,9 +140,9 @@ func (r readwriteTx) Insert(ctx context.Context, record dalrecord.Record, opts . return fmt.Errorf("record already exists: %s/%s", colDef.ID, recordKey) } record.SetError(nil) - data, ok := record.Data().(map[string]any) - if !ok { - return fmt.Errorf("record data is not map[string]any") + data, convErr := dalrecord.DataToMap(record.Data()) + if convErr != nil { + return convErr } encoded, encodeErr := encodeRecordContent(data, colDef.RecordFile.Format) if encodeErr != nil { @@ -217,12 +217,12 @@ func (r readwriteTx) Delete(ctx context.Context, key *dalrecord.Key) error { func (r readwriteTx) SetMulti(ctx context.Context, records []dalrecord.Record) error { _, _ = ctx, records - return fmt.Errorf("not implemented by %s", DatabaseID) + return fmt.Errorf("%w: not implemented by %s", dal.ErrNotImplementedYet, DatabaseID) } func (r readwriteTx) DeleteMulti(ctx context.Context, keys []*dalrecord.Key) error { _, _ = ctx, keys - return fmt.Errorf("not implemented by %s", DatabaseID) + return fmt.Errorf("%w: not implemented by %s", dal.ErrNotImplementedYet, DatabaseID) } // Update applies field-level updates by reading the record, mutating it in @@ -259,12 +259,12 @@ func (r readwriteTx) UpdateRecord(ctx context.Context, record dalrecord.Record, func (r readwriteTx) UpdateMulti(ctx context.Context, keys []*dalrecord.Key, updates []update.Update, preconditions ...dal.Precondition) error { _, _, _, _ = ctx, keys, updates, preconditions - return fmt.Errorf("not implemented by %s", DatabaseID) + return fmt.Errorf("%w: not implemented by %s", dal.ErrNotImplementedYet, DatabaseID) } func (r readwriteTx) InsertMulti(ctx context.Context, records []dalrecord.Record, opts ...dal.InsertOption) error { _, _, _ = ctx, records, opts - return fmt.Errorf("not implemented by %s", DatabaseID) + return fmt.Errorf("%w: not implemented by %s", dal.ErrNotImplementedYet, DatabaseID) } func (r readwriteTx) ID() string { diff --git a/tx_readwrite_test.go b/tx_readwrite_test.go index 2cc5050..b4c8efc 100644 --- a/tx_readwrite_test.go +++ b/tx_readwrite_test.go @@ -2,7 +2,7 @@ package dalgo2ghingitdb import ( "context" - "fmt" + "errors" "testing" "github.com/dal-go/dalgo/dal" @@ -46,9 +46,8 @@ func TestReadwriteTx_SetMulti(t *testing.T) { if setMultiErr == nil { t.Fatal("SetMulti() expected error, got nil") } - expectedMsg := fmt.Sprintf("not implemented by %s", DatabaseID) - if setMultiErr.Error() != expectedMsg { - t.Errorf("SetMulti() error = %q, want %q", setMultiErr.Error(), expectedMsg) + if !errors.Is(setMultiErr, dal.ErrNotImplementedYet) { + t.Errorf("SetMulti() error = %v, want dal.ErrNotImplementedYet", setMultiErr) } return nil }) @@ -72,9 +71,8 @@ func TestReadwriteTx_DeleteMulti(t *testing.T) { if deleteMultiErr == nil { t.Fatal("DeleteMulti() expected error, got nil") } - expectedMsg := fmt.Sprintf("not implemented by %s", DatabaseID) - if deleteMultiErr.Error() != expectedMsg { - t.Errorf("DeleteMulti() error = %q, want %q", deleteMultiErr.Error(), expectedMsg) + if !errors.Is(deleteMultiErr, dal.ErrNotImplementedYet) { + t.Errorf("DeleteMulti() error = %v, want dal.ErrNotImplementedYet", deleteMultiErr) } return nil }) @@ -146,9 +144,8 @@ func TestReadwriteTx_UpdateMulti(t *testing.T) { if updateMultiErr == nil { t.Fatal("UpdateMulti() expected error, got nil") } - expectedMsg := fmt.Sprintf("not implemented by %s", DatabaseID) - if updateMultiErr.Error() != expectedMsg { - t.Errorf("UpdateMulti() error = %q, want %q", updateMultiErr.Error(), expectedMsg) + if !errors.Is(updateMultiErr, dal.ErrNotImplementedYet) { + t.Errorf("UpdateMulti() error = %v, want dal.ErrNotImplementedYet", updateMultiErr) } return nil }) @@ -172,9 +169,8 @@ func TestReadwriteTx_InsertMulti(t *testing.T) { if insertMultiErr == nil { t.Fatal("InsertMulti() expected error, got nil") } - expectedMsg := fmt.Sprintf("not implemented by %s", DatabaseID) - if insertMultiErr.Error() != expectedMsg { - t.Errorf("InsertMulti() error = %q, want %q", insertMultiErr.Error(), expectedMsg) + if !errors.Is(insertMultiErr, dal.ErrNotImplementedYet) { + t.Errorf("InsertMulti() error = %v, want dal.ErrNotImplementedYet", insertMultiErr) } return nil })