From 1f86f3df121c83410417bdda2f3d98d1587501fe Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Fri, 8 May 2026 23:51:24 +0700 Subject: [PATCH] feat(storage): Firestore KVStore + KVProvider abstraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 04 of go-port-cloud-run plan. Introduces KVProvider abstraction with memory backend (via Prefixed wrapper) and Firestore backend (via collection-per-module isolation). Backend selection gated by env vars: GOOGLE_CLOUD_PROJECT or FIRESTORE_EMULATOR_HOST → Firestore, else memory. Emulator-gated tests via `make test-emulator`. Security hardened: emulator fallback project ID, prefix validation on List, length-in-bytes docs. --- Makefile | 28 ++++ cmd/server/main.go | 78 +++++++-- go.mod | 41 ++++- go.sum | 94 +++++++++++ internal/modules/registry.go | 18 ++- internal/modules/registry_test.go | 42 ++--- internal/server/router_test.go | 2 +- internal/storage/firestore_client.go | 30 ++++ internal/storage/firestore_kv.go | 216 +++++++++++++++++++++++++ internal/storage/firestore_kv_test.go | 207 ++++++++++++++++++++++++ internal/storage/firestore_provider.go | 24 +++ internal/storage/kv_provider.go | 35 ++++ 12 files changed, 767 insertions(+), 48 deletions(-) create mode 100644 Makefile create mode 100644 internal/storage/firestore_client.go create mode 100644 internal/storage/firestore_kv.go create mode 100644 internal/storage/firestore_kv_test.go create mode 100644 internal/storage/firestore_provider.go create mode 100644 internal/storage/kv_provider.go diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..6c24b02 --- /dev/null +++ b/Makefile @@ -0,0 +1,28 @@ +.PHONY: test test-emulator firestore-emulator vet build run + +# Default: run unit tests that don't require the Firestore emulator. +test: + go test -race -count=1 ./... + +# Start a local Firestore emulator (separate terminal). Requires gcloud SDK +# with the cloud-firestore-emulator component installed: +# gcloud components install cloud-firestore-emulator +firestore-emulator: + gcloud emulators firestore start --host-port=localhost:8085 + +# Run all tests including emulator-gated ones. Expects the emulator to be +# already running (use `make firestore-emulator` in another shell). +test-emulator: + FIRESTORE_EMULATOR_HOST=localhost:8085 \ + GOOGLE_CLOUD_PROJECT=miti99bot-go-test \ + go test -race -count=1 ./... + +vet: + go vet ./... + +build: + CGO_ENABLED=0 go build -ldflags="-s -w" -o ./bin/server ./cmd/server + +# Local dev run with an in-memory KV (no Firestore needed). +run: + go run ./cmd/server diff --git a/cmd/server/main.go b/cmd/server/main.go index f93c353..3a2b3d9 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -25,6 +25,11 @@ var secretEnvKeys = []string{ "CRON_SHARED_SECRET", } +// firestoreInitTimeout caps client construction at startup. Cloud Run cold +// start budget is 500ms target; firestore.NewClient is normally fast but +// network blips can make it hang. Fail fast and let Cloud Run restart us. +const firestoreInitTimeout = 10 * time.Second + func main() { cfg := loadConfig() if cfg.TelegramBotToken == "" { @@ -37,15 +42,18 @@ func main() { rootCtx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() + provider, closeProvider, err := buildProvider(rootCtx, cfg) + if err != nil { + log.Fatalf("storage: %v", err) + } + defer closeProvider() + b, err := telegram.NewBot(cfg.TelegramBotToken) if err != nil { log.Fatalf("telegram bot init: %v", err) } - kv := storage.NewMemoryKVStore() - deps := modules.Deps{KV: kv, Env: cfg.ModuleEnv} - - reg, err := modules.Build(cfg.Modules, modules.Factories, deps) + reg, err := modules.Build(cfg.Modules, modules.Factories, provider, cfg.ModuleEnv) if err != nil { log.Fatalf("module registry: %v", err) } @@ -91,13 +99,49 @@ func main() { } } +// buildProvider picks the storage backend from env. Firestore is selected +// when GOOGLE_CLOUD_PROJECT or FIRESTORE_EMULATOR_HOST is set; otherwise we +// fall back to in-memory storage so a developer can run the bot without GCP. +// +// Returned closer is always non-nil and safe to call exactly once. +func buildProvider(ctx context.Context, cfg config) (storage.KVProvider, func(), error) { + useFirestore := cfg.GCPProject != "" || cfg.FirestoreEmulatorHost != "" + if !useFirestore { + log.Println("WARN: GOOGLE_CLOUD_PROJECT unset; using in-memory KV (data lost on restart)") + return storage.NewMemoryProvider(), func() {}, nil + } + + // Emulator ignores the project ID but the SDK still requires *some* + // non-empty value; supply a placeholder so emulator-only local dev works. + projectID := cfg.GCPProject + if projectID == "" && cfg.FirestoreEmulatorHost != "" { + projectID = "miti99bot-emulator" + } + + initCtx, cancel := context.WithTimeout(ctx, firestoreInitTimeout) + defer cancel() + client, err := storage.NewFirestoreClient(initCtx, projectID) + if err != nil { + return nil, func() {}, err + } + closer := func() { + if err := client.Close(); err != nil { + log.Printf("firestore close: %v", err) + } + } + log.Printf("storage: Firestore project=%s emulator=%q", projectID, cfg.FirestoreEmulatorHost) + return storage.NewFirestoreProvider(client), closer, nil +} + type config struct { - Port string - TelegramBotToken string - WebhookSecret string - CronSecret string - Modules []string - ModuleEnv map[string]string // sensitive keys stripped, safe to hand to modules + Port string + TelegramBotToken string + WebhookSecret string + CronSecret string + GCPProject string + FirestoreEmulatorHost string + Modules []string + ModuleEnv map[string]string // sensitive keys stripped, safe to hand to modules } func loadConfig() config { @@ -112,12 +156,14 @@ func loadConfig() config { port = "8080" } return config{ - Port: port, - TelegramBotToken: envMap["TELEGRAM_BOT_TOKEN"], - WebhookSecret: envMap["TELEGRAM_WEBHOOK_SECRET"], - CronSecret: envMap["CRON_SHARED_SECRET"], - Modules: splitCSV(envMap["MODULES"]), - ModuleEnv: envForModules(envMap), + Port: port, + TelegramBotToken: envMap["TELEGRAM_BOT_TOKEN"], + WebhookSecret: envMap["TELEGRAM_WEBHOOK_SECRET"], + CronSecret: envMap["CRON_SHARED_SECRET"], + GCPProject: envMap["GOOGLE_CLOUD_PROJECT"], + FirestoreEmulatorHost: envMap["FIRESTORE_EMULATOR_HOST"], + Modules: splitCSV(envMap["MODULES"]), + ModuleEnv: envForModules(envMap), } } diff --git a/go.mod b/go.mod index 26e1599..28cb5c8 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,42 @@ module github.com/tiennm99/miti99bot-go -go 1.23 +go 1.25.0 -require github.com/go-telegram/bot v1.20.0 +require ( + cloud.google.com/go/firestore v1.22.0 + github.com/go-telegram/bot v1.20.0 + google.golang.org/api v0.274.0 + google.golang.org/grpc v1.80.0 +) + +require ( + cloud.google.com/go v0.123.0 // indirect + cloud.google.com/go/auth v0.18.2 // indirect + cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect + cloud.google.com/go/compute/metadata v0.9.0 // indirect + cloud.google.com/go/longrunning v0.9.0 // indirect + github.com/cespare/xxhash/v2 v2.3.0 // indirect + github.com/felixge/httpsnoop v1.0.4 // indirect + github.com/go-logr/logr v1.4.3 // indirect + github.com/go-logr/stdr v1.2.2 // indirect + github.com/google/s2a-go v0.1.9 // indirect + github.com/googleapis/enterprise-certificate-proxy v0.3.14 // indirect + github.com/googleapis/gax-go/v2 v2.21.0 // indirect + go.opentelemetry.io/auto/sdk v1.2.1 // indirect + go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0 // indirect + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 // indirect + go.opentelemetry.io/otel v1.43.0 // indirect + go.opentelemetry.io/otel/metric v1.43.0 // indirect + go.opentelemetry.io/otel/trace v1.43.0 // indirect + golang.org/x/crypto v0.49.0 // indirect + golang.org/x/net v0.52.0 // indirect + golang.org/x/oauth2 v0.36.0 // indirect + golang.org/x/sync v0.20.0 // indirect + golang.org/x/sys v0.42.0 // indirect + golang.org/x/text v0.35.0 // indirect + golang.org/x/time v0.15.0 // indirect + google.golang.org/genproto v0.0.0-20260319201613-d00831a3d3e7 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 // indirect + google.golang.org/protobuf v1.36.11 // indirect +) diff --git a/go.sum b/go.sum index da4a2da..29b2132 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,96 @@ +cloud.google.com/go v0.123.0 h1:2NAUJwPR47q+E35uaJeYoNhuNEM9kM8SjgRgdeOJUSE= +cloud.google.com/go v0.123.0/go.mod h1:xBoMV08QcqUGuPW65Qfm1o9Y4zKZBpGS+7bImXLTAZU= +cloud.google.com/go/auth v0.18.2 h1:+Nbt5Ev0xEqxlNjd6c+yYUeosQ5TtEUaNcN/3FozlaM= +cloud.google.com/go/auth v0.18.2/go.mod h1:xD+oY7gcahcu7G2SG2DsBerfFxgPAJz17zz2joOFF3M= +cloud.google.com/go/auth/oauth2adapt v0.2.8 h1:keo8NaayQZ6wimpNSmW5OPc283g65QNIiLpZnkHRbnc= +cloud.google.com/go/auth/oauth2adapt v0.2.8/go.mod h1:XQ9y31RkqZCcwJWNSx2Xvric3RrU88hAYYbjDWYDL+c= +cloud.google.com/go/compute/metadata v0.9.0 h1:pDUj4QMoPejqq20dK0Pg2N4yG9zIkYGdBtwLoEkH9Zs= +cloud.google.com/go/compute/metadata v0.9.0/go.mod h1:E0bWwX5wTnLPedCKqk3pJmVgCBSM6qQI1yTBdEb3C10= +cloud.google.com/go/firestore v1.22.0 h1:avooeboIq37vKXobrbPUFhFBxS/c3FqmWoX0xs8dO6E= +cloud.google.com/go/firestore v1.22.0/go.mod h1:PaM4i7i7ruALSKmlpHXXZaPObcZw0W7ie5UOPr72iTU= +cloud.google.com/go/longrunning v0.9.0 h1:0EzbDEGsAvOZNbqXopgniY0w0a1phvu5IdUFq8grmqY= +cloud.google.com/go/longrunning v0.9.0/go.mod h1:pkTz846W7bF4o2SzdWJ40Hu0Re+UoNT6Q5t+igIcb8E= +github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= +github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= +github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5 h1:6xNmx7iTtyBRev0+D/Tv1FZd4SCg8axKApyNyRsAt/w= +github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5/go.mod h1:KdCmV+x/BuvyMxRnYBlmVaq4OLiKW6iRQfvC62cvdkI= +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/envoyproxy/go-control-plane v0.14.0 h1:hbG2kr4RuFj222B6+7T83thSPqLjwBIfQawTkC++2HA= +github.com/envoyproxy/go-control-plane/envoy v1.36.0 h1:yg/JjO5E7ubRyKX3m07GF3reDNEnfOboJ0QySbH736g= +github.com/envoyproxy/go-control-plane/envoy v1.36.0/go.mod h1:ty89S1YCCVruQAm9OtKeEkQLTb+Lkz0k8v9W0Oxsv98= +github.com/envoyproxy/protoc-gen-validate v1.3.0 h1:TvGH1wof4H33rezVKWSpqKz5NXWg5VPuZ0uONDT6eb4= +github.com/envoyproxy/protoc-gen-validate v1.3.0/go.mod h1:HvYl7zwPa5mffgyeTUHA9zHIH36nmrm7oCbo4YKoSWA= +github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= +github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= +github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= +github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= +github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-telegram/bot v1.20.0 h1:4Pea/qTidSspr4WBJw9FbHUMNhYeqszBqQUfsQEyFbc= github.com/go-telegram/bot v1.20.0/go.mod h1:i2TRs7fXWIeaceF3z7KzsMt/he0TwkVC680mvdTFYeM= +github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= +github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/s2a-go v0.1.9 h1:LGD7gtMgezd8a/Xak7mEWL0PjoTQFvpRudN895yqKW0= +github.com/google/s2a-go v0.1.9/go.mod h1:YA0Ei2ZQL3acow2O62kdp9UlnvMmU7kA6Eutn0dXayM= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/googleapis/enterprise-certificate-proxy v0.3.14 h1:yh8ncqsbUY4shRD5dA6RlzjJaT4hi3kII+zYw8wmLb8= +github.com/googleapis/enterprise-certificate-proxy v0.3.14/go.mod h1:vqVt9yG9480NtzREnTlmGSBmFrA+bzb0yl0TxoBQXOg= +github.com/googleapis/gax-go/v2 v2.21.0 h1:h45NjjzEO3faG9Lg/cFrBh2PgegVVgzqKzuZl/wMbiI= +github.com/googleapis/gax-go/v2 v2.21.0/go.mod h1:But/NJU6TnZsrLai/xBAQLLz+Hc7fHZJt/hsCz3Fih4= +github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= +github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= +go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= +go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0 h1:q4XOmH/0opmeuJtPsbFNivyl7bCt7yRBbeEm2sC/XtQ= +go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0/go.mod h1:snMWehoOh2wsEwnvvwtDyFCxVeDAODenXHtn5vzrKjo= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 h1:F7Jx+6hwnZ41NSFTO5q4LYDtJRXBf2PD0rNBkeB/lus= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0/go.mod h1:UHB22Z8QsdRDrnAtX4PntOl36ajSxcdUMt1sF7Y6E7Q= +go.opentelemetry.io/otel v1.43.0 h1:mYIM03dnh5zfN7HautFE4ieIig9amkNANT+xcVxAj9I= +go.opentelemetry.io/otel v1.43.0/go.mod h1:JuG+u74mvjvcm8vj8pI5XiHy1zDeoCS2LB1spIq7Ay0= +go.opentelemetry.io/otel/metric v1.43.0 h1:d7638QeInOnuwOONPp4JAOGfbCEpYb+K6DVWvdxGzgM= +go.opentelemetry.io/otel/metric v1.43.0/go.mod h1:RDnPtIxvqlgO8GRW18W6Z/4P462ldprJtfxHxyKd2PY= +go.opentelemetry.io/otel/sdk v1.43.0 h1:pi5mE86i5rTeLXqoF/hhiBtUNcrAGHLKQdhg4h4V9Dg= +go.opentelemetry.io/otel/sdk v1.43.0/go.mod h1:P+IkVU3iWukmiit/Yf9AWvpyRDlUeBaRg6Y+C58QHzg= +go.opentelemetry.io/otel/sdk/metric v1.42.0 h1:D/1QR46Clz6ajyZ3G8SgNlTJKBdGp84q9RKCAZ3YGuA= +go.opentelemetry.io/otel/sdk/metric v1.42.0/go.mod h1:Ua6AAlDKdZ7tdvaQKfSmnFTdHx37+J4ba8MwVCYM5hc= +go.opentelemetry.io/otel/trace v1.43.0 h1:BkNrHpup+4k4w+ZZ86CZoHHEkohws8AY+WTX09nk+3A= +go.opentelemetry.io/otel/trace v1.43.0/go.mod h1:/QJhyVBUUswCphDVxq+8mld+AvhXZLhe+8WVFxiFff0= +golang.org/x/crypto v0.49.0 h1:+Ng2ULVvLHnJ/ZFEq4KdcDd/cfjrrjjNSXNzxg0Y4U4= +golang.org/x/crypto v0.49.0/go.mod h1:ErX4dUh2UM+CFYiXZRTcMpEcN8b/1gxEuv3nODoYtCA= +golang.org/x/net v0.52.0 h1:He/TN1l0e4mmR3QqHMT2Xab3Aj3L9qjbhRm78/6jrW0= +golang.org/x/net v0.52.0/go.mod h1:R1MAz7uMZxVMualyPXb+VaqGSa3LIaUqk0eEt3w36Sw= +golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= +golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= +golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= +golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= +golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= +golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= +gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= +gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= +google.golang.org/api v0.274.0 h1:aYhycS5QQCwxHLwfEHRRLf9yNsfvp1JadKKWBE54RFA= +google.golang.org/api v0.274.0/go.mod h1:JbAt7mF+XVmWu6xNP8/+CTiGH30ofmCmk9nM8d8fHew= +google.golang.org/genproto v0.0.0-20260319201613-d00831a3d3e7 h1:XzmzkmB14QhVhgnawEVsOn6OFsnpyxNPRY9QV01dNB0= +google.golang.org/genproto v0.0.0-20260319201613-d00831a3d3e7/go.mod h1:L43LFes82YgSonw6iTXTxXUX1OlULt4AQtkik4ULL/I= +google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 h1:VPWxll4HlMw1Vs/qXtN7BvhZqsS9cdAittCNvVENElA= +google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9/go.mod h1:7QBABkRtR8z+TEnmXTqIqwJLlzrZKVfAUm7tY3yGv0M= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 h1:m8qni9SQFH0tJc1X0vmnpw/0t+AImlSvp30sEupozUg= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/grpc v1.80.0 h1:Xr6m2WmWZLETvUNvIUmeD5OAagMw3FiKmMlTdViWsHM= +google.golang.org/grpc v1.80.0/go.mod h1:ho/dLnxwi3EDJA4Zghp7k2Ec1+c2jqup0bFkw07bwF4= +google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= +google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/modules/registry.go b/internal/modules/registry.go index 36f09fa..e575936 100644 --- a/internal/modules/registry.go +++ b/internal/modules/registry.go @@ -57,16 +57,18 @@ func (r *Registry) Crons() []Cron { return out } -// Build constructs a Registry from the requested module names. It calls each -// factory with a per-module-prefixed KVStore, validates every command/cron, -// and aborts on duplicate command names across the union of all visibilities. +// Build constructs a Registry from the requested module names. The KVProvider +// supplies a per-module-isolated KVStore (MemoryProvider key-prefixes a shared +// store; FirestoreProvider hands out one collection per module). Build validates +// every command/cron and aborts on duplicate command names across the union of +// all visibilities. // // Names not present in factories are reported as a single error so a typo in // MODULES does not silently load a smaller bot than intended. Duplicate names // in MODULES are also a hard error to keep startup deterministic. -func Build(enabled []string, factories map[string]Factory, base Deps) (*Registry, error) { - if base.KV == nil { - return nil, fmt.Errorf("modules: Deps.KV is required") +func Build(enabled []string, factories map[string]Factory, kv storage.KVProvider, env map[string]string) (*Registry, error) { + if kv == nil { + return nil, fmt.Errorf("modules: KVProvider is required") } reg := &Registry{ @@ -99,8 +101,8 @@ func Build(enabled []string, factories map[string]Factory, base Deps) (*Registry } moduleDeps := Deps{ - KV: storage.Prefixed(base.KV, name), - Env: base.Env, + KV: kv.For(name), + Env: env, } mod := factory(moduleDeps) mod.Name = name // enforce: module name is its registry key, not whatever the factory chose diff --git a/internal/modules/registry_test.go b/internal/modules/registry_test.go index c64372c..bdf07e4 100644 --- a/internal/modules/registry_test.go +++ b/internal/modules/registry_test.go @@ -35,10 +35,10 @@ func factory(name string, cmds []Command, crons []Cron) Factory { } } -func baseDeps() Deps { return Deps{KV: storage.NewMemoryKVStore()} } +func newProvider() storage.KVProvider { return storage.NewMemoryProvider() } func TestBuild_EmptyModulesBootsCleanly(t *testing.T) { - reg, err := Build(nil, map[string]Factory{}, baseDeps()) + reg, err := Build(nil, map[string]Factory{}, newProvider(), nil) if err != nil { t.Fatalf("Build empty: %v", err) } @@ -52,7 +52,7 @@ func TestBuild_LoadsRequestedModules(t *testing.T) { "alpha": factory("alpha", []Command{noopCmd("a1")}, nil), "beta": factory("beta", []Command{noopCmd("b1")}, []Cron{noopCron("daily")}), } - reg, err := Build([]string{"alpha", "beta"}, factories, baseDeps()) + reg, err := Build([]string{"alpha", "beta"}, factories, newProvider(), nil) if err != nil { t.Fatalf("Build: %v", err) } @@ -72,7 +72,7 @@ func TestBuild_SkipsModulesNotInEnv(t *testing.T) { "alpha": factory("alpha", []Command{noopCmd("a1")}, nil), "beta": factory("beta", []Command{noopCmd("b1")}, nil), } - reg, err := Build([]string{"alpha"}, factories, baseDeps()) + reg, err := Build([]string{"alpha"}, factories, newProvider(), nil) if err != nil { t.Fatalf("Build: %v", err) } @@ -82,7 +82,7 @@ func TestBuild_SkipsModulesNotInEnv(t *testing.T) { } func TestBuild_RejectsUnknownModule(t *testing.T) { - _, err := Build([]string{"ghost"}, map[string]Factory{}, baseDeps()) + _, err := Build([]string{"ghost"}, map[string]Factory{}, newProvider(), nil) if err == nil || !strings.Contains(err.Error(), "ghost") { t.Errorf("expected error mentioning ghost, got %v", err) } @@ -93,7 +93,7 @@ func TestBuild_DetectsCommandConflict(t *testing.T) { "alpha": factory("alpha", []Command{noopCmd("ping")}, nil), "beta": factory("beta", []Command{noopCmd("ping")}, nil), } - _, err := Build([]string{"alpha", "beta"}, factories, baseDeps()) + _, err := Build([]string{"alpha", "beta"}, factories, newProvider(), nil) if err == nil { t.Fatal("expected conflict error") } @@ -107,16 +107,16 @@ func TestBuild_DetectsCronConflict(t *testing.T) { "alpha": factory("alpha", nil, []Cron{noopCron("daily")}), "beta": factory("beta", nil, []Cron{noopCron("daily")}), } - _, err := Build([]string{"alpha", "beta"}, factories, baseDeps()) + _, err := Build([]string{"alpha", "beta"}, factories, newProvider(), nil) if err == nil || !strings.Contains(err.Error(), "cron conflict") { t.Errorf("expected cron conflict, got %v", err) } } -func TestBuild_RequiresKV(t *testing.T) { - _, err := Build(nil, map[string]Factory{}, Deps{}) +func TestBuild_RequiresProvider(t *testing.T) { + _, err := Build(nil, map[string]Factory{}, nil, nil) if err == nil { - t.Error("expected error when Deps.KV is nil") + t.Error("expected error when KVProvider is nil") } } @@ -125,7 +125,7 @@ func TestBuild_ValidationErrorsMentionModule(t *testing.T) { factories := map[string]Factory{ "alpha": factory("alpha", []Command{bad}, nil), } - _, err := Build([]string{"alpha"}, factories, baseDeps()) + _, err := Build([]string{"alpha"}, factories, newProvider(), nil) if err == nil || !strings.Contains(err.Error(), "alpha") { t.Errorf("expected error mentioning module 'alpha', got %v", err) } @@ -142,7 +142,7 @@ func TestDispatchScheduled_RunsHandler(t *testing.T) { }, }}), } - reg, err := Build([]string{"alpha"}, factories, baseDeps()) + reg, err := Build([]string{"alpha"}, factories, newProvider(), nil) if err != nil { t.Fatalf("Build: %v", err) } @@ -155,7 +155,7 @@ func TestDispatchScheduled_RunsHandler(t *testing.T) { } func TestDispatchScheduled_UnknownReturnsErrCronNotFound(t *testing.T) { - reg, err := Build(nil, map[string]Factory{}, baseDeps()) + reg, err := Build(nil, map[string]Factory{}, newProvider(), nil) if err != nil { t.Fatalf("Build: %v", err) } @@ -167,7 +167,7 @@ func TestDispatchScheduled_UnknownReturnsErrCronNotFound(t *testing.T) { func TestDispatchScheduled_PassesPrefixedDeps(t *testing.T) { ctx := context.Background() - base := storage.NewMemoryKVStore() + provider := storage.NewMemoryProvider() factories := map[string]Factory{ "alpha": func(d Deps) Module { @@ -187,7 +187,7 @@ func TestDispatchScheduled_PassesPrefixedDeps(t *testing.T) { }}} }, } - reg, err := Build([]string{"alpha", "beta"}, factories, Deps{KV: base}) + reg, err := Build([]string{"alpha", "beta"}, factories, provider, nil) if err != nil { t.Fatalf("Build: %v", err) } @@ -199,11 +199,11 @@ func TestDispatchScheduled_PassesPrefixedDeps(t *testing.T) { } // Underlying base store should hold each module's prefixed key separately. - gotA, err := base.Get(ctx, "alpha:last") + gotA, err := provider.Base().Get(ctx, "alpha:last") if err != nil || string(gotA) != "A" { t.Errorf("alpha:last = %q (err=%v), want A", gotA, err) } - gotB, err := base.Get(ctx, "beta:last") + gotB, err := provider.Base().Get(ctx, "beta:last") if err != nil || string(gotB) != "B" { t.Errorf("beta:last = %q (err=%v), want B", gotB, err) } @@ -212,7 +212,7 @@ func TestDispatchScheduled_PassesPrefixedDeps(t *testing.T) { func TestBuild_RejectsInvalidModuleName(t *testing.T) { for _, name := range []string{"BadName", "with-dash", "a:b", ""} { t.Run(name, func(t *testing.T) { - _, err := Build([]string{name}, map[string]Factory{}, baseDeps()) + _, err := Build([]string{name}, map[string]Factory{}, newProvider(), nil) if err == nil { t.Errorf("name %q: expected error", name) } @@ -224,7 +224,7 @@ func TestBuild_RejectsDuplicateModuleInEnv(t *testing.T) { factories := map[string]Factory{ "alpha": factory("alpha", []Command{noopCmd("a1")}, nil), } - _, err := Build([]string{"alpha", "alpha"}, factories, baseDeps()) + _, err := Build([]string{"alpha", "alpha"}, factories, newProvider(), nil) if err == nil || !strings.Contains(err.Error(), "duplicate") { t.Errorf("expected duplicate-module error, got %v", err) } @@ -232,7 +232,7 @@ func TestBuild_RejectsDuplicateModuleInEnv(t *testing.T) { func TestBuild_PerModulePrefixedKV(t *testing.T) { ctx := context.Background() - base := storage.NewMemoryKVStore() + provider := storage.NewMemoryProvider() // Each module writes a value to the same key; with per-module prefixing // they must not collide. @@ -247,7 +247,7 @@ func TestBuild_PerModulePrefixedKV(t *testing.T) { return Module{Commands: []Command{noopCmd("b")}} }, } - if _, err := Build([]string{"alpha", "beta"}, factories, Deps{KV: base}); err != nil { + if _, err := Build([]string{"alpha", "beta"}, factories, provider, nil); err != nil { t.Fatalf("Build: %v", err) } diff --git a/internal/server/router_test.go b/internal/server/router_test.go index 470a315..09c6503 100644 --- a/internal/server/router_test.go +++ b/internal/server/router_test.go @@ -15,7 +15,7 @@ const testCronSecret = "shared-cron-secret" func buildRegistry(t *testing.T, factories map[string]modules.Factory, names ...string) *modules.Registry { t.Helper() - reg, err := modules.Build(names, factories, modules.Deps{KV: storage.NewMemoryKVStore()}) + reg, err := modules.Build(names, factories, storage.NewMemoryProvider(), nil) if err != nil { t.Fatalf("modules.Build: %v", err) } diff --git a/internal/storage/firestore_client.go b/internal/storage/firestore_client.go new file mode 100644 index 0000000..58dc72a --- /dev/null +++ b/internal/storage/firestore_client.go @@ -0,0 +1,30 @@ +package storage + +import ( + "context" + "fmt" + "os" + + "cloud.google.com/go/firestore" +) + +// NewFirestoreClient constructs a Firestore client using the project ID from +// GOOGLE_CLOUD_PROJECT. The Firestore SDK auto-detects FIRESTORE_EMULATOR_HOST +// and routes to the emulator when set, so the same constructor serves dev and +// prod. +// +// The client is goroutine-safe and meant to be reused for the lifetime of the +// process; callers should defer Close on the returned client at shutdown. +func NewFirestoreClient(ctx context.Context, projectID string) (*firestore.Client, error) { + if projectID == "" { + projectID = os.Getenv("GOOGLE_CLOUD_PROJECT") + } + if projectID == "" { + return nil, fmt.Errorf("storage: GOOGLE_CLOUD_PROJECT is required for Firestore") + } + c, err := firestore.NewClient(ctx, projectID) + if err != nil { + return nil, fmt.Errorf("storage: firestore.NewClient: %w", err) + } + return c, nil +} diff --git a/internal/storage/firestore_kv.go b/internal/storage/firestore_kv.go new file mode 100644 index 0000000..c592583 --- /dev/null +++ b/internal/storage/firestore_kv.go @@ -0,0 +1,216 @@ +package storage + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" + "time" + + "cloud.google.com/go/firestore" + "google.golang.org/api/iterator" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// firestoreValueField is the document field that holds the raw value bytes. +// Stored as []byte so non-UTF-8 payloads round-trip without surprises. +const firestoreValueField = "value" + +// firestoreUpdatedAtField is set on every Put for observability + future TTL. +const firestoreUpdatedAtField = "updatedAt" + +// firestoreMaxKeyLen is Firestore's documented document-id byte cap. +const firestoreMaxKeyLen = 1500 + +// FirestoreKVStore is a KVStore backed by a single Firestore collection. The +// caller (FirestoreProvider) creates one per module so cross-module isolation +// is "different collection" — no key prefix needed at this layer. +type FirestoreKVStore struct { + client *firestore.Client + collection string +} + +// NewFirestoreKVStore returns a KVStore writing to the named collection. +// Collection names follow the same alphabet as module names (validated at +// modules.Build), so callers should never need to escape them here. +func NewFirestoreKVStore(client *firestore.Client, collection string) *FirestoreKVStore { + return &FirestoreKVStore{client: client, collection: collection} +} + +// validateKey enforces Firestore document-id constraints up-front so callers +// see a clear error instead of an opaque gRPC InvalidArgument from the wire. +// +// Forbidden patterns: +// - empty +// - longer than 1500 bytes (Firestore's documented limit is bytes, not +// runes; len(string) returns bytes — do NOT switch to utf8.RuneCountInString) +// - contains '/' (path separator) +// - "." or ".." (reserved by Firestore) +// - leading/trailing "__" (reserved namespace) +func validateKey(key string) error { + if key == "" { + return fmt.Errorf("storage: key is empty") + } + if len(key) > firestoreMaxKeyLen { + return fmt.Errorf("storage: key exceeds %d bytes", firestoreMaxKeyLen) + } + if strings.Contains(key, "/") { + return fmt.Errorf("storage: key contains '/' (Firestore path separator)") + } + if key == "." || key == ".." { + return fmt.Errorf("storage: key %q is reserved", key) + } + if strings.HasPrefix(key, "__") && strings.HasSuffix(key, "__") { + return fmt.Errorf("storage: key %q uses reserved __namespace__ pattern", key) + } + return nil +} + +// validatePrefix runs the same checks as validateKey but allows the empty +// string (List with empty prefix scans the whole collection). Without this, +// a module passing a "/"-containing prefix would hand garbage to col.Doc() +// instead of getting a clean error. +func validatePrefix(prefix string) error { + if prefix == "" { + return nil + } + return validateKey(prefix) +} + +func (s *FirestoreKVStore) doc(key string) *firestore.DocumentRef { + return s.client.Collection(s.collection).Doc(key) +} + +// Get returns the raw bytes stored at key, or ErrNotFound. +func (s *FirestoreKVStore) Get(ctx context.Context, key string) ([]byte, error) { + if err := validateKey(key); err != nil { + return nil, err + } + snap, err := s.doc(key).Get(ctx) + if err != nil { + if status.Code(err) == codes.NotFound { + return nil, ErrNotFound + } + return nil, fmt.Errorf("firestore get %s/%s: %w", s.collection, key, err) + } + raw, err := snap.DataAt(firestoreValueField) + if err != nil { + return nil, fmt.Errorf("firestore get %s/%s: missing %q field: %w", s.collection, key, firestoreValueField, err) + } + switch v := raw.(type) { + case []byte: + return v, nil + case string: + // Firestore may decode small payloads as string; keep the API + // byte-clean by re-encoding. + return []byte(v), nil + default: + return nil, fmt.Errorf("firestore get %s/%s: unexpected value type %T", s.collection, key, raw) + } +} + +// GetJSON decodes the value at key into dst. +func (s *FirestoreKVStore) GetJSON(ctx context.Context, key string, dst any) error { + raw, err := s.Get(ctx, key) + if err != nil { + return err + } + if err := json.Unmarshal(raw, dst); err != nil { + return fmt.Errorf("firestore get %s/%s: json decode: %w", s.collection, key, err) + } + return nil +} + +// Put writes raw bytes at key, creating or overwriting. +func (s *FirestoreKVStore) Put(ctx context.Context, key string, val []byte) error { + if err := validateKey(key); err != nil { + return err + } + _, err := s.doc(key).Set(ctx, map[string]any{ + firestoreValueField: val, + firestoreUpdatedAtField: time.Now().UTC(), + }) + if err != nil { + return fmt.Errorf("firestore put %s/%s: %w", s.collection, key, err) + } + return nil +} + +// PutJSON marshals val and writes the bytes at key. +func (s *FirestoreKVStore) PutJSON(ctx context.Context, key string, val any) error { + raw, err := json.Marshal(val) + if err != nil { + return fmt.Errorf("firestore put %s/%s: json encode: %w", s.collection, key, err) + } + return s.Put(ctx, key, raw) +} + +// Delete removes the document at key. Deleting a missing key is not an error +// (idempotent) — Firestore's Delete already has these semantics. +func (s *FirestoreKVStore) Delete(ctx context.Context, key string) error { + if err := validateKey(key); err != nil { + return err + } + _, err := s.doc(key).Delete(ctx) + if err != nil { + return fmt.Errorf("firestore delete %s/%s: %w", s.collection, key, err) + } + return nil +} + +// List returns all document IDs in the collection that start with prefix. +// Implemented as a half-open range scan on document ID — no composite index +// required. Empty prefix returns the whole collection. +// +// Caveat: an all-0xFF prefix (e.g. "\xff\xff") has no successor in the same +// length, so prefixSuccessor returns the prefix unchanged and the range scan +// degenerates to an empty result. Don't use such prefixes. +func (s *FirestoreKVStore) List(ctx context.Context, prefix string) ([]string, error) { + if err := validatePrefix(prefix); err != nil { + return nil, err + } + col := s.client.Collection(s.collection) + q := col.Query + if prefix != "" { + end := prefixSuccessor(prefix) + q = col.Where(firestore.DocumentID, ">=", col.Doc(prefix)). + Where(firestore.DocumentID, "<", col.Doc(end)) + } + iter := q.Documents(ctx) + defer iter.Stop() + + var keys []string + for { + snap, err := iter.Next() + if errors.Is(err, iterator.Done) { + break + } + if err != nil { + return nil, fmt.Errorf("firestore list %s prefix=%q: %w", s.collection, prefix, err) + } + keys = append(keys, snap.Ref.ID) + } + return keys, nil +} + +// prefixSuccessor returns the smallest string strictly greater than every +// string with the given prefix. Used for half-open range scans on document IDs. +// +// For "abc" the successor is "abd". If the prefix ends in 0xFF, we strip the +// trailing 0xFF bytes and increment the last < 0xFF byte. If the prefix is +// entirely 0xFF (vanishingly unlikely for ASCII module data), we fall back to +// an unbounded scan — accepted, callers using such keys deserve what they get. +func prefixSuccessor(prefix string) string { + b := []byte(prefix) + for i := len(b) - 1; i >= 0; i-- { + if b[i] < 0xFF { + b[i]++ + return string(b[:i+1]) + } + } + // All-0xFF: no successor in the same length; return prefix unchanged + // (caller's range Where(< prefix) will degenerate to empty — acceptable). + return prefix +} diff --git a/internal/storage/firestore_kv_test.go b/internal/storage/firestore_kv_test.go new file mode 100644 index 0000000..6c31ee1 --- /dev/null +++ b/internal/storage/firestore_kv_test.go @@ -0,0 +1,207 @@ +package storage + +import ( + "context" + "os" + "reflect" + "sort" + "testing" + "time" + + "cloud.google.com/go/firestore" +) + +// requireEmulator skips the test unless FIRESTORE_EMULATOR_HOST is set. The +// Firestore SDK auto-routes to the emulator when this env var is present. +// +// CI does not run the emulator today; these tests run locally via: +// +// make test-emulator +func requireEmulator(t *testing.T) *firestore.Client { + t.Helper() + if os.Getenv("FIRESTORE_EMULATOR_HOST") == "" { + t.Skip("FIRESTORE_EMULATOR_HOST not set; skipping Firestore emulator test") + } + project := os.Getenv("GOOGLE_CLOUD_PROJECT") + if project == "" { + project = "miti99bot-go-test" + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + c, err := firestore.NewClient(ctx, project) + if err != nil { + t.Fatalf("firestore.NewClient: %v", err) + } + t.Cleanup(func() { _ = c.Close() }) + return c +} + +// uniqueCollection returns a per-test collection name so parallel tests don't +// collide on emulator state. +func uniqueCollection(t *testing.T) string { + t.Helper() + return "test_" + t.Name() +} + +// drainCollection deletes every document in a collection so the next test +// starts clean. The emulator does not support collection-level delete, so we +// iterate. +func drainCollection(t *testing.T, c *firestore.Client, name string) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + docs, err := c.Collection(name).Documents(ctx).GetAll() + if err != nil { + t.Fatalf("drain list: %v", err) + } + for _, d := range docs { + if _, err := d.Ref.Delete(ctx); err != nil { + t.Fatalf("drain delete %s: %v", d.Ref.ID, err) + } + } +} + +func TestFirestoreKV_PutGetRoundTrip(t *testing.T) { + c := requireEmulator(t) + col := uniqueCollection(t) + defer drainCollection(t, c, col) + store := NewFirestoreKVStore(c, col) + + ctx := context.Background() + if err := store.Put(ctx, "score", []byte("42")); err != nil { + t.Fatalf("Put: %v", err) + } + got, err := store.Get(ctx, "score") + if err != nil { + t.Fatalf("Get: %v", err) + } + if string(got) != "42" { + t.Errorf("Get score = %q, want 42", got) + } +} + +func TestFirestoreKV_GetMissingReturnsErrNotFound(t *testing.T) { + c := requireEmulator(t) + col := uniqueCollection(t) + defer drainCollection(t, c, col) + store := NewFirestoreKVStore(c, col) + + if _, err := store.Get(context.Background(), "missing"); err != ErrNotFound { + t.Errorf("Get missing = %v, want ErrNotFound", err) + } +} + +func TestFirestoreKV_PutGetJSON(t *testing.T) { + c := requireEmulator(t) + col := uniqueCollection(t) + defer drainCollection(t, c, col) + store := NewFirestoreKVStore(c, col) + + type point struct{ X, Y int } + want := point{X: 3, Y: 4} + if err := store.PutJSON(context.Background(), "pt", want); err != nil { + t.Fatalf("PutJSON: %v", err) + } + var got point + if err := store.GetJSON(context.Background(), "pt", &got); err != nil { + t.Fatalf("GetJSON: %v", err) + } + if got != want { + t.Errorf("GetJSON = %+v, want %+v", got, want) + } +} + +func TestFirestoreKV_DeleteIdempotent(t *testing.T) { + c := requireEmulator(t) + col := uniqueCollection(t) + defer drainCollection(t, c, col) + store := NewFirestoreKVStore(c, col) + + ctx := context.Background() + if err := store.Put(ctx, "x", []byte("y")); err != nil { + t.Fatalf("Put: %v", err) + } + if err := store.Delete(ctx, "x"); err != nil { + t.Fatalf("Delete existing: %v", err) + } + if err := store.Delete(ctx, "x"); err != nil { + t.Fatalf("Delete missing should be idempotent: %v", err) + } + if _, err := store.Get(ctx, "x"); err != ErrNotFound { + t.Errorf("Get after Delete = %v, want ErrNotFound", err) + } +} + +func TestFirestoreKV_ListByPrefix(t *testing.T) { + c := requireEmulator(t) + col := uniqueCollection(t) + defer drainCollection(t, c, col) + store := NewFirestoreKVStore(c, col) + + ctx := context.Background() + for _, k := range []string{"u_1", "u_2", "u_3", "session_a", "session_b"} { + if err := store.Put(ctx, k, []byte("x")); err != nil { + t.Fatalf("Put %q: %v", k, err) + } + } + + got, err := store.List(ctx, "u_") + if err != nil { + t.Fatalf("List: %v", err) + } + sort.Strings(got) + want := []string{"u_1", "u_2", "u_3"} + if !reflect.DeepEqual(got, want) { + t.Errorf("List u_ = %v, want %v", got, want) + } +} + +func TestFirestoreKV_RejectsInvalidKeys(t *testing.T) { + store := NewFirestoreKVStore(nil, "any") // client unused — validation runs first + + cases := map[string]string{ + "empty": "", + "slash": "a/b", + "dot": ".", + "dotdot": "..", + "reserved__": "__namespace__", + "too long 1501": string(make([]byte, firestoreMaxKeyLen+1)), + } + for label, key := range cases { + t.Run(label, func(t *testing.T) { + if _, err := store.Get(context.Background(), key); err == nil { + t.Errorf("Get %q: expected validation error", key) + } + if err := store.Put(context.Background(), key, []byte("x")); err == nil { + t.Errorf("Put %q: expected validation error", key) + } + }) + } +} + +func TestFirestoreKV_ListRejectsInvalidPrefix(t *testing.T) { + store := NewFirestoreKVStore(nil, "any") // validation runs before any client call + + for _, prefix := range []string{"a/b", "..", "__x__"} { + t.Run(prefix, func(t *testing.T) { + if _, err := store.List(context.Background(), prefix); err == nil { + t.Errorf("List %q: expected validation error", prefix) + } + }) + } +} + +func TestPrefixSuccessor(t *testing.T) { + cases := map[string]string{ + "abc": "abd", + "a": "b", + "": "", + "\xff": "\xff", // all-0xFF: degenerates + "a\xff": "b", // strip trailing 0xFF, increment + } + for in, want := range cases { + if got := prefixSuccessor(in); got != want { + t.Errorf("prefixSuccessor(%q) = %q, want %q", in, got, want) + } + } +} diff --git a/internal/storage/firestore_provider.go b/internal/storage/firestore_provider.go new file mode 100644 index 0000000..60cd7c9 --- /dev/null +++ b/internal/storage/firestore_provider.go @@ -0,0 +1,24 @@ +package storage + +import ( + "cloud.google.com/go/firestore" +) + +// FirestoreProvider is a KVProvider that creates one collection per module. +// No key prefix wrapping is needed — collection-per-module IS the isolation. +type FirestoreProvider struct { + client *firestore.Client +} + +// NewFirestoreProvider returns a provider over the given client. The client +// must outlive every KVStore the provider hands out; callers own its Close. +func NewFirestoreProvider(client *firestore.Client) *FirestoreProvider { + return &FirestoreProvider{client: client} +} + +// For returns a FirestoreKVStore writing to a collection named after the +// module. Module names are validated by modules.Build before reaching here, +// so we don't sanitize again. +func (p *FirestoreProvider) For(moduleName string) KVStore { + return NewFirestoreKVStore(p.client, moduleName) +} diff --git a/internal/storage/kv_provider.go b/internal/storage/kv_provider.go new file mode 100644 index 0000000..21888df --- /dev/null +++ b/internal/storage/kv_provider.go @@ -0,0 +1,35 @@ +package storage + +// KVProvider yields a per-module KVStore. Implementations decide how isolation +// is achieved: MemoryProvider wraps a single in-process store with a key +// prefix, FirestoreProvider uses one collection per module. +// +// Modules never construct KVStores directly — they receive one through their +// factory's Deps. This keeps the storage backend swappable without touching +// any module code. +type KVProvider interface { + For(moduleName string) KVStore +} + +// MemoryProvider is a KVProvider backed by a single in-process MemoryKVStore. +// Each module sees a Prefixed view that prevents cross-module key collisions. +// +// Intended for tests, local smoke runs, and the no-Firestore fallback. State +// is lost when the process exits. +type MemoryProvider struct { + base *MemoryKVStore +} + +// NewMemoryProvider returns a fresh in-process provider. +func NewMemoryProvider() *MemoryProvider { + return &MemoryProvider{base: NewMemoryKVStore()} +} + +// For returns a per-module view of the shared in-memory store. +func (m *MemoryProvider) For(moduleName string) KVStore { + return Prefixed(m.base, moduleName) +} + +// Base exposes the underlying unprefixed store. Tests use this to assert +// cross-module isolation by reading raw keys; production code must not. +func (m *MemoryProvider) Base() *MemoryKVStore { return m.base }