Fix precedence and add tests for required variables

Signed-off-by: Ulysses Souza <[email protected]>

Author: ulyssessouza

interpolation/interpolation_test.go

@@ -17,6 +17,7 @@
 package interpolation
 
 import (
+	"fmt"
 	"strconv"
 	"testing"
 
@@ -98,16 +99,29 @@ func TestValidUnexistentInterpolation(t *testing.T) {
 	var testcases = []struct {
 		test     string
 		expected string
+		errMsg   string
 	}{
 		{test: "{{{ ${FOO:-foo_} }}}", expected: "{{{ foo_ }}}"},
 		{test: "{{{ ${FOO:-foo-bar-value} }}}", expected: "{{{ foo-bar-value }}}"},
+		{test: "{{{ ${FOO:-foo?bar?value} }}}", expected: "{{{ foo?bar?value }}}"},
+		{test: "{{{ ${FOO:-foo:?bar:?value} }}}", expected: "{{{ foo:?bar:?value }}}"},
 		{test: "{{{ ${FOO:-foo} ${BAR:-DEFAULT_VALUE} }}}", expected: "{{{ foo DEFAULT_VALUE }}}"},
 		{test: "{{{ ${BAR} }}}", expected: "{{{  }}}"},
 		{test: "${FOO:-baz} }}}", expected: "baz }}}"},
 		{test: "${FOO-baz} }}}", expected: "baz }}}"},
+
+		{test: "{{{ ${FOO:?foo_} }}}", errMsg: "foo_"},
+		{test: "{{{ ${FOO:?foo-bar-value} }}}", errMsg: "foo-bar-value"},
+		{test: "{{{ ${FOO:?foo} ${BAR:-DEFAULT_VALUE} }}}", errMsg: "foo"},
+		{test: "{{{ ${BAR} }}}", expected: "{{{  }}}"},
+		{test: "${FOO:?baz} }}}", errMsg: "baz"},
+		{test: "${FOO?baz} }}}", errMsg: "baz"},
 	}
 
 	getServiceConfig := func(val string) map[string]interface{} {
+		if val == "" {
+			return map[string]interface{}{}
+		}
 		return map[string]interface{}{
 			"myservice": map[string]interface{}{
 				"environment": map[string]interface{}{
@@ -117,9 +131,17 @@ func TestValidUnexistentInterpolation(t *testing.T) {
 		}
 	}
 
+	getFullErrorMsg := func(msg string) string {
+		return fmt.Sprintf("invalid interpolation format for myservice.environment.TESTVAR: "+
+			"\"required variable FOO is missing a value: %s\". You may need to escape any $ with another $.", msg)
+	}
+
 	for _, testcase := range testcases {
 		result, err := Interpolate(getServiceConfig(testcase.test), Options{})
-		assert.NilError(t, err)
+		if testcase.errMsg != "" {
+			assert.Assert(t, err != nil, fmt.Sprintf("This should result in an error %q", testcase.errMsg))
+			assert.Equal(t, getFullErrorMsg(testcase.errMsg), err.Error())
+		}
 		assert.Check(t, is.DeepEqual(getServiceConfig(testcase.expected), result))
 	}
 }

template/template.go

@@ -61,12 +61,7 @@ type SubstituteFunc func(string, Mapping) (string, bool, error)
 // It accepts additional substitute function.
 func SubstituteWith(template string, mapping Mapping, pattern *regexp.Regexp, subsFuncs ...SubstituteFunc) (string, error) {
 	if len(subsFuncs) == 0 {
-		subsFuncs = []SubstituteFunc{
-			softDefault,
-			hardDefault,
-			requiredNonEmpty,
-			required,
-		}
+		subsFuncs = getDefaultSortedSubstitutionFunctions(template)
 	}
 	var err error
 	result := pattern.ReplaceAllStringFunc(template, func(substring string) string {
@@ -126,6 +121,25 @@ func SubstituteWith(template string, mapping Mapping, pattern *regexp.Regexp, su
 	return result, err
 }
 
+func getDefaultSortedSubstitutionFunctions(template string, fns ...SubstituteFunc) []SubstituteFunc {
+	hyphenIndex := strings.IndexByte(template, '-')
+	questionIndex := strings.IndexByte(template, '?')
+	if hyphenIndex < 0 || hyphenIndex > questionIndex {
+		return []SubstituteFunc{
+			requiredNonEmpty,
+			required,
+			softDefault,
+			hardDefault,
+		}
+	}
+	return []SubstituteFunc{
+		softDefault,
+		hardDefault,
+		requiredNonEmpty,
+		required,
+	}
+}
+
 func getFirstBraceClosingIndex(s string) int {
 	openVariableBraces := 0
 	for i := 0; i < len(s); i++ {

template/template_test.go

@@ -262,6 +262,48 @@ func TestSubstituteWithCustomFunc(t *testing.T) {
 	assert.Check(t, is.ErrorContains(err, "required variable"))
 }
 
+// TestPrecedence tests is the precedence on '-' and '?' is of the first match
+func TestPrecedence(t *testing.T) {
+
+	testCases := []struct {
+		template string
+		expected string
+		err      error
+	}{
+		{
+			template: "${UNSET_VAR?bar-baz}", // Unexistent variable
+			expected: "",
+			err: &InvalidTemplateError{
+				Template: "required variable UNSET_VAR is missing a value: bar-baz",
+			},
+		},
+		{
+			template: "${UNSET_VAR-myerror?msg}", // Unexistent variable
+			expected: "myerror?msg",
+			err:      nil,
+		},
+
+		{
+			template: "${FOO?bar-baz}", // Existent variable
+			expected: "first",
+		},
+		{
+			template: "${BAR:-default_value_for_empty_var}", // Existent empty variable
+			expected: "default_value_for_empty_var",
+		},
+		{
+			template: "${UNSET_VAR-default_value_for_unset_var}", // Unset variable
+			expected: "default_value_for_unset_var",
+		},
+	}
+
+	for _, tc := range testCases {
+		result, err := Substitute(tc.template, defaultMapping)
+		assert.Check(t, is.DeepEqual(tc.err, err))
+		assert.Check(t, is.Equal(tc.expected, result))
+	}
+}
+
 func TestExtractVariables(t *testing.T) {
 	testCases := []struct {
 		name     string