diff --git a/remoteconfig/condition_evaluator.go b/remoteconfig/condition_evaluator.go index 24889802..31e8815d 100644 --- a/remoteconfig/condition_evaluator.go +++ b/remoteconfig/condition_evaluator.go @@ -148,11 +148,13 @@ func (ce *conditionEvaluator) evaluatePercentCondition(percentCondition *percent } func computeInstanceMicroPercentile(seed string, randomizationID string) uint32 { - seedPrefix := "" + var sb strings.Builder if len(seed) > 0 { - seedPrefix = fmt.Sprintf("%s.", seed) + sb.WriteString(seed) + sb.WriteRune('.') } - stringToHash := fmt.Sprintf("%s%s", seedPrefix, randomizationID) + sb.WriteString(randomizationID) + stringToHash := sb.String() hash := sha256.New() hash.Write([]byte(stringToHash)) diff --git a/remoteconfig/condition_evaluator_test.go b/remoteconfig/condition_evaluator_test.go index f75b8bb5..c7265cff 100644 --- a/remoteconfig/condition_evaluator_test.go +++ b/remoteconfig/condition_evaluator_test.go @@ -296,41 +296,41 @@ func TestPercentConditionMicroPercent(t *testing.T) { }{ { description: "Evaluate LESS_OR_EQUAL to true when MicroPercent is max", - operator: "LESS_OR_EQUAL", + operator: lessThanOrEqual, microPercent: 100_000_000, outcome: true, }, { description: "Evaluate LESS_OR_EQUAL to false when MicroPercent is min", - operator: "LESS_OR_EQUAL", + operator: lessThanOrEqual, microPercent: 0, outcome: false, }, { description: "Evaluate LESS_OR_EQUAL to false when MicroPercent is not set (MicroPercent should use zero)", - operator: "LESS_OR_EQUAL", + operator: lessThanOrEqual, outcome: false, }, { description: "Evaluate GREATER_THAN to true when MicroPercent is not set (MicroPercent should use zero)", - operator: "GREATER_THAN", + operator: greaterThan, outcome: true, }, { description: "Evaluate GREATER_THAN max to false", - operator: "GREATER_THAN", + operator: greaterThan, outcome: false, microPercent: 100_000_000, }, { description: "Evaluate LESS_OR_EQUAL to 9571542 to true", - operator: "LESS_OR_EQUAL", + operator: lessThanOrEqual, microPercent: 9_571_542, // instanceMicroPercentile of abcdef.123 (testSeed.testRandomizationID) is 9_571_542 outcome: true, }, { description: "Evaluate greater than 9571542 to true", - operator: "GREATER_THAN", + operator: greaterThan, microPercent: 9_571_541, // instanceMicroPercentile of abcdef.123 (testSeed.testRandomizationID) is 9_571_542 outcome: true, }, @@ -361,40 +361,40 @@ func TestPercentConditionMicroPercentRange(t *testing.T) { }{ { description: "Evaluate to false when microPercentRange is not set", - operator: "BETWEEN", + operator: between, outcome: false, }, { description: "Evaluate to false when upper bound is not set", microPercentLb: 0, - operator: "BETWEEN", + operator: between, outcome: false, }, { description: "Evaluate to true when lower bound is not set and upper bound is max", microPercentUb: 100_000_000, - operator: "BETWEEN", + operator: between, outcome: true, }, { description: "Evaluate to true when between lower and upper bound", // instanceMicroPercentile of abcdef.123 (testSeed.testRandomizationID) is 9_571_542 microPercentLb: 9_000_000, microPercentUb: 9_571_542, // interval is (9_000_000, 9_571_542] - operator: "BETWEEN", + operator: between, outcome: true, }, { description: "Evaluate to false when lower and upper bounds are equal", microPercentLb: 98_000_000, microPercentUb: 98_000_000, - operator: "BETWEEN", + operator: between, outcome: false, }, { description: "Evaluate to false when not between 9_400_000 and 9_500_000", // instanceMicroPercentile of abcdef.123 (testSeed.testRandomizationID) is 9_571_542 microPercentLb: 9_400_000, microPercentUb: 9_500_000, - operator: "BETWEEN", + operator: between, outcome: false, }, } diff --git a/remoteconfig/remoteconfig.go b/remoteconfig/remoteconfig.go index c212fb07..45a87c77 100644 --- a/remoteconfig/remoteconfig.go +++ b/remoteconfig/remoteconfig.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package remoteconfig allows clients to use Firebase Remote Config with Go. +// Package remoteconfig provides functions to fetch and evaluate a server-side Remote Config template. package remoteconfig import ( @@ -67,6 +67,7 @@ func newRcClient(client *internal.HTTPClient, conf *internal.RemoteConfigClientC internal.WithHeader("x-goog-api-client", internal.GetMetricsHeader(conf.Version)), } + // Handles errors for non-success HTTP status codes from Remote Config servers. client.CreateErrFn = handleRemoteConfigError return &rcClient{ @@ -77,7 +78,7 @@ func newRcClient(client *internal.HTTPClient, conf *internal.RemoteConfigClientC } } -// GetServerTemplate Initializes a new ServerTemplate instance and fetches the server template. +// GetServerTemplate initializes a new ServerTemplate instance and fetches the server template. func (c *rcClient) GetServerTemplate(ctx context.Context, defaultConfig map[string]any) (*ServerTemplate, error) { template, err := c.InitServerTemplate(defaultConfig, "") diff --git a/remoteconfig/server_config.go b/remoteconfig/server_config.go index 2e5b04d7..d1b68db1 100644 --- a/remoteconfig/server_config.go +++ b/remoteconfig/server_config.go @@ -15,6 +15,7 @@ package remoteconfig import ( + "slices" "strconv" "strings" ) @@ -26,8 +27,8 @@ type ValueSource int const ( sourceUnspecified ValueSource = iota Static // Static represents a statically defined value. - Remote // Default represents a default value. - Default // Remote represents a value fetched from a remote source. + Remote // Remote represents a value fetched from a remote source. + Default // Default represents a default value. ) // Value defines the interface for configuration values. @@ -38,39 +39,50 @@ type value struct { // Default Values for config parameters. const ( - DefaultValueForBoolean = false - DefaultValueForString = "" - DefaultValueForNumber = 0 + defaultValueForBoolean = false + defaultValueForString = "" + defaultValueForNumber = 0 ) var booleanTruthyValues = []string{"1", "true", "t", "yes", "y", "on"} // ServerConfig is the implementation of the ServerConfig interface. type ServerConfig struct { - ConfigValues map[string]value + configValues map[string]value } // NewServerConfig creates a new ServerConfig instance. -func NewServerConfig(configValues map[string]value) *ServerConfig { - return &ServerConfig{ConfigValues: configValues} +func newServerConfig(configValues map[string]value) *ServerConfig { + return &ServerConfig{configValues: configValues} } // GetBoolean returns the boolean value associated with the given key. +// +// It returns true if the string value is "1", "true", "t", "yes", "y", or "on" (case-insensitive). +// Otherwise, or if the key is not found, it returns the default boolean value (false). func (s *ServerConfig) GetBoolean(key string) bool { return s.getValue(key).asBoolean() } // GetInt returns the integer value associated with the given key. +// +// If the parameter value cannot be parsed as an integer, or if the key is not found, +// it returns the default numeric value (0). func (s *ServerConfig) GetInt(key string) int { return s.getValue(key).asInt() } // GetFloat returns the float value associated with the given key. +// +// If the parameter value cannot be parsed as a float64, or if the key is not found, +// it returns the default float value (0). func (s *ServerConfig) GetFloat(key string) float64 { return s.getValue(key).asFloat() } // GetString returns the string value associated with the given key. +// +// If the key is not found, it returns the default string value (""). func (s *ServerConfig) GetString(key string) string { return s.getValue(key).asString() } @@ -82,16 +94,16 @@ func (s *ServerConfig) GetValueSource(key string) ValueSource { // getValue returns the value associated with the given key. func (s *ServerConfig) getValue(key string) *value { - if val, ok := s.ConfigValues[key]; ok { + if val, ok := s.configValues[key]; ok { return &val } - return newValue(Static, "") + return newValue(Static, defaultValueForString) } // newValue creates a new value instance. func newValue(source ValueSource, customValue string) *value { if customValue == "" { - customValue = DefaultValueForString + customValue = defaultValueForString } return &value{source: source, value: customValue} } @@ -104,41 +116,35 @@ func (v *value) asString() string { // asBoolean returns the value as a boolean. func (v *value) asBoolean() bool { if v.source == Static { - return DefaultValueForBoolean - } - - for _, truthyValue := range booleanTruthyValues { - if strings.ToLower(v.value) == truthyValue { - return true - } + return defaultValueForBoolean } - return false + return slices.Contains(booleanTruthyValues, strings.ToLower(v.value)) } // asInt returns the value as an integer. func (v *value) asInt() int { if v.source == Static { - return DefaultValueForNumber + return defaultValueForNumber } num, err := strconv.Atoi(v.value) if err != nil { - return DefaultValueForNumber + return defaultValueForNumber } return num } -// asFloat returns the value as an integer. +// asFloat returns the value as a float. func (v *value) asFloat() float64 { if v.source == Static { - return DefaultValueForNumber + return defaultValueForNumber } - num, err := strconv.ParseFloat(v.value, 64) + num, err := strconv.ParseFloat(v.value, doublePrecision) if err != nil { - return DefaultValueForNumber + return defaultValueForNumber } return num diff --git a/remoteconfig/server_config_test.go b/remoteconfig/server_config_test.go new file mode 100644 index 00000000..0a643489 --- /dev/null +++ b/remoteconfig/server_config_test.go @@ -0,0 +1,111 @@ +package remoteconfig + +import "testing" + +type configGetterTestCase struct { + name string + key string + expectedString string + expectedInt int + expectedBool bool + expectedFloat float64 + expectedSource ValueSource +} + +func getTestConfig() ServerConfig { + config := ServerConfig{ + configValues: map[string]value{ + paramOne: { + value: valueOne, + source: Default, + }, + paramTwo: { + value: valueTwo, + source: Remote, + }, + paramThree: { + value: valueThree, + source: Default, + }, + paramFour: { + value: valueFour, + source: Remote, + }, + }, + } + return config +} + +func TestServerConfigGetters(t *testing.T) { + config := getTestConfig() + testCases := []configGetterTestCase{ + { + name: "Parameter Value : String, Default Source", + key: paramOne, + expectedString: valueOne, + expectedInt: 0, + expectedBool: false, + expectedFloat: 0, + expectedSource: Default, + }, + { + name: "Parameter Value : JSON, Remote Source", + key: paramTwo, + expectedString: valueTwo, + expectedInt: 0, + expectedBool: false, + expectedFloat: 0, + expectedSource: Remote, + }, + { + name: "Unknown Parameter Value", + key: "unknown_param", + expectedString: "", + expectedInt: 0, + expectedBool: false, + expectedFloat: 0, + expectedSource: Static, + }, + { + name: "Parameter Value - Float, Default Source", + key: paramThree, + expectedString: "123456789.123", + expectedInt: 0, + expectedBool: false, + expectedFloat: 123456789.123, + expectedSource: Default, + }, + { + name: "Parameter Value - Boolean, Remote Source", + key: paramFour, + expectedString: "1", + expectedInt: 1, + expectedBool: true, + expectedFloat: 1, + expectedSource: Remote, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if got := config.GetString(tc.key); got != tc.expectedString { + t.Errorf("GetString(%q): got %q, want %q", tc.key, got, tc.expectedString) + } + + if got := config.GetInt(tc.key); got != tc.expectedInt { + t.Errorf("GetInt(%q): got %d, want %d", tc.key, got, tc.expectedInt) + } + + if got := config.GetBoolean(tc.key); got != tc.expectedBool { + t.Errorf("GetBoolean(%q): got %t, want %t", tc.key, got, tc.expectedBool) + } + + if got := config.GetFloat(tc.key); got != tc.expectedFloat { + t.Errorf("GetFloat(%q): got %f, want %f", tc.key, got, tc.expectedFloat) + } + + if got := config.GetValueSource(tc.key); got != tc.expectedSource { + t.Errorf("GetValueSource(%q): got %v, want %v", tc.key, got, tc.expectedSource) + } + }) + } +} diff --git a/remoteconfig/server_template.go b/remoteconfig/server_template.go index 86363816..180c9329 100644 --- a/remoteconfig/server_template.go +++ b/remoteconfig/server_template.go @@ -28,15 +28,16 @@ import ( // serverTemplateData stores the internal representation of the server template. type serverTemplateData struct { + // A list of conditions in descending order by priority. Parameters map[string]parameter `json:"parameters,omitempty"` + // Map of parameter keys to their optional default values and optional conditional values. Conditions []namedCondition `json:"conditions,omitempty"` - Version struct { - VersionNumber string `json:"versionNumber"` - IsLegacy bool `json:"isLegacy"` - } `json:"version"` + // Version information for the current Remote Config template. + Version *version `json:"version,omitempty"` + // Current Remote Config template ETag. ETag string `json:"etag"` } @@ -65,7 +66,7 @@ func newServerTemplate(rcClient *rcClient, defaultConfig map[string]any) (*Serve // Marshal the value to JSON bytes jsonBytes, err := json.Marshal(value) if err != nil { - return nil, fmt.Errorf("failed to marshal config key '%s': %w", key, err) + return nil, fmt.Errorf("unable to stringify default value for parameter '%s': %w", key, err) } stringifiedConfig[key] = string(jsonBytes) @@ -124,26 +125,28 @@ func (s *ServerTemplate) Evaluate(context map[string]any) (*ServerConfig, error) } config := make(map[string]value) + // Initialize config with in-app default values. for key, inAppDefault := range s.stringifiedDefaultConfig { config[key] = value{source: Default, value: inAppDefault} } + usedConditions := s.cache.Load().filterUsedConditions() ce := conditionEvaluator{ - conditions: s.cache.Load().Conditions, + conditions: usedConditions, evaluationContext: context, } evaluatedConditions := ce.evaluateConditions() - // Overlays config Value objects derived by evaluating the template. + // Overlays config value objects derived by evaluating the template. for key, parameter := range s.cache.Load().Parameters { var paramValueWrapper parameterValue - var matchedConditionName string // Track the name of the condition that matched + var matchedConditionName string - for _, condition := range s.cache.Load().Conditions { - // Iterates in order over the condition list; conditions are ordered in decreasing priority. + // Iterate through used conditions in decreasing priority order. + for _, condition := range usedConditions { if value, ok := parameter.ConditionalValues[condition.Name]; ok && evaluatedConditions[condition.Name] { paramValueWrapper = value - matchedConditionName = condition.Name // Store the name when a match occurs + matchedConditionName = condition.Name break } } @@ -158,5 +161,24 @@ func (s *ServerTemplate) Evaluate(context map[string]any) (*ServerConfig, error) config[key] = value{source: Remote, value: *parameter.DefaultValue.Value} } } - return NewServerConfig(config), nil + return newServerConfig(config), nil +} + +// filterUsedConditions identifies conditions that are referenced by parameters and returns them in order of decreasing priority. +func (s *serverTemplateData) filterUsedConditions() []namedCondition { + usedConditionNames := make(map[string]struct{}) + for _, parameter := range s.Parameters { + for name := range parameter.ConditionalValues { + usedConditionNames[name] = struct{}{} + } + } + + // Filter the original conditions list, preserving order + conditionsToEvaluate := make([]namedCondition, 0, len(usedConditionNames)) + for _, condition := range s.Conditions { + if _, ok := usedConditionNames[condition.Name]; ok { + conditionsToEvaluate = append(conditionsToEvaluate, condition) + } + } + return conditionsToEvaluate } diff --git a/remoteconfig/server_template_test.go b/remoteconfig/server_template_test.go index a0942e38..d09e4e2a 100644 --- a/remoteconfig/server_template_test.go +++ b/remoteconfig/server_template_test.go @@ -19,37 +19,42 @@ import ( ) const ( - paramOne = "test_param_one" - valueOne = "test_value_one" - paramTwo = "test_param_two" - valueTwo = "{\"test\" : \"value\"}" - paramThree = "test_param_three" - valueThree = "123456789.123" - paramFour = "test_param_four" - valueFour = "1" - conditionOne = "test_condition_one" - conditionTwo = "test_condition_two" + paramOne = "test_param_one" + paramTwo = "test_param_two" + paramThree = "test_param_three" + paramFour = "test_param_four" + paramFive = "test_param_five" + + valueOne = "test_value_one" + valueTwo = "{\"test\" : \"value\"}" + valueThree = "123456789.123" + valueFour = "1" + + conditionOne = "test_condition_one" + conditionTwo = "test_condition_two" + customSignalKeyOne = "custom_signal_key_one" - testEtag = "test-etag" - testVersion = "test-version" + + testEtag = "test-etag" + testVersion = "test-version" ) // Test newServerTemplate with valid default config func TestNewServerTemplateStringifiesDefaults(t *testing.T) { defaultConfig := map[string]any{ - "key1": "value1", - "key2": 123, - "key3": true, - "key4": nil, - "key5": "{\"test_param\" : \"test_value\"}", + paramOne: "value1", + paramTwo: 123, + paramThree: true, + paramFour: nil, + paramFive: "{\"test_param\" : \"test_value\"}", } expectedStringified := map[string]string{ - "key1": "value1", - "key2": "123", - "key3": "true", - "key4": "", // nil becomes empty string - "key5": "{\"test_param\" : \"test_value\"}", + paramOne: "value1", + paramTwo: "123", + paramThree: "true", + paramFour: "", // nil becomes empty string + paramFive: "{\"test_param\" : \"test_value\"}", } rcClient := &rcClient{} @@ -102,10 +107,7 @@ func TestServerTemplateToJSONSuccess(t *testing.T) { }, }, }, - Version: struct { - VersionNumber string "json:\"versionNumber\"" - IsLegacy bool "json:\"isLegacy\"" - }{ + Version: &version{ VersionNumber: testVersion, IsLegacy: true, }, @@ -134,10 +136,7 @@ func TestServerTemplateReturnsDefaultFromRemote(t *testing.T) { }, }, }, - Version: struct { - VersionNumber string "json:\"versionNumber\"" - IsLegacy bool "json:\"isLegacy\"" - }{ + Version: &version{ VersionNumber: testVersion, }, ETag: testEtag, @@ -173,10 +172,7 @@ func TestEvaluateReturnsInAppDefault(t *testing.T) { }, }, }, - Version: struct { - VersionNumber string "json:\"versionNumber\"" - IsLegacy bool "json:\"isLegacy\"" - }{ + Version: &version{ VersionNumber: testVersion, }, ETag: testEtag, @@ -268,10 +264,7 @@ func TestEvaluate_WithACondition_ReturnsConditionalRemoteValue(t *testing.T) { }, }, }, - Version: struct { - VersionNumber string "json:\"versionNumber\"" - IsLegacy bool "json:\"isLegacy\"" - }{ + Version: &version{ VersionNumber: testVersion, }, ETag: testEtag, @@ -350,10 +343,7 @@ func TestEvaluate_WithACondition_ReturnsConditionalInAppDefaultValue(t *testing. }, }, }, - Version: struct { - VersionNumber string "json:\"versionNumber\"" - IsLegacy bool "json:\"isLegacy\"" - }{ + Version: &version{ VersionNumber: testVersion, }, ETag: testEtag, @@ -378,3 +368,99 @@ func TestEvaluate_WithACondition_ReturnsConditionalInAppDefaultValue(t *testing. t.Fatalf("ServerTemplate.Evaluate returned incorrect source: %v want %v", src, Default) } } + +func TestGetUsedConditions(t *testing.T) { + ncOne := namedCondition{Name: "ncOne"} + ncTwo := namedCondition{Name: "ncTwo"} + ncThree := namedCondition{Name: "ncThree"} + + paramVal := valueOne + + testCases := []struct { + name string + data *serverTemplateData + expectedConditions []namedCondition + }{ + { + name: "No parameters, no conditions", + data: &serverTemplateData{}, + expectedConditions: []namedCondition{}, + }, + { + name: "Parameters, but no conditions", + data: &serverTemplateData{ + Parameters: map[string]parameter{ + paramOne: {DefaultValue: parameterValue{Value: ¶mVal}}, + }, + }, + expectedConditions: []namedCondition{}, + }, + { + name: "Conditions, but no parameters", + data: &serverTemplateData{ + Conditions: []namedCondition{ncOne, ncTwo}, + }, + expectedConditions: []namedCondition{}, + }, + { + name: "Conditions, but parameters use no conditional values", + data: &serverTemplateData{ + Parameters: map[string]parameter{ + paramOne: {DefaultValue: parameterValue{Value: ¶mVal}}, + }, + Conditions: []namedCondition{ncOne, ncTwo}, + }, + expectedConditions: []namedCondition{}, + }, + { + name: "One parameter uses one condition", + data: &serverTemplateData{ + Parameters: map[string]parameter{ + paramOne: {ConditionalValues: map[string]parameterValue{"ncOne": {Value: ¶mVal}}}, + }, + Conditions: []namedCondition{ncOne, ncTwo}, + }, + expectedConditions: []namedCondition{ncOne}, + }, + { + name: "One parameter uses multiple conditions", + data: &serverTemplateData{ + Parameters: map[string]parameter{ + paramOne: {ConditionalValues: map[string]parameterValue{ + "ncOne": {Value: ¶mVal}, + "ncThree": {Value: ¶mVal}, + }}, + }, + Conditions: []namedCondition{ncOne, ncTwo, ncThree}, + }, + expectedConditions: []namedCondition{ncOne, ncThree}, + }, + { + name: "Multiple parameters use overlapping conditions", + data: &serverTemplateData{ + Parameters: map[string]parameter{ + paramOne: {ConditionalValues: map[string]parameterValue{"ncTwo": {Value: ¶mVal}}}, + paramTwo: {ConditionalValues: map[string]parameterValue{"ncOne": {Value: ¶mVal}, "ncTwo": {Value: ¶mVal}}}, + }, + Conditions: []namedCondition{ncTwo, ncThree, ncOne}, + }, + expectedConditions: []namedCondition{ncTwo, ncOne}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + used := tc.data.filterUsedConditions() + + if len(used) != len(tc.expectedConditions) { + t.Fatalf("filterUsedConditions() returned %d conditions, want %d", len(used), len(tc.expectedConditions)) + } + + for idx, ec := range tc.expectedConditions { + if used[idx].Name != ec.Name { + t.Errorf("Condition at index %d has name %q, want %q", idx, used[idx].Name, ec.Name) + } + } + }) + } +} diff --git a/remoteconfig/server_template_types.go b/remoteconfig/server_template_types.go index ca7f410d..cc19f39f 100644 --- a/remoteconfig/server_template_types.go +++ b/remoteconfig/server_template_types.go @@ -127,3 +127,46 @@ type parameterValue struct { // If true, indicates that the in-app default value is to be used for the parameter UseInAppDefault *bool `json:"useInAppDefault,omitempty"` } + +// Structure representing a Remote Config template version. +// Output only, except for the version description. Contains metadata about a particular +// version of the Remote Config template. All fields are set at the time the specified Remote Config template is published. +type version struct { + // The version number of a Remote Config template. + VersionNumber string `json:"versionNumber,omitempty"` + + // The timestamp of when this version of the Remote Config template was written to the + // Remote Config backend. + UpdateTime string `json:"updateTime,omitempty"` + + // The origin of the template update action. + UpdateOrigin string `json:"updateOrigin,omitempty"` + + // The type of the template update action. + UpdateType string `json:"updateType,omitempty"` + + // Aggregation of all metadata fields about the account that performed the update. + UpdateUser *remoteConfigUser `json:"updateUser,omitempty"` + + // The user-provided description of the corresponding Remote Config template. + Description string `json:"description,omitempty"` + + // The version number of the Remote Config template that has become the current version + // due to a rollback. Only present if this version is the result of a rollback. + RollbackSource string `json:"rollbackSource,omitempty"` + + // Indicates whether this Remote Config template was published before version history was supported. + IsLegacy bool `json:"isLegacy,omitempty"` +} + +// Represents a Remote Config user. +type remoteConfigUser struct { + // Email address. Output only. + Email string `json:"email,omitempty"` + + // Display name. Output only. + Name string `json:"name,omitempty"` + + // Image URL. Output only. + ImageURL string `json:"imageUrl,omitempty"` +}