Merge pull request #286 from compose-spec/refactor-interpolation-symbol-choice

Refactor interpolation symbols choice algorithm.

Author: ulyssessouza

template/template.go

@@ -19,6 +19,7 @@ package template
 import (
 	"fmt"
 	"regexp"
+	"sort"
 	"strings"
 
 	"github.com/sirupsen/logrus"
@@ -60,10 +61,12 @@ type SubstituteFunc func(string, Mapping) (string, bool, error)
 // SubstituteWith substitute variables in the string with their values.
 // It accepts additional substitute function.
 func SubstituteWith(template string, mapping Mapping, pattern *regexp.Regexp, subsFuncs ...SubstituteFunc) (string, error) {
+	var err error
+
 	if len(subsFuncs) == 0 {
-		subsFuncs = getDefaultSortedSubstitutionFunctions(template)
+		_, subsFunc := getSubstitutionFunctionForTemplate(template)
+		subsFuncs = []SubstituteFunc{subsFunc}
 	}
-	var err error
 	result := pattern.ReplaceAllStringFunc(template, func(substring string) string {
 		closingBraceIndex := getFirstBraceClosingIndex(substring)
 		rest := ""
@@ -121,23 +124,31 @@ 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 getSubstitutionFunctionForTemplate(template string) (string, SubstituteFunc) {
+	interpolationMapping := []struct {
+		string
+		SubstituteFunc
+	}{
+		{":?", requiredErrorWhenEmptyOrUnset},
+		{"?", requiredErrorWhenUnset},
+		{":-", defaultWhenEmptyOrUnset},
+		{"-", defaultWhenUnset},
+		{":+", defaultWhenNotEmpty},
+		{"+", defaultWhenSet},
 	}
+	sort.Slice(interpolationMapping, func(i, j int) bool {
+		idxI := strings.Index(template, interpolationMapping[i].string)
+		idxJ := strings.Index(template, interpolationMapping[j].string)
+		if idxI < 0 {
+			return false
+		}
+		if idxJ < 0 {
+			return true
+		}
+		return idxI < idxJ
+	})
+
+	return interpolationMapping[0].string, interpolationMapping[0].SubstituteFunc
 }
 
 func getFirstBraceClosingIndex(s string) int {
@@ -252,26 +263,36 @@ func extractVariable(value interface{}, pattern *regexp.Regexp) ([]Variable, boo
 }
 
 // Soft default (fall back if unset or empty)
-func softDefault(substitution string, mapping Mapping) (string, bool, error) {
-	sep := ":-"
-	if !strings.Contains(substitution, sep) {
-		return "", false, nil
-	}
-	name, defaultValue := partition(substitution, sep)
-	defaultValue, err := Substitute(defaultValue, mapping)
-	if err != nil {
-		return "", false, err
-	}
-	value, ok := mapping(name)
-	if !ok || value == "" {
-		return defaultValue, true, nil
-	}
-	return value, true, nil
+func defaultWhenEmptyOrUnset(substitution string, mapping Mapping) (string, bool, error) {
+	return withDefaultWhenAbsence(substitution, mapping, true)
 }
 
 // Hard default (fall back if-and-only-if empty)
-func hardDefault(substitution string, mapping Mapping) (string, bool, error) {
+func defaultWhenUnset(substitution string, mapping Mapping) (string, bool, error) {
+	return withDefaultWhenAbsence(substitution, mapping, false)
+}
+
+func defaultWhenNotEmpty(substitution string, mapping Mapping) (string, bool, error) {
+	return "", false, nil // TODO Implement ":+"
+}
+
+func defaultWhenSet(substitution string, mapping Mapping) (string, bool, error) {
+	return "", false, nil // TODO Implement "+"
+}
+
+func requiredErrorWhenEmptyOrUnset(substitution string, mapping Mapping) (string, bool, error) {
+	return withRequired(substitution, mapping, ":?", func(v string) bool { return v != "" })
+}
+
+func requiredErrorWhenUnset(substitution string, mapping Mapping) (string, bool, error) {
+	return withRequired(substitution, mapping, "?", func(_ string) bool { return true })
+}
+
+func withDefaultWhenAbsence(substitution string, mapping Mapping, emptyOrUnset bool) (string, bool, error) {
 	sep := "-"
+	if emptyOrUnset {
+		sep = ":-"
+	}
 	if !strings.Contains(substitution, sep) {
 		return "", false, nil
 	}
@@ -281,20 +302,12 @@ func hardDefault(substitution string, mapping Mapping) (string, bool, error) {
 		return "", false, err
 	}
 	value, ok := mapping(name)
-	if !ok {
+	if !ok || (emptyOrUnset && value == "") {
 		return defaultValue, true, nil
 	}
 	return value, true, nil
 }
 
-func requiredNonEmpty(substitution string, mapping Mapping) (string, bool, error) {
-	return withRequired(substitution, mapping, ":?", func(v string) bool { return v != "" })
-}
-
-func required(substitution string, mapping Mapping) (string, bool, error) {
-	return withRequired(substitution, mapping, "?", func(_ string) bool { return true })
-}
-
 func withRequired(substitution string, mapping Mapping, sep string, valid func(string) bool) (string, bool, error) {
 	if !strings.Contains(substitution, sep) {
 		return "", false, nil

template/template_test.go

@@ -407,3 +407,35 @@ func TestExtractVariables(t *testing.T) {
 		})
 	}
 }
+
+func TestSubstitutionFunctionChoice(t *testing.T) {
+	testcases := []struct {
+		name   string
+		input  string
+		symbol string
+	}{
+		{"Error when EMPTY or UNSET", "VARNAME:?val?ue", ":?"},
+		{"Error when UNSET 1", "VARNAME?val:?ue", "?"},
+		{"Error when UNSET 2", "VARNAME?va-lu+e:?e", "?"},
+		{"Error when UNSET 3", "VARNAME?va+lu-e:?e", "?"},
+
+		{"Default when EMPTY or UNSET", "VARNAME:-value", ":-"},
+		{"Default when UNSET 1", "VARNAME-va:-lu:?e", "-"},
+		{"Default when UNSET 2", "VARNAME-va+lu?e", "-"},
+		{"Default when UNSET 3", "VARNAME-va?lu+e", "-"},
+
+		{"Default when NOT EMPTY", "VARNAME:+va:?lu:-e", ":+"},
+		{"Default when SET 1", "VARNAME+va:+lue", "+"},
+		{"Default when SET 2", "VARNAME+va?lu-e", "+"},
+		{"Default when SET 3", "VARNAME+va-lu?e", "+"},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			symbol, _ := getSubstitutionFunctionForTemplate(tc.input)
+			assert.Equal(t, symbol, tc.symbol,
+				fmt.Sprintf("Wrong on output for: %s got symbol -> %#v", tc.input, symbol),
+			)
+		})
+	}
+}