Merge pull request #2 from go-andiamo/vars

Path vars

path_part.go

@@ -44,9 +44,9 @@ func (pt *pathPart) setName(name string, pos int) error {
 
 func (pt *pathPart) addFound(vars PathVars, val string) {
 	if pt.name != "" {
-		vars.AddNamedValue(pt.name, val)
+		_ = vars.AddNamedValue(pt.name, val)
 	} else {
-		vars.AddPositionalValue(val)
+		_ = vars.AddPositionalValue(val)
 	}
 }
 
@@ -150,22 +150,21 @@ func (pt *pathPart) pathFrom(tracker *positionsTracker) (string, error) {
 
 type positionsTracker struct {
 	vars           PathVars
-	positional     bool
 	position       int
 	namedPositions map[string]int
 }
 
-func (t *positionsTracker) getVar(pt *pathPart) (string, error) {
-	if t.positional {
-		if str, ok := t.vars.GetPositional(t.position); ok {
-			t.position++
+func (tr *positionsTracker) getVar(pt *pathPart) (string, error) {
+	if tr.vars.VarsType() == Positions {
+		if str, ok := tr.vars.GetPositional(tr.position); ok {
+			tr.position++
 			return str, nil
 		}
-		return "", fmt.Errorf("no var for position %d", t.position+1)
+		return "", fmt.Errorf("no var for position %d", tr.position+1)
 	} else {
-		np := t.namedPositions[pt.name]
-		if str, ok := t.vars.GetNamed(pt.name, np); ok {
-			t.namedPositions[pt.name] = np + 1
+		np := tr.namedPositions[pt.name]
+		if str, ok := tr.vars.GetNamed(pt.name, np); ok {
+			tr.namedPositions[pt.name] = np + 1
 			return str, nil
 		} else if np == 0 {
 			return "", fmt.Errorf("no var for '%s'", pt.name)

path_part_test.go

@@ -83,7 +83,7 @@ func TestPathPart_OverallRegexp(t *testing.T) {
 	ss = rx.FindStringSubmatch(`--a+z++`)
 	require.NotEmpty(t, ss)
 
-	vars := newPathVars(Positions)
+	vars := newPathVars(Names)
 	ok := pt.multiMatch(`--a+z++12345`, 0, vars, nil)
 	require.True(t, ok)
 	require.Equal(t, 2, vars.Len())
@@ -105,7 +105,7 @@ func TestPathPart_OverallRegexp(t *testing.T) {
 }
 
 func TestPathPart_OverallRegexpMatch(t *testing.T) {
-	vars := newPathVars(Positions)
+	vars := newPathVars(Names)
 	pt := pathPart{
 		subParts: []pathPart{
 			{

path_vars.go

@@ -1,5 +1,7 @@
 package urit
 
+import "errors"
+
 type PathVar struct {
 	Name          string
 	NamedPosition int
@@ -21,8 +23,8 @@ type PathVars interface {
 	Clear()
 	// VarsType returns the path vars type (Positions or Names)
 	VarsType() PathVarsType
-	AddNamedValue(name string, val string)
-	AddPositionalValue(val string)
+	AddNamedValue(name string, val string) error
+	AddPositionalValue(val string) error
 }
 
 type pathVars struct {
@@ -112,7 +114,10 @@ func (pvs *pathVars) VarsType() PathVarsType {
 	return pvs.varsType
 }
 
-func (pvs *pathVars) AddNamedValue(name string, val string) {
+func (pvs *pathVars) AddNamedValue(name string, val string) error {
+	if pvs.varsType != Names {
+		return errors.New("cannot add named var to non-names vars")
+	}
 	np := len(pvs.named[name])
 	v := PathVar{
 		Name:          name,
@@ -122,20 +127,25 @@ func (pvs *pathVars) AddNamedValue(name string, val string) {
 	}
 	pvs.named[name] = append(pvs.named[name], v)
 	pvs.all = append(pvs.all, v)
+	return nil
 }
 
-func (pvs *pathVars) AddPositionalValue(val string) {
+func (pvs *pathVars) AddPositionalValue(val string) error {
+	if pvs.varsType != Positions {
+		return errors.New("cannot add positional var to non-positionals vars")
+	}
 	pvs.all = append(pvs.all, PathVar{
 		Position: len(pvs.all),
 		Value:    val,
 	})
+	return nil
 }
 
 // Positional creates a positional PathVars from the values supplied
 func Positional(values ...string) PathVars {
 	result := newPathVars(Positions)
 	for _, val := range values {
-		result.AddPositionalValue(val)
+		_ = result.AddPositionalValue(val)
 	}
 	return result
 }
@@ -150,7 +160,7 @@ func Named(namesAndValues ...string) PathVars {
 	}
 	result := newPathVars(Names)
 	for i := 0; i < len(namesAndValues); i += 2 {
-		result.AddNamedValue(namesAndValues[i], namesAndValues[i+1])
+		_ = result.AddNamedValue(namesAndValues[i], namesAndValues[i+1])
 	}
 	return result
 }

path_vars_test.go

@@ -53,6 +53,12 @@ func TestPositional(t *testing.T) {
 	require.Equal(t, "b", all[1].Value)
 	require.Equal(t, 1, all[1].Position)
 	require.Equal(t, "", all[1].Name)
+
+	err := args.AddPositionalValue("c")
+	require.NoError(t, err)
+	require.Equal(t, 3, args.Len())
+	err = args.AddNamedValue("foo", "bar")
+	require.Error(t, err)
 }
 
 func TestNamed(t *testing.T) {
@@ -75,6 +81,12 @@ func TestNamed(t *testing.T) {
 	require.Equal(t, 2, all[2].Position)
 	require.Equal(t, "c", all[2].Value)
 	require.Equal(t, 1, all[2].NamedPosition)
+
+	err := args.AddNamedValue("qux", "d")
+	require.NoError(t, err)
+	require.Equal(t, 4, args.Len())
+	err = args.AddPositionalValue("whoops")
+	require.Error(t, err)
 }
 
 func TestNamedPanics(t *testing.T) {

template.go

@@ -25,9 +25,7 @@ func NewTemplate(path string, options ...interface{}) (Template, error) {
 	return (&template{
 		originalTemplate: slashPrefix(path),
 		pathParts:        make([]pathPart, 0),
-		positionalVars:   make([]pathPart, 0),
-		namedVars:        map[string][]pathPart{},
-		posOnlyCount:     0,
+		posVarsCount:     0,
 		fixedMatchOpts:   fs,
 		varMatchOpts:     vs,
 		pathSplitOpts:    so,
@@ -66,9 +64,8 @@ type Template interface {
 type template struct {
 	originalTemplate string
 	pathParts        []pathPart
-	positionalVars   []pathPart
-	namedVars        map[string][]pathPart
-	posOnlyCount     int
+	posVarsCount     int
+	nameVarsCount    int
 	varsType         PathVarsType
 	fixedMatchOpts   fixedMatchOptions
 	varMatchOpts     varMatchOptions
@@ -80,7 +77,6 @@ func (t *template) PathFrom(vars PathVars) (string, error) {
 	var pb strings.Builder
 	tracker := &positionsTracker{
 		vars:           vars,
-		positional:     t.posOnlyCount > 0,
 		position:       0,
 		namedPositions: map[string]int{},
 	}
@@ -126,7 +122,7 @@ func (t *template) Sub(path string, options ...interface{}) (Template, error) {
 		return nil, err
 	}
 	ra, _ := add.(*template)
-	if (ra.posOnlyCount > 0 && len(t.namedVars) > 0) || (t.posOnlyCount > 0 && len(ra.namedVars) > 0) {
+	if (ra.posVarsCount > 0 && t.nameVarsCount > 0) || (t.posVarsCount > 0 && ra.nameVarsCount > 0) {
 		return nil, newTemplateParseError("template cannot contain both positional and named path variables", 0, nil)
 	}
 	result := t.clone()
@@ -138,25 +134,22 @@ func (t *template) Sub(path string, options ...interface{}) (Template, error) {
 	for _, pt := range ra.pathParts {
 		result.pathParts = append(result.pathParts, pt)
 	}
-	for _, argPt := range ra.positionalVars {
-		result.addVar(argPt)
-	}
+	result.posVarsCount += ra.posVarsCount
+	result.nameVarsCount += ra.nameVarsCount
 	return result, nil
 }
 
 // ResolveTo generates a new template, filling in any known path vars from the supplied vars
 func (t *template) ResolveTo(vars PathVars) (Template, error) {
 	tracker := &positionsTracker{
 		vars:           vars,
-		positional:     t.posOnlyCount > 0,
 		position:       0,
 		namedPositions: map[string]int{},
 	}
 	result := &template{
-		pathParts:      make([]pathPart, 0, len(t.pathParts)),
-		positionalVars: make([]pathPart, 0, len(t.positionalVars)),
-		namedVars:      map[string][]pathPart{},
-		posOnlyCount:   0,
+		pathParts:     make([]pathPart, 0, len(t.pathParts)),
+		posVarsCount:  0,
+		nameVarsCount: 0,
 	}
 	var orgBuilder strings.Builder
 	for _, pt := range t.pathParts {
@@ -173,12 +166,11 @@ func (t *template) ResolveTo(vars PathVars) (Template, error) {
 			} else {
 				result.pathParts = append(result.pathParts, pt)
 				if pt.name == "" {
-					result.posOnlyCount++
-					result.positionalVars = append(result.positionalVars, pt)
+					result.posVarsCount++
 					orgBuilder.WriteString(`/?`)
 				} else {
 					orgBuilder.WriteString(`/{` + pt.name)
-					result.namedVars[pt.name] = append(result.namedVars[pt.name], pt)
+					result.nameVarsCount++
 					if pt.orgRegexp != "" {
 						orgBuilder.WriteString(`:` + pt.orgRegexp)
 					}
@@ -203,7 +195,7 @@ func (t *template) ResolveTo(vars PathVars) (Template, error) {
 					})
 				} else {
 					np.subParts = append(np.subParts, sp)
-					result.namedVars[sp.name] = append(result.namedVars[sp.name], sp)
+					result.nameVarsCount++
 				}
 			}
 			orgBuilder.WriteString(`/`)
@@ -239,7 +231,7 @@ func (t *template) ResolveTo(vars PathVars) (Template, error) {
 
 // VarsType returns the path vars type (Positions or Names)
 func (t *template) VarsType() PathVarsType {
-	if t.posOnlyCount != 0 {
+	if t.posVarsCount != 0 {
 		return Positions
 	}
 	return Names
@@ -325,14 +317,11 @@ func (t *template) clone() *template {
 	result := &template{
 		originalTemplate: t.originalTemplate,
 		pathParts:        make([]pathPart, 0, len(t.pathParts)),
-		positionalVars:   make([]pathPart, 0, len(t.positionalVars)),
-		namedVars:        map[string][]pathPart{},
+		posVarsCount:     t.posVarsCount,
+		nameVarsCount:    t.nameVarsCount,
+		varsType:         t.varsType,
 	}
 	result.pathParts = append(result.pathParts, t.pathParts...)
-	result.positionalVars = append(result.positionalVars, t.positionalVars...)
-	for k, v := range t.namedVars {
-		result.namedVars[k] = v
-	}
 	return result
 }
 
@@ -342,8 +331,10 @@ func (t *template) parse() (Template, error) {
 	}
 	splitOps := append(t.pathSplitOpts, &partCapture{template: t})
 	_, err := uriSplitter.Split(t.originalTemplate, splitOps...)
-	if t.posOnlyCount > 0 && len(t.namedVars) > 0 {
+	if t.posVarsCount > 0 && t.nameVarsCount > 0 {
 		return nil, newTemplateParseError("template cannot contain both positional and named path variables", 0, nil)
+	} else if t.nameVarsCount > 0 {
+		t.varsType = Names
 	}
 	if err != nil {
 		if terr := errors.Unwrap(err); terr != nil {
@@ -377,11 +368,10 @@ func (t *template) newUriPathPart(pt string, pos int, subParts []splitter.SubPar
 func (t *template) addVar(pt pathPart) {
 	if !pt.fixed {
 		if pt.name != "" {
-			t.namedVars[pt.name] = append(t.namedVars[pt.name], pt)
+			t.nameVarsCount++
 		} else {
-			t.posOnlyCount++
+			t.posVarsCount++
 		}
-		t.positionalVars = append(t.positionalVars, pt)
 	}
 }
 

template_parse_test.go

@@ -33,10 +33,8 @@ func TestNewTemplate(t *testing.T) {
 	require.Equal(t, `^[a-z]*$`, rt.pathParts[4].regexp.String())
 	require.Equal(t, `qux`, rt.pathParts[4].name)
 
-	require.Equal(t, 3, len(rt.namedVars))
-	require.Equal(t, 2, len(rt.namedVars["bar"]))
-	require.Equal(t, 1, len(rt.namedVars["baz"]))
-	require.Equal(t, 1, len(rt.namedVars["qux"]))
+	require.Equal(t, 4, rt.nameVarsCount)
+	require.Equal(t, 0, rt.posVarsCount)
 }
 
 func TestNewTemplatePositional(t *testing.T) {

template_test.go

@@ -154,6 +154,8 @@ func TestTemplate_ResolveTo(t *testing.T) {
 	rt, ok := tmp2.(*template)
 	require.True(t, ok)
 	require.NotNil(t, rt)
+	require.Equal(t, 0, rt.posVarsCount)
+	require.Equal(t, 1, rt.nameVarsCount)
 	require.Equal(t, 4, len(rt.pathParts))
 	require.True(t, rt.pathParts[0].fixed)
 	require.True(t, rt.pathParts[1].fixed)
@@ -172,10 +174,18 @@ func TestTemplate_ResolveTo(t *testing.T) {
 		"bar", "345"))
 	require.NoError(t, err)
 	require.Equal(t, `/foo/fooey/bar/--abc-345--`, tmp2.OriginalTemplate())
+	rt, ok = tmp2.(*template)
+	require.True(t, ok)
+	require.Equal(t, 0, rt.posVarsCount)
+	require.Equal(t, 0, rt.nameVarsCount)
 
 	tmp2, err = tmp.ResolveTo(Named("bar", "abc"))
 	require.NoError(t, err)
 	require.Equal(t, `/foo/{foo:[a-z]*}/bar/--abc-{bar:[0-9]*}--`, tmp2.OriginalTemplate())
+	rt, ok = tmp2.(*template)
+	require.True(t, ok)
+	require.Equal(t, 0, rt.posVarsCount)
+	require.Equal(t, 2, rt.nameVarsCount)
 }
 
 func TestTemplate_ResolveTo_Positional(t *testing.T) {
@@ -185,10 +195,18 @@ func TestTemplate_ResolveTo_Positional(t *testing.T) {
 	tmp2, err := tmp.ResolveTo(Positional())
 	require.NoError(t, err)
 	require.Equal(t, `/foo/?/bar/?/baz/?`, tmp2.OriginalTemplate())
+	rt2, ok := tmp2.(*template)
+	require.True(t, ok)
+	require.Equal(t, 3, rt2.posVarsCount)
+	require.Equal(t, 0, rt2.nameVarsCount)
 
 	tmp2, err = tmp.ResolveTo(Positional("fooey"))
 	require.NoError(t, err)
 	require.Equal(t, `/foo/fooey/bar/?/baz/?`, tmp2.OriginalTemplate())
+	rt2, ok = tmp2.(*template)
+	require.True(t, ok)
+	require.Equal(t, 2, rt2.posVarsCount)
+	require.Equal(t, 0, rt2.nameVarsCount)
 
 	tmp2, err = tmp.ResolveTo(Positional("fooey", "barey"))
 	require.NoError(t, err)
@@ -202,6 +220,10 @@ func TestTemplate_ResolveTo_Positional(t *testing.T) {
 	str, err := tmp2.PathFrom(Positional())
 	require.NoError(t, err)
 	require.Equal(t, `/foo/fooey/bar/barey/baz/bazey`, str)
+	rt2, ok = tmp2.(*template)
+	require.True(t, ok)
+	require.Equal(t, 0, rt2.posVarsCount)
+	require.Equal(t, 0, rt2.nameVarsCount)
 }
 
 func TestCaseInsensitiveFixed_Match(t *testing.T) {