chore: adopt sealed dal.DB from dalgo v0.64.2#4
Merged
Conversation
dal.DB is now a sealed interface produced only by dal.NewDB(backend); the shape an adapter implements is the same method set, renamed dal.Backend. Retarget the compile-time assertions on githubDB and BatchingGitHubDB from dal.DB to dal.Backend, and wrap the *githubDB values returned by NewGitHubDB / NewGitHubDBWithDef in dal.NewDB(...) so callers keep getting a real dal.DB. BatchingGitHubDB embeds the concrete *githubDB (a Backend), not a dal.DB interface value, so it is a Backend composed over another Backend rather than a decorator over a sealed DB. It does not need to embed dal.DB, and NewBatchingGitHubDB keeps returning the concrete type unchanged: in-package tests reach into the embedded githubDB field directly, and nothing in this repo needs a sealed dal.DB from it. A caller that does can wrap it with dal.NewDB itself, same as for githubDB. One whitebox test recovered the concrete *githubDB via a direct type assertion on the dal.DB returned by NewGitHubDBWithDef; that assertion now targets dal.BackendOf(db) instead, since the returned value is the framework's validatedDB wrapper. Attempted to wire the shared dalgotest.RunConformance suite against the existing httptest Contents-API fake (no live GitHub credentials needed). It runs, but fails "record data is not map[string]any" on every write of a suite fixture: this adapter's write path only accepts pre-decoded map[string]any record data, while the suite writes typed structs. That is a pre-existing property of the adapter's data model, not something this migration introduced, so TestConformance documents the gap and skips rather than papering over it or silently omitting the wiring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkkrXdtf8mU2GRo2hHsHT1 Signed-off-by: Alexander Trakhimenok <alex@trakhimenok.com>
… for real
readwriteTx.Set / Insert and batchingTx.Set / Insert used to type-assert
record.Data() straight to map[string]any and reject anything else with a
bespoke error. Every write path now goes through record.DataToMap instead
(a no-op for the map[string]any this adapter always wrote, a JSON
round-trip for anything else), matching the dalgo2ingitdb sibling. That
lets TestConformance (added in the previous commit as a documented skip)
actually run the shared dalgotest.RunConformance suite instead of
skipping it outright.
15 of 16 checks now genuinely pass. The remaining one ("accepts a valid
record on UpdateRecord") is still skipped, but for a precise, narrow, and
pre-existing reason rather than the previous blanket one: the plain
githubDB reads through the eventually-consistent GitHub Contents API and
does not guarantee read-your-writes within a transaction, which is
exactly what that check needs (a Set immediately followed by a Get inside
one transaction). The package's own docs already call this out by name
as the reason BatchingGitHubDB exists; wiring the conformance factory to
BatchingGitHubDB instead would need a fuller Git Data API mock
(tree/blob/commit) than is warranted here, so the gap is left visible
rather than papered over.
Also makes the four Multi methods (SetMulti/DeleteMulti/UpdateMulti/
InsertMulti) wrap dal.ErrNotImplementedYet instead of returning a bare
error, so the conformance suite's "unsupported operation" tolerance
recognises them; the four whitebox tests asserting on the old exact
message now check errors.Is instead. The pre-existing invalid-record-data
tests (record.Data() is a plain string) now check for the
record.DataToMap conversion error instead of the old fixed message, since
that is what actually comes back now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SkkrXdtf8mU2GRo2hHsHT1
Signed-off-by: Alexander Trakhimenok <alex@trakhimenok.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
github.com/dal-go/dalgov0.63.1 → v0.64.2 (this repo had already migrated togithub.com/dal-go/recordin chore: migrate to dal-go/record #3; that migration is unaffected by this change).dal.DBis now sealed, produced only bydal.NewDB(backend); the adapter-facing method set is unchanged, renameddal.Backend.githubDBandBatchingGitHubDBassertions now targetdal.Backend;NewGitHubDB/NewGitHubDBWithDefwrap their*githubDBindal.NewDB(...)before returning.BatchingGitHubDBembeds the concrete*githubDB(a Backend), not adal.DBvalue, so it is a Backend composed over another Backend, not a decorator over a sealed DB — no embedding change needed, andNewBatchingGitHubDBkeeps returning the concrete type since in-package tests reach into the embeddedgithubDBfield directly.*githubDBviadb.(*githubDB); switched todal.BackendOf(db).(*githubDB)sincedbis now the framework'svalidatedDBwrapper.TestConformance, wiring the shareddalgotest.RunConformancesuite against the existing httptest Contents-API fake (no live GitHub credentials needed). Getting it to run for real surfaced that every write path type-assertedrecord.Data()straight tomap[string]any; switched Set/Insert (both the plain and batching tx) torecord.DataToMapinstead (a no-op for a map, a JSON round-trip for anything else — matching thedalgo2ingitdbsibling). 15 of 16 conformance checks now genuinely pass. The remaining one is skipped for a precise, pre-existing reason: the plaingithubDBreads via the eventually-consistent GitHub Contents API and doesn't guarantee read-your-writes within a transaction (exactly what that one check needs); the package's own docs already nameBatchingGitHubDBas the fix for this, but wiring the suite to it would need a fuller Git Data API mock than is warranted here.dal.ErrNotImplementedYetinstead of a bare error, so the suite's "unsupported operation" tolerance recognises them; four whitebox tests updated from exact-string toerrors.Isaccordingly. A few pre-existing invalid-record-data tests updated to match the new (more accurate)record.DataToMapconversion error.Test plan
gofmt -l .— cleanGOWORK=off go vet ./...— cleanGOWORK=off go test -race -count=1 ./...— all pass (TestConformanceis 15 real passes + 1 precisely-explained skip)🤖 Generated with Claude Code
https://claude.ai/code/session_01SkkrXdtf8mU2GRo2hHsHT1