feat(): add string type support

README.md

@@ -24,7 +24,7 @@ Each of these components are comprised of lower level libraries that you can use
 
 ### About Feature Flags
 
-Feature flags have many use cases and there are many implementations. With Decider, the three supported types of flags are `boolean`, `percentile`, and `scalar`. For our purposes at [VSCO](http://vsco.co), these have been enough to handle our needs.
+Feature flags have many use cases and there are many implementations. With Decider, the supported types of flags are `boolean`, `percentile`, `scalar`, and `string`. For our purposes at [VSCO](http://vsco.co), these have been enough to handle our needs.
 
 #### Boolean Flags
 An example use case for a `boolean` flag would be an API kill switch that could alleviate load for a backing database.
@@ -75,6 +75,21 @@ waitMS := dcdr.ScaleValue("daemon-db-insert-wait-ms", 0, 1000)
 time.Sleep(waitMS * time.Millisecond)
 ```
 
+#### String Flags
+A `string` flag holds a free-form text value. Strings require an explicit `--type=string` (`-t string`) when setting them, while `boolean` and `percentile` are inferred from the value when `--type` is omitted. A common use case is a runtime-tunable setting such as a minimum log level.
+
+```
+# --type=string is required for string values
+dcdr set -n min-log-level -v debug -t string
+
+min-log-level => "debug"
+```
+
+```Go
+// Returns the configured value, or "" if the flag is absent or not a string.
+level := dcdr.GetString("min-log-level")
+```
+
 [Read more](#using-the-go-client) on how to use the `Client`.
 
 ### Caveat
@@ -349,7 +364,7 @@ if err != nil {
 
 ### Checking feature flags
 
-The client has three main methods for interacting with flags `IsAvailable(feature string)`. `IsAvailableForID(feature string, id uint64)`, and `ScaleValue(feature string, min float64, max float64)`.
+The client has four main methods for interacting with flags `IsAvailable(feature string)`. `IsAvailableForID(feature string, id uint64)`, `ScaleValue(feature string, min float64, max float64)`, and `GetString(feature string)`.
 
 #### IsAvailable
 
@@ -472,6 +487,24 @@ for {
 }
 ```
 
+### GetString
+
+`GetString` returns the value of a `string` feature. It returns `""` when the feature is absent or is not a string-typed flag, so callers should fall back to a sane default for unknown values.
+
+```
+# set a string feature (--type=string is required)
+dcdr set -n min-log-level -v debug -t string
+```
+
+```Go
+// min-log-level would be "debug"
+level := dcdr.GetString("min-log-level")
+
+if level == "" {
+	level = "info" // fall back to a default
+}
+```
+
 ## Building a custom Server
 
 Exposing your feature flags to the open internet would be a terrible idea in most cases. The default server will work fine as long as access is restricted to internal network clients but what if we want to allow access to mobile devices? Since there are entirely too many auth strategies to cover and we are kind of lazy, Decider `Server` allows you to add middleware to customize its behavior to suit your authentication needs.

cli/api/client_test.go

@@ -127,6 +127,26 @@ func TestDeleteWithError(t *testing.T) {
 
 	assert.Equal(t, e, err)
 }
+func TestKVsToFeatureMapStringValue(t *testing.T) {
+	kvb := stores.KVBytes{
+		&stores.KVByte{
+			Key:   "dcdr/features/default/min-log-level",
+			Bytes: []byte(`{ "feature_type": "string", "key": "min-log-level", "value": "debug" }`),
+		},
+	}
+
+	cs := stores.NewMockStore(nil, nil)
+	cfg := config.DefaultConfig()
+	c := New(cs, &stores.MockRepo{}, cfg, nil)
+
+	fm, err := c.KVsToFeatureMap(kvb)
+	assert.Nil(t, err)
+	assert.NotNil(t, fm)
+
+	// the served value map flattens to just the value, preserving the string
+	assert.Equal(t, "debug", fm.Dcdr.Defaults()["min-log-level"])
+}
+
 func TestKVsToFeatureMapInfoExistByNameSpace(t *testing.T) {
 
 	kvb := stores.KVBytes{

cli/cli.go

@@ -78,7 +78,7 @@ func (c *CLI) Commands() []climax.Command {
 		{
 			Name:  "set",
 			Brief: "create or update a feature flag",
-			Usage: `set -name flag_name -value [0.0-1.0|true/false] -comment "flag description"`,
+			Usage: `set -name flag_name -value <value> [-type boolean|percentile|string] -comment "flag description"`,
 			Help: `
 
 
@@ -121,6 +121,13 @@ func (c *CLI) Commands() []climax.Command {
 					Help:     `the value of the flag`,
 					Variable: true,
 				},
+				{
+					Name:     "type",
+					Short:    "t",
+					Usage:    `--type=boolean|percentile|string`,
+					Help:     `flag type; required for string values, inferred for boolean/percentile when omitted`,
+					Variable: true,
+				},
 				{
 					Name:     "comment",
 					Short:    "c",
@@ -146,6 +153,10 @@ func (c *CLI) Commands() []climax.Command {
 					Usecase:     `-n "flag_name" -v false -c "the flag desc"`,
 					Description: `sets a boolean flag to false`,
 				},
+				{
+					Usecase:     `-n "flag_name" -v debug -t string -c "the flag desc"`,
+					Description: `sets a string flag to "debug"`,
+				},
 			},
 
 			Handle: c.Ctrl.Set,

cli/controller/controller.go

@@ -23,9 +23,13 @@ import (
 const filePerms = 0775
 
 var (
-	errInvalidFeatureType = errors.New("invalid -value format. use -value=[0.0-1.0] or [true|false]")
-	errInvalidRange       = errors.New("invalid -value for percentile. use -value=[0.0-1.0]")
-	errNameRequired       = errors.New("-name is required")
+	errInvalidType           = errors.New("invalid -type. use boolean, percentile, or string")
+	errTypeRequiredForString = errors.New("-type=string is required for non-numeric, non-boolean values")
+	errInvalidBool           = errors.New("invalid -value for boolean. use -value=[true|false]")
+	errInvalidPercentile     = errors.New("invalid -value for percentile. must be a number")
+	errEmptyString           = errors.New("invalid -value for string. must not be empty or whitespace-only")
+	errInvalidRange          = errors.New("invalid -value for percentile. use -value=[0.0-1.0]")
+	errNameRequired          = errors.New("-name is required")
 )
 
 // Controller handler for CLI commands
@@ -277,6 +281,7 @@ func (cc *Controller) Watch(ctx climax.Context) int {
 func (cc *Controller) ParseContext(ctx climax.Context) (*models.Feature, error) {
 	name, _ := ctx.Get("name")
 	val, _ := ctx.Get("value")
+	typ, _ := ctx.Get("type")
 	cmt, _ := ctx.Get("comment")
 	scp, _ := ctx.Get("scope")
 
@@ -288,16 +293,11 @@ func (cc *Controller) ParseContext(ctx climax.Context) (*models.Feature, error)
 	var ft models.FeatureType
 
 	if val != "" {
-		v, ft = models.ParseValueAndFeatureType(val)
-
-		if ft == models.Invalid {
-			return nil, errInvalidFeatureType
-		}
+		var err error
+		v, ft, err = parseValue(val, typ)
 
-		if ft == models.Percentile {
-			if v.(float64) > 1.0 || v.(float64) < 0 {
-				return nil, errInvalidRange
-			}
+		if err != nil {
+			return nil, err
 		}
 	}
 
@@ -306,3 +306,61 @@ func (cc *Controller) ParseContext(ctx climax.Context) (*models.Feature, error)
 
 	return f, nil
 }
+
+// parseValue resolves the value and feature type for a `set` command.
+//   - When -type is provided, the value is parsed strictly for that type.
+//   - When -type is omitted, the type is inferred: boolean and percentile are
+//     accepted, but an inferred string is rejected (see below).
+func parseValue(val string, typ string) (interface{}, models.FeatureType, error) {
+	var v interface{}
+	var ft models.FeatureType
+
+	if typ != "" {
+		var ok bool
+		ft, ok = models.ParseFeatureType(typ)
+
+		if !ok {
+			return nil, models.Invalid, errInvalidType
+		}
+
+		var err error
+		if v, err = models.ParseValueForType(val, ft); err != nil {
+			switch ft {
+			case models.Boolean:
+				return nil, models.Invalid, errInvalidBool
+			case models.Percentile:
+				return nil, models.Invalid, errInvalidPercentile
+			case models.String:
+				return nil, models.Invalid, errEmptyString
+			default:
+				return nil, models.Invalid, err
+			}
+		}
+	} else {
+		v, ft = models.ParseValueAndFeatureType(val)
+
+		// An inferred string is ambiguous (e.g. a typo'd bool/number), so
+		// require the caller to opt in explicitly with -type=string.
+		if ft == models.String {
+			return nil, models.Invalid, errTypeRequiredForString
+		}
+	}
+
+	// shared validation for both the explicit and inferred paths
+	if ft == models.Percentile {
+		if err := validatePercentile(v); err != nil {
+			return nil, models.Invalid, err
+		}
+	}
+
+	return v, ft, nil
+}
+
+// validatePercentile ensures a percentile value falls within the 0.0-1.0 range.
+func validatePercentile(v interface{}) error {
+	if f := v.(float64); f > 1.0 || f < 0 {
+		return errInvalidRange
+	}
+
+	return nil
+}

cli/controller/controller_test.go

@@ -125,3 +125,105 @@ func TestSet(t *testing.T) {
 
 	assert.Equal(t, Success, code)
 }
+
+func newParseController() *Controller {
+	return New(config.DefaultConfig(), NewMockClient(nil, nil, nil))
+}
+
+func parseCtx(vars map[string]string) climax.Context {
+	return climax.Context{Variable: vars}
+}
+
+func TestParseContextExplicitTypes(t *testing.T) {
+	ctl := newParseController()
+
+	f, err := ctl.ParseContext(parseCtx(map[string]string{
+		"name": "min-log-level", "value": "debug", "type": "string",
+	}))
+	assert.NoError(t, err)
+	assert.Equal(t, models.String, f.FeatureType)
+	assert.Equal(t, "debug", f.Value)
+
+	// string type stores bool/number-looking values verbatim
+	f, err = ctl.ParseContext(parseCtx(map[string]string{
+		"name": "literal", "value": "true", "type": "string",
+	}))
+	assert.NoError(t, err)
+	assert.Equal(t, models.String, f.FeatureType)
+	assert.Equal(t, "true", f.Value)
+
+	// surrounding whitespace is trimmed from string values
+	f, err = ctl.ParseContext(parseCtx(map[string]string{
+		"name": "padded", "value": " debug ", "type": "string",
+	}))
+	assert.NoError(t, err)
+	assert.Equal(t, "debug", f.Value)
+
+	f, err = ctl.ParseContext(parseCtx(map[string]string{
+		"name": "flag", "value": "true", "type": "boolean",
+	}))
+	assert.NoError(t, err)
+	assert.Equal(t, models.Boolean, f.FeatureType)
+	assert.Equal(t, true, f.Value)
+
+	f, err = ctl.ParseContext(parseCtx(map[string]string{
+		"name": "flag", "value": "0.5", "type": "percentile",
+	}))
+	assert.NoError(t, err)
+	assert.Equal(t, models.Percentile, f.FeatureType)
+	assert.Equal(t, 0.5, f.Value)
+}
+
+func TestParseContextExplicitTypeErrors(t *testing.T) {
+	ctl := newParseController()
+
+	_, err := ctl.ParseContext(parseCtx(map[string]string{
+		"name": "flag", "value": "debug", "type": "nope",
+	}))
+	assert.Equal(t, errInvalidType, err)
+
+	_, err = ctl.ParseContext(parseCtx(map[string]string{
+		"name": "flag", "value": "notabool", "type": "boolean",
+	}))
+	assert.Equal(t, errInvalidBool, err)
+
+	_, err = ctl.ParseContext(parseCtx(map[string]string{
+		"name": "flag", "value": "notanumber", "type": "percentile",
+	}))
+	assert.Equal(t, errInvalidPercentile, err)
+
+	_, err = ctl.ParseContext(parseCtx(map[string]string{
+		"name": "flag", "value": "2.0", "type": "percentile",
+	}))
+	assert.Equal(t, errInvalidRange, err)
+
+	_, err = ctl.ParseContext(parseCtx(map[string]string{
+		"name": "flag", "value": "   ", "type": "string",
+	}))
+	assert.Equal(t, errEmptyString, err)
+}
+
+func TestParseContextInference(t *testing.T) {
+	ctl := newParseController()
+
+	// bool/percentile still work without -type
+	f, err := ctl.ParseContext(parseCtx(map[string]string{"name": "flag", "value": "false"}))
+	assert.NoError(t, err)
+	assert.Equal(t, models.Boolean, f.FeatureType)
+
+	f, err = ctl.ParseContext(parseCtx(map[string]string{"name": "flag", "value": "0.25"}))
+	assert.NoError(t, err)
+	assert.Equal(t, models.Percentile, f.FeatureType)
+
+	// a string value without -type is rejected
+	_, err = ctl.ParseContext(parseCtx(map[string]string{"name": "flag", "value": "debug"}))
+	assert.Equal(t, errTypeRequiredForString, err)
+
+	// a numeric typo infers to string and is likewise rejected
+	_, err = ctl.ParseContext(parseCtx(map[string]string{"name": "flag", "value": "0.5x"}))
+	assert.Equal(t, errTypeRequiredForString, err)
+
+	// out-of-range percentile without -type
+	_, err = ctl.ParseContext(parseCtx(map[string]string{"name": "flag", "value": "2.0"}))
+	assert.Equal(t, errInvalidRange, err)
+}

client/client.go

@@ -16,6 +16,7 @@ import (
 type IFace interface {
 	IsAvailable(feature string) bool
 	IsAvailableForID(feature string, id uint64) bool
+	GetString(feature string) string
 	ScaleValue(feature string, min float64, max float64) float64
 	UpdateFeatures(bts []byte)
 	FeatureExists(feature string) bool
@@ -168,6 +169,16 @@ func (c *Client) IsAvailable(feature string) bool {
 	}
 }
 
+// GetString returns the string value of `feature`, or "" if it is absent or
+// not a string-typed feature.
+func (c *Client) GetString(feature string) string {
+	if val, ok := c.Features()[feature].(string); ok {
+		return val
+	}
+
+	return ""
+}
+
 // IsAvailableForID used to check features with float values between 0.0-1.0.
 // Returns false if a non-percentile type `feature` is passed.
 func (c *Client) IsAvailableForID(feature string, id uint64) bool {

client/client_test.go

@@ -49,7 +49,8 @@ var JSONBytes = []byte(`{
         "float": 0,
         "bool_false": false,
         "bool": true,
-        "default_float": 0.5
+        "default_float": 0.5,
+        "str": "debug"
       }
     },
     "info": {
@@ -151,6 +152,24 @@ func TestIsAvailableForID(t *testing.T) {
 	assert.True(t, c.IsAvailableForID("default_float", 5))
 }
 
+func TestGetString(t *testing.T) {
+	m := MockFeatureMap()
+	c := NewTestClient().SetFeatureMap(m)
+
+	assert.Equal(t, "debug", c.GetString("str"))
+	assert.Equal(t, "", c.GetString("nope"))
+	assert.Equal(t, "", c.GetString("bool"))
+}
+
+// TestGetStringRoundTrip proves a string value survives the full client API
+// path: served JSON payload -> UpdateFeatures -> GetString.
+func TestGetStringRoundTrip(t *testing.T) {
+	c := NewTestClient()
+	c.UpdateFeatures(JSONBytes)
+
+	assert.Equal(t, "debug", c.GetString("str"))
+}
+
 func TestScaleValue(t *testing.T) {
 	m := MockFeatureMap()
 	c := NewTestClient().SetFeatureMap(m)