From 611706aa25356761016e60f2a304c3a59d845f5c Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:21:33 -0700 Subject: [PATCH] feat: added Switcher Remote flag for Hybrid Mode --- .github/workflows/master.yml | 10 ++- .github/workflows/sonar.yml | 2 +- .github/workflows/staging.yml | 8 +++ ARCHITECTURE.md | 71 +++++++++++---------- Makefile | 5 +- README.md | 6 +- switcher.go | 27 +++++++- switcher_test.go | 116 ++++++++++++++++++++++++++++++++++ 8 files changed, 207 insertions(+), 38 deletions(-) diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index e8fece8..fb50f0d 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -29,6 +29,14 @@ jobs: with: version: v2.12.2 + - name: Format + run: | + if [ -n "$(gofmt -s -l .)" ]; then + echo "The following files are not formatted correctly:" + gofmt -s -l . + exit 1 + fi + - name: Test run: go test -p 1 -v ./... -coverprofile="coverage.out" @@ -40,7 +48,7 @@ jobs: - name: Vulnerability Check run: | - go install golang.org/x/vuln/cmd/govulncheck@3e6f44f962742443c11ae2261f02e0c917aeb2bc + go install golang.org/x/vuln/cmd/govulncheck@19b0bb6a272792b9afa8a6983c3e9b9a1816947f govulncheck ./... build-test: diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index f013444..a4b2ca1 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -62,5 +62,5 @@ jobs: - name: Vulnerability Check run: | - go install golang.org/x/vuln/cmd/govulncheck@3e6f44f962742443c11ae2261f02e0c917aeb2bc + go install golang.org/x/vuln/cmd/govulncheck@19b0bb6a272792b9afa8a6983c3e9b9a1816947f govulncheck ./... \ No newline at end of file diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 4c90427..e9acd8d 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -42,5 +42,13 @@ jobs: with: version: v2.12.2 + - name: Format + run: | + if [ -n "$(gofmt -s -l .)" ]; then + echo "The following files are not formatted correctly:" + gofmt -s -l . + exit 1 + fi + - name: Test run: go test -p 1 -v ./... \ No newline at end of file diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index c0108f9..37eff94 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -5,14 +5,6 @@ public API surface down to the individual files that implement it. The package i intentionally flat (`package client`, no internal sub-packages), so this document exists to make the relationships between files explicit. -> **TODO / known gap**: README.md's [Hybrid Mode](README.md#hybrid-mode) section -> documents `client.GetSwitcher("FEATURE01").Remote().IsOn()`, but no `Remote()` -> method exists on `Switcher` in the current codebase (verified against -> `switcher.go`). Either implement `Switcher.Remote()` (a per-call override that -> forces `executionModeRemote`, bypassing `Options.Local`/Silent Mode) or update -> README.md to remove/replace the example. See [§2](#2-api-surface-map-feature--code) -> and [§6](#6-execution-modes-local-remote-silent). - ## Table of Contents - [1. Design Goals](#1-design-goals) @@ -63,7 +55,7 @@ This maps README.md features to the files that implement them. | Prepare/Execute pattern | `Switcher.Prepare` | `switcher.go` | | Error notifications | `SubscribeNotifyError` | `client.go` | | Throttling (stale-while-revalidate) | `Switcher.Throttle` | `switcher.go`, `execution_logger.go` | -| Hybrid mode (force remote) | `Switcher.Remote` *(documented in README; not yet present in code — see note below)* | `switcher.go` | +| Hybrid mode (force remote) | `Switcher.Remote` | `switcher.go` | | Circuit breaker / Silent Mode | `Options.SilentMode` | `client_silent_mode.go` | | Snapshot loading | `LoadSnapshot` | `client.go`, `snapshot.go` | | Snapshot version check | `CheckSnapshot`, `SnapshotVersion` | `client.go`, `remote.go` | @@ -80,39 +72,39 @@ This maps README.md features to the files that implement them. ``` ┌─────────────────────────────┐ │ Application Code (user) │ - └──────────────┬───────────────┘ + └──────────────┬──────────────┘ │ package-level API ▼ ┌─────────────────────────────┐ - │ client.go (facade layer) │ BuildContext / GetSwitcher / - │ defaultClient() singleton │ LoadSnapshot / CheckSnapshot ... - └──────────────┬───────────────┘ + │ client.go (facade layer) │ BuildContext / GetSwitcher / + │ defaultClient() singleton │ LoadSnapshot / CheckSnapshot ... + └──────────────┬──────────────┘ │ delegates to ▼ - ┌─────────────────────────────┐ - │ *Client (core) │ - │ context.go – configuration │ - │ client.go – switcher cache,│ - │ execution log, │ - │ throttle tokens│ - └───┬─────────┬─────────┬───────┘ + ┌─────────────────────────────────┐ + │ *Client (core) │ + │ context.go – configuration │ + │ client.go – switcher cache, │ + │ execution log, │ + │ throttle tokens │ + └───┬─────────┬─────────┬─────────┘ │ │ │ ┌────────────────┘ │ └────────────────┐ ▼ ▼ ▼ ┌───────────────────┐ ┌───────────────────┐ ┌────────────────────┐ - │ switcher.go │ │ remote.go │ │ snapshot.go / │ - │ Switcher (fluent │ │ HTTP transport, │ │ resolver.go │ - │ API + execution │◄───┤ auth, criteria, │ │ snapshot state, │ - │ orchestration) │ │ snapshot fetch │ │ local evaluation │ - └─────────┬───────────┘ └─────────┬──────────┘ └──────────┬─────────┘ - │ │ │ - ▼ ▼ ▼ - ┌───────────────────┐ ┌───────────────────┐ ┌────────────────────┐ - │ execution_logger.go│ │ client_silent_ │ │ local_strategies.go│ - │ (throttle cache/ │ │ mode.go │ │ (per-strategy │ - │ logging) │ │ client_auto_ │ │ criteria engine) │ - │ │ │ renew.go │ │ │ - └───────────────────┘ └───────────────────┘ └────────────────────┘ + │ switcher.go │ │ remote.go │ │ snapshot.go / │ + │ Switcher (fluent │ │ HTTP transport, │ │ resolver.go │ + │ API + execution │◄────┤ auth, criteria, │ │ snapshot state, │ + │ orchestration) │ │ snapshot fetch │ │ local evaluation │ + └─────────┬─────────┘ └─────────┬─────────┘ └──────────┬─────────┘ + │ │ │ + ▼ ▼ ▼ + ┌─────────────────────┐ ┌───────────────────┐ ┌────────────────────┐ + │ execution_logger.go │ │ client_silent_ │ │ local_strategies.go│ + │ (throttle cache/ │ │ mode.go │ │ (per-strategy │ + │ logging) │ │ client_auto_ │ │ criteria engine) │ + │ │ │ renew.go │ │ │ + └─────────────────────┘ └───────────────────┘ └────────────────────┘ Cross-cutting: mock.go (test overrides), errors.go (typed errors), result.go (ResultDetail), snapshot_watcher.go / snapshot_auto_updater.go (background jobs) @@ -206,6 +198,19 @@ This makes Silent Mode a **circuit breaker with local-snapshot fallback** rather a simple retry: local snapshot data must be self-sufficient (no Relay dependency) for switchers evaluated in this mode — see the `RestrictRelay` check in `resolver.go`. +**Hybrid Mode override** (`Switcher.Remote(force ...bool)`, `switcher.go`): a per-call +opt-out from local execution. Setting `forceRemote = true` on a `Switcher` (the +default when calling `Remote()` with no arguments; `Remote(false)` clears it) changes +`resolveExecutionMode`'s first branch from `Options.Local` to +`Options.Local && !forceRemote`, so a forced switcher always falls through to the +`Validate` → token → `executionModeRemote` (or `executionModeSilentLocal`, if a +silent-mode window is active) path even while the client is otherwise configured for +Local Mode. `Validate()` requires `Options.Local == true` whenever `forceRemote` is +set — calling `Remote()` while local mode is disabled has no effect on mode +selection (the client is already remote-only) but does yield a +`"something went wrong: local mode is not enabled"` error from `Validate`, `Prepare`, +`IsOn`, and `IsOnWithDetails`. + ## 7. Snapshot Lifecycle ``` diff --git a/Makefile b/Makefile index 39c96c7..9b7a5b6 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ .PHONY: test fmt cover cover-html lint lint-install godoc-install docs vulncheck vulncheck-install GOLANGCI_LINT_VERSION=v2.12.2 -GOVULNCHECK_VERSION=v1.5.0 +GOVULNCHECK_VERSION=v1.6.0 test-clean: go clean -testcache @@ -9,6 +9,9 @@ test-clean: test: go test -p 1 -v ./... +fmt-check: + gofmt -s -l . + fmt: gofmt -s -w . diff --git a/README.md b/README.md index 049bc54..c9f621a 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,6 @@ A Go SDK for Switcher API [![Quality Gate Status](https://sonarcloud.io/api/project_badges/measure?project=switcherapi_switcher-client-go&metric=alert_status)](https://sonarcloud.io/dashboard?id=switcherapi_switcher-client-go) ![Known Vulnerabilities](https://snyk.io/test/github/switcherapi/switcher-client-go/badge.svg) ![Go](https://img.shields.io/badge/go-1.25%2B-blue.svg) -![Status](https://img.shields.io/badge/status-under_development-orange.svg) ![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg) [![Slack: Switcher-HQ](https://img.shields.io/badge/slack-@switcher/hq-blue.svg?logo=slack)](https://switcher-hq.slack.com/) @@ -377,6 +376,9 @@ fmt.Println(logged.Response.Metadata["cached"]) ``` #### Hybrid Mode + +When `ContextOptions.Local` is enabled, evaluations are resolved against the local snapshot by default. Chain `.Remote()` on a specific Switcher to force that call to always use the remote API instead, without disabling Local Mode for the rest of the client. + ```go _, err := client.GetSwitcher("FEATURE01").Remote().IsOn() if err != nil { @@ -384,6 +386,8 @@ if err != nil { } ``` +`Remote()` requires `ContextOptions.Local` to be `true` — otherwise `Validate`/`IsOn`/`IsOnWithDetails` return an error. Pass `Remote(false)` to explicitly clear the override and fall back to the client's normal `Local`/Silent Mode behavior. + #### Circuit Breaker: Silent Mode This feature allows you to specify how long the client SDK should attempt to restore connectivity in case of remote API failures. diff --git a/switcher.go b/switcher.go index e75ca7d..2af2236 100644 --- a/switcher.go +++ b/switcher.go @@ -18,6 +18,7 @@ type Switcher struct { entries []criteriaEntry throttlePeriod time.Duration nextRefreshAt time.Time + forceRemote bool mu sync.RWMutex } @@ -34,6 +35,11 @@ const ( // Returns an error describing missing fields when validation fails. func (s *Switcher) Validate() error { ctx := s.client.Context() + + if s.forceRemote && !ctx.Options.Local { + return fmt.Errorf("something went wrong: local mode is not enabled") + } + missingFields := make([]string, 0, 3) if strings.TrimSpace(ctx.URL) == "" { @@ -136,6 +142,24 @@ func (s *Switcher) Throttle(period time.Duration) *Switcher { return s } +// Remote forces this Switcher to always use the remote API for evaluation, bypassing +// Options.Local (Hybrid Mode). When called without arguments the override defaults to true; +// passing Remote(false) clears the override so Options.Local is honored normally. +// +// The remote override requires Options.Local to be enabled; otherwise Validate (and therefore +// Prepare/IsOn/IsOnWithDetails) returns an error. +func (s *Switcher) Remote(force ...bool) *Switcher { + s.mu.Lock() + defer s.mu.Unlock() + + s.forceRemote = true + if len(force) > 0 { + s.forceRemote = force[0] + } + + return s +} + // Prepare validates the switcher can be executed and ensures an auth token is present. // If key is non-empty it will be set on the Switcher. Useful when preparing before execution. func (s *Switcher) Prepare(key string) error { @@ -255,7 +279,7 @@ func (s *Switcher) execute(execution *Switcher, showDetails bool) (ResultDetail, } func (s *Switcher) resolveExecutionMode() (executionMode, error) { - if s.client.Context().Options.Local { + if s.client.Context().Options.Local && !s.forceRemote { return executionModeLocal, nil } @@ -322,6 +346,7 @@ func (s *Switcher) snapshotForExecution() *Switcher { entries: clonedEntries, throttlePeriod: s.throttlePeriod, nextRefreshAt: s.nextRefreshAt, + forceRemote: s.forceRemote, } } diff --git a/switcher_test.go b/switcher_test.go index 5b11637..352d1d2 100644 --- a/switcher_test.go +++ b/switcher_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSwitcherValidate(t *testing.T) { @@ -137,3 +138,118 @@ func TestSwitcherMustOrDefault(t *testing.T) { assert.Equal(t, ResultDetail{Result: true, Reason: "Success", Metadata: map[string]any{"env": "prod"}}, gotD) }) } + +func TestSwitcherRemoteHybrid(t *testing.T) { + t.Run("should call the remote API even when local mode is enabled and a snapshot is loaded", func(t *testing.T) { + server := newRemoteTestServer(t, remoteTestHandlers{ + authStatus: http.StatusOK, + authBody: map[string]any{"token": "[token]", "exp": time.Now().Add(time.Hour).Unix()}, + criteriaStatus: http.StatusOK, + // Local snapshot has FF2FOR2022 activated (true) with no strategies; the remote + // mock returns false so we can prove the remote path (not local) was used. + criteriaBody: map[string]any{"result": false, "reason": "remote"}, + }) + defer server.Close() + + client := NewClient(Context{ + Domain: "My Domain", + URL: server.URL, + APIKey: "[YOUR_API_KEY]", + Component: "MyApp", + Options: ContextOptions{ + Local: true, + SnapshotLocation: snapshotFixtureDir(), + }, + }) + + _, loadErr := client.LoadSnapshot(nil) + require.NoError(t, loadErr) + + // sanity check: plain local evaluation returns true and never hits the server. + localResult, localErr := client.GetSwitcher("FF2FOR2022").IsOn() + require.NoError(t, localErr) + assert.True(t, localResult) + + remoteResult, remoteErr := client.GetSwitcher("FF2FOR2022").Remote().IsOn() + require.NoError(t, remoteErr) + assert.False(t, remoteResult) + }) + + t.Run("should return an error from Validate/IsOn when local mode is not enabled", func(t *testing.T) { + client := NewClient(Context{ + Domain: "My Domain", + URL: "https://api.switcherapi.com", + APIKey: "[YOUR_API_KEY]", + Component: "MyApp", + }) + + switcher := client.GetSwitcher("FEATURE_LOGIN_V2").Remote() + + err := switcher.Validate() + assert.EqualError(t, err, "something went wrong: local mode is not enabled") + + _, isOnErr := switcher.IsOn() + assert.EqualError(t, isOnErr, "something went wrong: local mode is not enabled") + }) + + t.Run("should behave as a no-op override when Remote(false) is used", func(t *testing.T) { + server := newRemoteTestServer(t, remoteTestHandlers{ + authStatus: http.StatusOK, + authBody: map[string]any{"token": "[token]", "exp": time.Now().Add(time.Hour).Unix()}, + criteriaStatus: http.StatusOK, + criteriaBody: map[string]any{"result": false, "reason": "remote"}, + }) + defer server.Close() + + client := NewClient(Context{ + Domain: "My Domain", + URL: server.URL, + APIKey: "[YOUR_API_KEY]", + Component: "MyApp", + Options: ContextOptions{ + Local: true, + SnapshotLocation: snapshotFixtureDir(), + }, + }) + + _, loadErr := client.LoadSnapshot(nil) + require.NoError(t, loadErr) + + result, err := client.GetSwitcher("FF2FOR2022").Remote(false).IsOn() + require.NoError(t, err) + assert.True(t, result) + }) + + t.Run("should compose with Check*, Throttle and IsOnWithDetails chains", func(t *testing.T) { + server := newRemoteTestServer(t, remoteTestHandlers{ + authStatus: http.StatusOK, + authBody: map[string]any{"token": "[token]", "exp": time.Now().Add(time.Hour).Unix()}, + criteriaStatus: http.StatusOK, + criteriaBody: map[string]any{"result": true, "reason": "Success"}, + }) + defer server.Close() + + client := NewClient(Context{ + Domain: "My Domain", + URL: server.URL, + APIKey: "[YOUR_API_KEY]", + Component: "MyApp", + Options: ContextOptions{ + Local: true, + SnapshotLocation: snapshotFixtureDir(), + }, + }) + + _, loadErr := client.LoadSnapshot(nil) + require.NoError(t, loadErr) + + result, err := client.GetSwitcher("FF2FOR2022"). + Remote(). + CheckValue("USER_1"). + Throttle(time.Second). + IsOnWithDetails() + + require.NoError(t, err) + assert.Equal(t, ResultDetail{Result: true, Reason: "Success", Metadata: map[string]any{}}, result) + }) +}