feat(storage)!: typed options, and let Update write only the named fields

[Lutherwaves]

Fixes #261 and #263. Stacked on #258 — review that first; this branch is based on it, so the diff here is only the second commit.

Problem

Update writes every field of the item it is given. Two callers that each read a record and then change different fields both write back the values they read for the fields they never touched, so the second write silently reverts the first. Neither caller gets an error.

WithFields lets a caller that knows which fields changed say so.

err := adapter.Update(&task, filter, storage.WithFields("title", "modified_at"))

Why this also replaces the params map

The option could have been another key in params ...map[string]any. That map was the wrong place to put anything else:

• it is stringly typed, so the compiler checks neither the key nor the value type;
• it does not appear in the godoc of the operations that read it;
• Update ignored it entirely on SQL while CosmosDB read "pk_field" and "pk_value" out of it as bare string literals.

Adding a second typed mechanism beside it would leave two extension conventions in an interface whose whole purpose is being uniform across backends. So the map is replaced everywhere instead:

was	is
map[string]any{storage.SortDirectionKey: "DESC"}	storage.WithSortDirection(storage.Descending)
map[string]any{"pk_field": f, "pk_value": v}	storage.WithPartitionKey(f, v)
map[string]any{"pk_field": f}	storage.WithPartitionKeyField(f)
—	storage.WithFields("name", "modified_at")

Validation now happens in one place, Options.validate, before any adapter sees a value.

Change

• storage/options.go: Option, Options, the With* constructors, ResolveOptions, validate.
• ResolveOptions is exported because anyone implementing StorageAdapter outside the package — a forwarding wrapper, or a test fake — receives []Option and would otherwise have to apply them by hand and reimplement the validation. That gap showed up on the first external consumer, not in review.
• Every operation and *Context variant takes ...Option in place of params ...map[string]any.
• UpdateContext selects the named fields instead of "*", and rejects a field the item does not have.
• Docs and README updated; docs/observability-internals.md interface listing too.

Selecting is also what makes clearing a field work: Updates on a struct skips zero-valued fields unless they are selected.

Breaking changes

• Every storage operation takes ...storage.Option instead of params ...map[string]any.
• storage.SortDirectionKey is removed. WithSortDirection takes a SortingDirection, so the old case-insensitive "desc" string no longer works.
• CosmosDB's pk_field / pk_value keys are replaced by the options above.
• An empty partition key field name now reads as "unspecified" (falling back to "pk") rather than returning an error. A value supplied without a field is still rejected, by validate. The old error existed because the map could hold a non-string; that case is now a compile error.

allow-initial-development-versions: true is set in release.yml, so this releases as v0.20.0, not v1.0.0.

What a caller must change

The compiler finds every one of these, and a call that passed no params compiles
unchanged. Full guide with copy-paste agent prompts:
docs/migration.md.

// sort direction
adapter.List(&tasks, "created_at", nil, 100, "",
-   map[string]any{storage.SortDirectionKey: "DESC"})
+   storage.WithSortDirection(storage.Descending))

// CosmosDB partition key
-params := map[string]any{"pk_field": "tenant", "pk_value": "acme-corp"}
-adapter.Get(&user, map[string]any{"id": id}, params)
+adapter.Get(&user, map[string]any{"id": id},
+   storage.WithPartitionKey("tenant", "acme-corp"))

A wrapper, decorator or test fake that satisfies StorageAdapter changes its
signatures, and applies the options the way an adapter does:

func (f *fakeAdapter) List(dest any, sortKey string, filter map[string]any,
    limit int, cursor string, opts ...storage.Option) (string, error) {

    options, err := storage.ResolveOptions(opts...)
    if err != nil {
        return "", err
    }
    f.lastDirection = options.SortDirection
    ...
}

One-liner for a consumer's coding agent

Upgrade this repository's github.com/tink3rlabs/magic dependency and fix the call
sites. Every storage operation's trailing `params ...map[string]any` is now
`opts ...storage.Option`:

- a map carrying storage.SortDirectionKey becomes
  storage.WithSortDirection(storage.Ascending) or (storage.Descending)
- CosmosDB pk_field / pk_value become storage.WithPartitionKey(field, value),
  or WithPartitionKeyField(field) to read the value off the item
- any type of ours that implements storage.StorageAdapter — a wrapper, a
  decorator, a test fake or a generated mock — needs its signatures changed, and
  should apply the options with storage.ResolveOptions(opts...)

A call that passed no params needs no change. Do not invent an option to preserve
a map's shape. Work until `go build ./...` and `go vet ./...` are clean.

Testing

go build ./..., go vet ./..., go test ./... -count=1 — all nine packages pass. Everything touched is gofmt clean (four files were already unformatted on main and are left alone).

Each new guard was checked by reverting the implementation and watching it fail:

test	implementation reverted	result
TestSQLAdapterUpdateWritesOnlyNamedFields	the Select branch	fails — name="original"
TestSQLAdapterUpdateNamedFieldWritesZeroValue	the Select branch	fails — color wiped
TestSQLAdapterUpdateRejectsUnknownField	the Select branch	fails — no error
TestSQLAdapterUpdateRejectsEmptyFieldList	the validate check	fails
TestResolveOptionsRejectsUnsafeFieldName	the identifier regex	fails

TestSQLAdapterUpdateRejectsUnsafeFieldName is deliberately not in that table: it passes with the regex removed, because requireKnownFields rejects "name, color" as an unknown field first. It guards the schema lookup, not the regex. That is why options_internal_test.go exercises ResolveOptions directly — an adapter with no schema to consult has nothing but that validation between a caller and an injected identifier.

Two tests pass with or without the change, by design: TestSQLAdapterUpdateWithoutFieldsWritesEveryField (the default is unchanged) and TestSQLAdapterUpdateNamedFieldsRespectFilter (WithFields narrows what is written, never what is matched).

Three existing tests were deleted because the compiler now rejects what they asserted — two covered a non-string pk_field, one a non-[]string field list.

Adapter coverage (second commit)

Nothing exercised the DynamoDB or CosmosDB adapters against anything speaking their API. Writing that coverage found three pre-existing defects, all fixed or pinned here:

1. List could not do an unfiltered scan at all. ORDER BY was appended whenever sortKey != "", regardless of whether a WHERE had been emitted, and PartiQL refuses ORDER BY without one — so an unfiltered List came back as an opaque ValidationException from inside ExecuteStatement. Worse than it first appears: validateSortKey rejects an empty sort key, so the if sortKey != "" guard is dead code and a caller cannot opt out of ordering. Now fails with an error that names the missing filter.
2. The sort direction was silently dropped on DynamoDB. List/Search emitted a bare ORDER BY with no ASC/DESC and never read their options. Fixed — and the guard uses a composite-key table, because with only a hash key a partition-scoped filter returns one item and the ordering is unobservable, so the test would have passed either way.
3. Update was a whole-item PutItem that discarded its filter — an attribute absent from the struct was deleted, not left alone, and a filter matching nothing still wrote, always returning nil. Filed as issue: storage/dynamodb: Update replaces the whole item and ignores its filter #263 and fixed in the third commit: UpdateItem with an UpdateExpression, the filter as a ConditionExpression, ConditionalCheckFailedException → ErrNotFound, and attribute_exists on the hash key so Update still never creates. WithFields now means something on this adapter too.

Conformance assertions added for all four adapters; only MemoryAdapter had one, so a drifting signature on the other three was caught only by whichever caller stopped compiling.

How it runs. The DynamoDB tests reach a real endpoint via DYNAMODB_TEST_ENDPOINT and skip without it. CI now starts amazon/dynamodb-local and sets it, so the skip cannot become a silent pass. Verified locally against both DynamoDB Local and LocalStack.

CosmosDB needs no emulator for what changed here: every operation resolves its options before touching the database client, so a zero-value adapter proves an invalid option is rejected rather than carried into a request. Each case fails if it panics — which is what it would do had validation moved after the first client use.

Not covered

CosmosDB's read/write paths still have no coverage against the Azure emulator. CountContext remains the // TODO Implement stub returning (0, nil) on both adapters (#200), untouched here.

WithFields is accepted and ignored by the CosmosDB adapter: only SQL, Memory and DynamoDB honour it. A CosmosDB caller that follows the new docs therefore gets no error and still loses concurrent partial updates. Either honour it in the read/merge/replace path or reject it on that adapter — accepting it silently is the one shape a consumer cannot debug.

Third commit: the DynamoDB Update fix (#263)

Addressing the entry needs the table's key attribute names, so there is now one DescribeTable per table, cached — a key schema is fixed once the table exists. Key attributes are skipped rather than rejected when building the SET clause, because without WithFields the write set is every attribute of the item and the key is always among them; key attributes may still appear in the condition, so the filter needs no special casing. The write set is sorted when it comes from the item rather than WithFields, since ranging a map would vary the generated expression between otherwise identical calls.

Its guards were written first and watched fail: UpdateRespectsItsFilter (= <nil>; want ErrNotFound), UpdateDoesNotCreateMissingItem (same), and UpdateHonoursWithFields (got {Id:d1 Name:renamed Color:}; want color=red). UpdateWithoutFieldsWritesEveryAttribute passes either way by design — it pins the default, which is unchanged.

Verified against both DynamoDB Local and LocalStack, which differ on conditional expressions.

This is a third breaking change: Update no longer creates a missing entry, no longer ignores its filter, and no longer removes attributes absent from the item.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Lutherwaves]

@deanefrati feedback would be welcome here