From 4b15d07d1b559f72527294aca0eea64451c50fe9 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Sun, 28 Jun 2026 12:52:40 +0700 Subject: [PATCH] refactor(storage): remove legacy firestore backend Delete Firestore provider implementation and tests (5 files). Extract shared key helpers to new internal/storage/keys.go module. Remove Firestore case from server bootstrap and storage factory. Drop cloud.google.com/go/firestore and related dependencies from go.mod. Remove firestore-emulator test targets from Makefile. Update README storage section to reflect MongoDB-only backend. --- .github/workflows/ci.yml | 7 +- Makefile | 17 +- README.md | 2 +- cmd/server/main.go | 45 +--- go.mod | 12 +- go.sum | 29 +-- internal/storage/firestore_client.go | 30 --- internal/storage/firestore_kv.go | 266 -------------------- internal/storage/firestore_kv_test.go | 245 ------------------ internal/storage/firestore_provider.go | 38 --- internal/storage/firestore_provider_test.go | 45 ---- internal/storage/keys.go | 66 +++++ internal/storage/keys_test.go | 40 +++ 13 files changed, 123 insertions(+), 719 deletions(-) delete mode 100644 internal/storage/firestore_client.go delete mode 100644 internal/storage/firestore_kv.go delete mode 100644 internal/storage/firestore_kv_test.go delete mode 100644 internal/storage/firestore_provider.go delete mode 100644 internal/storage/firestore_provider_test.go create mode 100644 internal/storage/keys.go create mode 100644 internal/storage/keys_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7a7d811..ba9a11c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,10 +47,9 @@ jobs: go install golang.org/x/vuln/cmd/govulncheck@latest govulncheck ./... - # Firestore emulator step removed: storage_test.go skips gracefully - # when FIRESTORE_EMULATOR_HOST is unset, and the emulator install - # adds 30-60s of CI time for tests not yet on the merge-gating path. - # Re-add when storage-layer changes need emulator coverage in CI. + # No DB emulator step: storage integration tests (mongodb, dynamodb) skip + # gracefully when MONGODB_TEST_URL / DYNAMODB_LOCAL_URL are unset. Run them + # locally via `make test-mongo` / `make test-dynamodb`. - name: go test env: # Quiet test logs so real failures stand out. diff --git a/Makefile b/Makefile index b7079f6..b546d9b 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: help test test-emulator test-dynamodb test-mongo firestore-emulator dynamodb-local dynamodb-local-stop mongo-local mongo-local-stop vet build build-lambda run sam-validate sam-build sam-deploy telegram-setup telegram-webhook telegram-webhook-info telegram-commands telegram-commands-info telegram-commands-selfhost telegram-deletewebhook-selfhost telegram-webhook-info-selfhost migrate-dynamo-to-mongo migrate-verify logs clean +.PHONY: help test test-dynamodb test-mongo dynamodb-local dynamodb-local-stop mongo-local mongo-local-stop vet build build-lambda run sam-validate sam-build sam-deploy telegram-setup telegram-webhook telegram-webhook-info telegram-commands telegram-commands-info telegram-commands-selfhost telegram-deletewebhook-selfhost telegram-webhook-info-selfhost migrate-dynamo-to-mongo migrate-verify logs clean # Lambda target architecture. Match Globals.Architectures in template.yaml. LAMBDA_GOOS ?= linux @@ -28,19 +28,6 @@ help: ## Show this help test: ## Unit tests (no emulator required) 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: ## Start Firestore emulator on :8085 (foreground) - gcloud emulators firestore start --host-port=localhost:8085 - -# Run all tests including Firestore-emulator-gated ones. Expects the emulator -# to already be running (use `make firestore-emulator` in another shell). -test-emulator: ## Run tests with Firestore emulator (must be running) - FIRESTORE_EMULATOR_HOST=localhost:8085 \ - GOOGLE_CLOUD_PROJECT=miti99bot-test \ - go test -race -count=1 ./... - # Run DynamoDB integration tests against DynamoDB Local. # Override DDB_PORT if 8001 is taken on your host. DDB_PORT ?= 8001 @@ -75,7 +62,7 @@ build-lambda: ## Cross-compile bootstrap for Lambda (linux/arm64) # ---- Run ------------------------------------------------------------------ -# Local dev run with an in-memory KV (no Firestore / DynamoDB needed). +# Local dev run with an in-memory KV (no database needed). run: ## Run locally (in-memory KV) go run ./cmd/server diff --git a/README.md b/README.md index ebdfc9c..3dc67b7 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ internal/server/ HTTP routes (/ health, /cron/{name} manual trigger) internal/telegram/ Telegram long-polling bot wrapper internal/cron/ in-process cron scheduler (replaces EventBridge) internal/modules/ Module framework, registry, dispatchers, modules -internal/storage/ KVProvider interface; memory + firestore + dynamodb + mongodb +internal/storage/ KVProvider interface; memory + dynamodb + mongodb (values stored as native BSON documents) internal/ai/ Gemini client (used by twentyq) docker-compose.yml Coolify self-host stack (single bot service) docs/deploy-coolify-selfhosted.md Self-host onboarding + cutover runbook diff --git a/cmd/server/main.go b/cmd/server/main.go index d1c7988..d32213b 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -60,11 +60,6 @@ func factories() map[string]modules.Factory { } } -// firestoreInitTimeout caps Firestore 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 Lambda restart us. -const firestoreInitTimeout = 10 * time.Second - // dynamodbInitTimeout caps DynamoDB client construction at startup. Lambda // has a 10s init phase; we want to leave headroom for module wiring. const dynamodbInitTimeout = 5 * time.Second @@ -215,13 +210,13 @@ func main() { } // buildProvider picks the storage backend. Selection order: -// 1. Explicit KV_PROVIDER env (memory|firestore|dynamodb|mongodb) wins. +// 1. Explicit KV_PROVIDER env (memory|dynamodb|mongodb) wins. // 2. Auto-detect: MONGO_URL set → mongodb; otherwise memory. // // The self-host default is mongodb (just set MONGO_URL + MONGO_DATABASE — no -// KV_PROVIDER needed). dynamodb/firestore remain reachable only via an explicit -// KV_PROVIDER, kept for the data migrator and the integration tests; the old -// AWS_LAMBDA_FUNCTION_NAME auto-detect is removed (AWS is decommissioned). +// KV_PROVIDER needed). dynamodb remains reachable only via an explicit +// KV_PROVIDER, kept for the data migrator and integration tests. The legacy +// firestore backend and the AWS_LAMBDA_FUNCTION_NAME auto-detect are removed. // // Returned closer is always non-nil and safe to call exactly once. func buildProvider(ctx context.Context, cfg config) (storage.KVProvider, func(), error) { @@ -266,30 +261,6 @@ func buildProvider(ctx context.Context, cfg config) (storage.KVProvider, func(), log.Info("storage backend", "backend", "mongodb", "database", cfg.MongoDatabase) return storage.NewMongoProvider(db), closer, nil - case "firestore": - // 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.FirestoreProject - 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.Error("firestore close failed", "err", err) - } - } - log.Info("storage backend", - "backend", "firestore", - "project", projectID, - "emulator", cfg.FirestoreEmulatorHost) - return storage.NewFirestoreProvider(client), closer, nil - case "dynamodb": if cfg.DynamoDBTable == "" { return nil, func() {}, errors.New("KV_PROVIDER=dynamodb requires DYNAMODB_TABLE") @@ -308,7 +279,7 @@ func buildProvider(ctx context.Context, cfg config) (storage.KVProvider, func(), return storage.NewDynamoDBProvider(client, cfg.DynamoDBTable), func() {}, nil default: - return nil, func() {}, fmt.Errorf("unknown KV_PROVIDER %q (want memory|firestore|dynamodb|mongodb)", backend) + return nil, func() {}, fmt.Errorf("unknown KV_PROVIDER %q (want memory|dynamodb|mongodb)", backend) } } @@ -316,8 +287,6 @@ type config struct { Port string TelegramBotToken string CronSecret string - FirestoreProject string - FirestoreEmulatorHost string GeminiAPIKey string GoldPriceAPIURL string GoldFXAPIURL string @@ -329,7 +298,7 @@ type config struct { Modules []string BotOwnerID int64 AdminUserIDs map[int64]bool - KVProvider string // empty = auto-detect; or "memory"|"firestore"|"dynamodb"|"mongodb" + KVProvider string // empty = auto-detect; or "memory"|"dynamodb"|"mongodb" DynamoDBTable string // required when KVProvider=dynamodb MongoURL string // required when KVProvider=mongodb (Atlas SRV connection string; SECRET — never log) MongoDatabase string // required when KVProvider=mongodb @@ -360,8 +329,6 @@ func loadConfig() config { Port: port, TelegramBotToken: envMap["TELEGRAM_BOT_TOKEN"], CronSecret: envMap["CRON_SHARED_SECRET"], - FirestoreProject: envMap["GOOGLE_CLOUD_PROJECT"], - FirestoreEmulatorHost: envMap["FIRESTORE_EMULATOR_HOST"], GeminiAPIKey: envMap["GEMINI_API_KEY"], GoldPriceAPIURL: envMap["GOLD_PRICE_API_URL"], GoldFXAPIURL: envMap["GOLD_FX_API_URL"], diff --git a/go.mod b/go.mod index b374456..2c76607 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/tiennm99/miti99bot go 1.25.0 require ( - cloud.google.com/go/firestore v1.22.0 github.com/aws/aws-sdk-go-v2 v1.41.7 github.com/aws/aws-sdk-go-v2/config v1.32.17 github.com/aws/aws-sdk-go-v2/service/dynamodb v1.57.3 @@ -12,17 +11,13 @@ require ( github.com/robfig/cron/v3 v3.0.1 go.mongodb.org/mongo-driver/v2 v2.7.0 golang.org/x/time v0.15.0 - google.golang.org/api v0.274.0 google.golang.org/genai v1.56.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/aws/aws-sdk-go-v2/credentials v1.19.16 // indirect github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.23 // indirect github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.23 // indirect @@ -51,19 +46,18 @@ require ( github.com/xdg-go/stringprep v1.0.4 // indirect github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78 // 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/sdk v1.43.0 // indirect + go.opentelemetry.io/otel/sdk/metric v1.43.0 // indirect go.opentelemetry.io/otel/trace v1.43.0 // indirect golang.org/x/crypto v0.51.0 // indirect golang.org/x/net v0.54.0 // indirect - golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.20.0 // indirect golang.org/x/sys v0.44.0 // indirect golang.org/x/text v0.37.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/grpc v1.80.0 // indirect google.golang.org/protobuf v1.36.11 // indirect ) diff --git a/go.sum b/go.sum index 71eac91..a226ef9 100644 --- a/go.sum +++ b/go.sum @@ -2,14 +2,8 @@ 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/aws/aws-sdk-go-v2 v1.41.7 h1:DWpAJt66FmnnaRIOT/8ASTucrvuDPZASqhhLey6tLY8= github.com/aws/aws-sdk-go-v2 v1.41.7/go.mod h1:4LAfZOPHNVNQEckOACQx60Y8pSRjIkNZQz1w92xpMJc= github.com/aws/aws-sdk-go-v2/config v1.32.17 h1:FpL4/758/diKwqbytU0prpuiu60fgXKUWCpDJtApclU= @@ -46,15 +40,8 @@ github.com/aws/smithy-go v1.25.1 h1:J8ERsGSU7d+aCmdQur5Txg6bVoYelvQJgtZehD12GkI= github.com/aws/smithy-go v1.25.1/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= 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= @@ -80,8 +67,6 @@ github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aN github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/klauspost/compress v1.17.6 h1:60eq2E/jlfwQXtvZEeBUYADs+BwKBWURIY+Gj2eRGjI= github.com/klauspost/compress v1.17.6/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= -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/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= @@ -101,8 +86,6 @@ go.mongodb.org/mongo-driver/v2 v2.7.0 h1:RO+zqavD2/GCL3cxOMyZhx6R9Irzr8/6gsoqx5t go.mongodb.org/mongo-driver/v2 v2.7.0/go.mod h1:yOI9kBsufol30iFsl1slpdq1I0eHPzybRWdyYUs8K/0= 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= @@ -111,8 +94,8 @@ go.opentelemetry.io/otel/metric v1.43.0 h1:d7638QeInOnuwOONPp4JAOGfbCEpYb+K6DVWv 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/sdk/metric v1.43.0 h1:S88dyqXjJkuBNLeMcVPRFXpRw2fuwdvfCGLEo89fDkw= +go.opentelemetry.io/otel/sdk/metric v1.43.0/go.mod h1:C/RJtwSEJ5hzTiUz5pXF1kILHStzb9zFlIEe85bhj6A= 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.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= @@ -125,8 +108,6 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= -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.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= @@ -154,14 +135,8 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= 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/genai v1.56.0 h1:IwWrg1K0cn1/WBiPno/dYr0Q6o75NeH/bh3G4JEFERE= google.golang.org/genai v1.56.0/go.mod h1:A3kkl0nyBjyFlNjgxIwKq70julKbIxpSxqKO5gw/gmk= -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= diff --git a/internal/storage/firestore_client.go b/internal/storage/firestore_client.go deleted file mode 100644 index 58dc72a..0000000 --- a/internal/storage/firestore_client.go +++ /dev/null @@ -1,30 +0,0 @@ -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 deleted file mode 100644 index fb56eea..0000000 --- a/internal/storage/firestore_kv.go +++ /dev/null @@ -1,266 +0,0 @@ -package storage - -import ( - "bytes" - "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 -} - -func (s *FirestoreKVStore) CompareAndSwap(ctx context.Context, key string, expected []byte, val []byte) error { - if err := validateKey(key); err != nil { - return err - } - ref := s.doc(key) - err := s.client.RunTransaction(ctx, func(ctx context.Context, tx *firestore.Transaction) error { - snap, err := tx.Get(ref) - if err != nil { - if status.Code(err) == codes.NotFound && expected == nil { - return tx.Set(ref, map[string]any{ - firestoreValueField: val, - firestoreUpdatedAtField: time.Now().UTC(), - }) - } - if status.Code(err) == codes.NotFound { - return ErrConflict - } - return err - } - raw, err := snap.DataAt(firestoreValueField) - if err != nil { - return fmt.Errorf("missing %q field: %w", firestoreValueField, err) - } - var current []byte - switch v := raw.(type) { - case []byte: - current = v - case string: - current = []byte(v) - default: - return fmt.Errorf("unexpected value type %T", raw) - } - if expected == nil || !bytes.Equal(current, expected) { - return ErrConflict - } - return tx.Set(ref, map[string]any{ - firestoreValueField: val, - firestoreUpdatedAtField: time.Now().UTC(), - }) - }) - if err != nil { - if errors.Is(err, ErrConflict) { - return ErrConflict - } - return fmt.Errorf("firestore compare-and-swap %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 deleted file mode 100644 index 6bd64e0..0000000 --- a/internal/storage/firestore_kv_test.go +++ /dev/null @@ -1,245 +0,0 @@ -package storage - -import ( - "context" - "errors" - "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-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_CompareAndSwap(t *testing.T) { - c := requireEmulator(t) - col := uniqueCollection(t) - defer drainCollection(t, c, col) - store := NewFirestoreKVStore(c, col) - - ctx := context.Background() - - // Create-if-absent succeeds once, then conflicts. - if err := store.CompareAndSwap(ctx, "user:1", nil, []byte("v1")); err != nil { - t.Fatalf("CompareAndSwap create: %v", err) - } - if err := store.CompareAndSwap(ctx, "user:1", nil, []byte("v2")); !errors.Is(err, ErrConflict) { - t.Errorf("CompareAndSwap create over existing: got %v, want ErrConflict", err) - } - - // Swap with matching expected succeeds; stale expected conflicts. - if err := store.CompareAndSwap(ctx, "user:1", []byte("v1"), []byte("v2")); err != nil { - t.Fatalf("CompareAndSwap matching: %v", err) - } - if err := store.CompareAndSwap(ctx, "user:1", []byte("v1"), []byte("v3")); !errors.Is(err, ErrConflict) { - t.Errorf("CompareAndSwap stale: got %v, want ErrConflict", err) - } - got, err := store.Get(ctx, "user:1") - if err != nil { - t.Fatalf("Get: %v", err) - } - if string(got) != "v2" { - t.Errorf("stored value = %q, want %q", got, "v2") - } - - // Non-nil expected on a missing key conflicts (caller reloads and retries). - if err := store.CompareAndSwap(ctx, "user:missing", []byte("v1"), []byte("v2")); !errors.Is(err, ErrConflict) { - t.Errorf("CompareAndSwap missing key: got %v, want ErrConflict", 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 deleted file mode 100644 index a87e529..0000000 --- a/internal/storage/firestore_provider.go +++ /dev/null @@ -1,38 +0,0 @@ -package storage - -import ( - "regexp" - - "cloud.google.com/go/firestore" -) - -// collectionNameRe mirrors modules.moduleNameRe. Defense-in-depth: callers -// should validate first (modules.Build does), but a junk collection name -// that escapes validation could let any caller drop docs into someone -// else's namespace. Match the canonical alphabet here too. -var collectionNameRe = regexp.MustCompile(`^[a-z0-9_-]{1,32}$`) - -// 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. moduleName is re-validated against collectionNameRe — defense in -// depth against caller bugs that bypass modules.Build. An invalid name -// returns a store whose every operation errors with ErrInvalidModuleName, -// so the bug surfaces at first use rather than silently writing to a -// junk-named collection. -func (p *FirestoreProvider) For(moduleName string) KVStore { - if !collectionNameRe.MatchString(moduleName) { - return invalidStore{name: moduleName} - } - return NewFirestoreKVStore(p.client, moduleName) -} diff --git a/internal/storage/firestore_provider_test.go b/internal/storage/firestore_provider_test.go deleted file mode 100644 index 9587a0d..0000000 --- a/internal/storage/firestore_provider_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package storage - -import ( - "context" - "errors" - "testing" -) - -// FirestoreProvider.For re-validates the module name as defense-in-depth. -// We can't actually exercise valid names without a Firestore client, but -// invalid names return invalidStore (no client touched), which is the -// branch worth locking. -func TestFirestoreProvider_For_RejectsInvalidName(t *testing.T) { - p := &FirestoreProvider{client: nil} - - bogus := []string{ - "", // empty - "with spaces", // not allowed - "WITHCAPS", // not allowed - "path/traversal", // attempted slash injection - "../etc/passwd", // attempted traversal - "way-too-long-for-our-32-char-limit-x", // exceeds 32 chars - "with:colon", // explicit ban — colon is the prefixed-store delimiter - } - for _, name := range bogus { - store := p.For(name) - _, err := store.Get(context.Background(), "any-key") - if !errors.Is(err, ErrInvalidModuleName) { - t.Errorf("For(%q).Get → %v, want ErrInvalidModuleName", name, err) - } - } -} - -func TestFirestoreProvider_For_AcceptsCanonicalNames(t *testing.T) { - // Canonical names match the regex: lowercase + digits + underscore + hyphen, - // 1..32 chars. We can't dereference the returned FirestoreKVStore (nil - // client), but we can assert it's NOT an invalidStore — validation passed. - p := &FirestoreProvider{client: nil} - for _, name := range []string{"misc", "demo-mod", "wordle", "x", "a1_b-2"} { - store := p.For(name) - if _, ok := store.(invalidStore); ok { - t.Errorf("For(%q) returned invalidStore; expected validation to pass", name) - } - } -} diff --git a/internal/storage/keys.go b/internal/storage/keys.go new file mode 100644 index 0000000..79ab0dd --- /dev/null +++ b/internal/storage/keys.go @@ -0,0 +1,66 @@ +package storage + +import ( + "fmt" + "regexp" + "strings" +) + +// maxKeyLen bounds a key's byte length. Originally Firestore's document-id cap; +// retained as a backend-neutral guard so keys behave identically across all +// backends (len(string) is bytes, not runes — do NOT switch to RuneCount). +const maxKeyLen = 1500 + +// collectionNameRe is the canonical module-name alphabet (mirrors +// modules.moduleNameRe). Providers re-validate module names against it as +// defense-in-depth before using a name as a collection/partition. +var collectionNameRe = regexp.MustCompile(`^[a-z0-9_-]{1,32}$`) + +// validateKey enforces key constraints up-front so callers get a clear error +// instead of an opaque backend error. Kept uniform across backends for parity. +// +// Forbidden: empty; longer than maxKeyLen bytes; contains '/'; "." or ".."; +// leading+trailing "__" (reserved namespace). +func validateKey(key string) error { + if key == "" { + return fmt.Errorf("storage: key is empty") + } + if len(key) > maxKeyLen { + return fmt.Errorf("storage: key exceeds %d bytes", maxKeyLen) + } + if strings.Contains(key, "/") { + return fmt.Errorf("storage: key contains '/'") + } + 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 validateKey but allows the empty string (List with an +// empty prefix scans the whole collection/partition). +func validatePrefix(prefix string) error { + if prefix == "" { + return nil + } + return validateKey(prefix) +} + +// prefixSuccessor returns the smallest string strictly greater than every +// string with the given prefix, for half-open range scans on keys. "abc" → "abd". +// Trailing 0xFF bytes are stripped and the last < 0xFF byte incremented. An +// all-0xFF prefix has no same-length successor and is returned unchanged +// (callers using such keys get an empty range — acceptable). +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]) + } + } + return prefix +} diff --git a/internal/storage/keys_test.go b/internal/storage/keys_test.go new file mode 100644 index 0000000..a9cd8a5 --- /dev/null +++ b/internal/storage/keys_test.go @@ -0,0 +1,40 @@ +package storage + +import "testing" + +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) + } + } +} + +func TestValidateKey(t *testing.T) { + valid := []string{"user:1", "a", "config:daily", "u1_b-2", "vnappmob:api_key"} + for _, k := range valid { + if err := validateKey(k); err != nil { + t.Errorf("validateKey(%q) = %v, want nil", k, err) + } + } + invalid := []string{"", "path/sep", ".", "..", "__reserved__"} + for _, k := range invalid { + if err := validateKey(k); err == nil { + t.Errorf("validateKey(%q) = nil, want error", k) + } + } + // validatePrefix permits empty (whole-collection scan). + if err := validatePrefix(""); err != nil { + t.Errorf("validatePrefix(\"\") = %v, want nil", err) + } + if err := validatePrefix("bad/prefix"); err == nil { + t.Error("validatePrefix(\"bad/prefix\") = nil, want error") + } +}