config: refactor, simplify, test, remove redundant comments

internal/config/config.go

@@ -16,17 +16,18 @@ type Server struct {
 	Services []*Service
 }
 
-// Parse a reader into a server. Server.Types must be set beforehand to
+// Parse a configuration into a server. Server.Types must be set beforehand to
 // make datatypes available when checking and formatting the read configuration.
 func (srv *Server) Parse(r io.Reader) error {
-	if err := json.NewDecoder(r).Decode(&srv.Services); err != nil {
+	err := json.NewDecoder(r).Decode(&srv.Services)
+	if err != nil {
 		return fmt.Errorf("%s: %w", errRead, err)
 	}
 
-	if err := srv.validate(); err != nil {
+	err = srv.validate()
+	if err != nil {
 		return fmt.Errorf("%s: %w", errFormat, err)
 	}
-
 	return nil
 }
 
@@ -39,11 +40,9 @@ func (server Server) validate(datatypes ...datatype.T) error {
 		}
 	}
 
-	// check for collisions
 	if err := server.collide(); err != nil {
 		return fmt.Errorf("%s: %w", errFormat, err)
 	}
-
 	return nil
 }
 
@@ -58,7 +57,11 @@ func (server Server) Find(r *http.Request) *Service {
 	return nil
 }
 
-// collide returns if there is collision between services
+// collide returns if there is collision between any service for the same method and colliding paths.
+// Note that service path collision detection relies on datatypes:
+//  - example 1: `/user/{id}` and `/user/articles` will not collide as {id} is an int and "articles" is not
+//  - example 2: `/user/{name}` and `/user/articles` will collide as {name} is a string so as "articles"
+//  - example 3: `/user/{name}` and `/user/{id}` will collide as {name} and {id} cannot be checked against their potential values
 func (server *Server) collide() error {
 	length := len(server.Services)
 
@@ -68,103 +71,104 @@ func (server *Server) collide() error {
 			aService := server.Services[a]
 			bService := server.Services[b]
 
-			// ignore different method
 			if aService.Method != bService.Method {
 				continue
 			}
 
-			aParts := SplitURL(aService.Pattern)
+			aURIParts := SplitURL(aService.Pattern)
-			bParts := SplitURL(bService.Pattern)
+			bURIParts := SplitURL(bService.Pattern)
-
+			if len(aURIParts) != len(bURIParts) {
-			// not same size
-			if len(aParts) != len(bParts) {
 				continue
 			}
 
-			partErrors := make([]error, 0)
+			err := checkURICollision(aURIParts, bURIParts, aService.Input, bService.Input)
-
+			if err != nil {
-			// for each part
+				return fmt.Errorf("(%s '%s') vs (%s '%s'): %w", aService.Method, aService.Pattern, bService.Method, bService.Pattern, err)
-			for pi, aPart := range aParts {
-				bPart := bParts[pi]
-
-				aIsCapture := len(aPart) > 1 && aPart[0] == '{'
-				bIsCapture := len(bPart) > 1 && bPart[0] == '{'
-
-				// both captures -> as we cannot check, consider a collision
-				if aIsCapture && bIsCapture {
-					partErrors = append(partErrors, fmt.Errorf("(%s '%s') vs (%s '%s'): %w (path %s and %s)", aService.Method, aService.Pattern, bService.Method, bService.Pattern, errPatternCollision, aPart, bPart))
-					continue
-				}
-
-				// no capture -> check equal
-				if !aIsCapture && !bIsCapture {
-					if aPart == bPart {
-						partErrors = append(partErrors, fmt.Errorf("(%s '%s') vs (%s '%s'): %w (same path '%s')", aService.Method, aService.Pattern, bService.Method, bService.Pattern, errPatternCollision, aPart))
-						continue
-					}
-				}
-
-				// A captures B -> check type (B is A ?)
-				if aIsCapture {
-					input, exists := aService.Input[aPart]
-
-					// fail if no type or no validator
-					if !exists || input.Validator == nil {
-						partErrors = append(partErrors, fmt.Errorf("(%s '%s') vs (%s '%s'): %w (invalid type for %s)", aService.Method, aService.Pattern, bService.Method, bService.Pattern, errPatternCollision, aPart))
-						continue
-					}
-
-					// fail if not valid
-					if _, valid := input.Validator(bPart); valid {
-						partErrors = append(partErrors, fmt.Errorf("(%s '%s') vs (%s '%s'): %w (%s captures '%s')", aService.Method, aService.Pattern, bService.Method, bService.Pattern, errPatternCollision, aPart, bPart))
-						continue
-					}
-
-					// B captures A -> check type (A is B ?)
-				} else if bIsCapture {
-					input, exists := bService.Input[bPart]
-
-					// fail if no type or no validator
-					if !exists || input.Validator == nil {
-						partErrors = append(partErrors, fmt.Errorf("(%s '%s') vs (%s '%s'): %w (invalid type for %s)", aService.Method, aService.Pattern, bService.Method, bService.Pattern, errPatternCollision, bPart))
-						continue
-					}
-
-					// fail if not valid
-					if _, valid := input.Validator(aPart); valid {
-						partErrors = append(partErrors, fmt.Errorf("(%s '%s') vs (%s '%s'): %w (%s captures '%s')", aService.Method, aService.Pattern, bService.Method, bService.Pattern, errPatternCollision, bPart, aPart))
-						continue
-					}
-				}
-
-				partErrors = append(partErrors, nil)
-
 			}
-
-			// if at least 1 url part does not match -> ok
-			var firstError error
-			oneMismatch := false
-			for _, err := range partErrors {
-				if err != nil && firstError == nil {
-					firstError = err
-				}
-
-				if err == nil {
-					oneMismatch = true
-					continue
-				}
-			}
-
-			if !oneMismatch {
-				return firstError
-			}
-
 		}
 	}
 
 	return nil
 }
 
+// check if uri of services A and B collide
+func checkURICollision(uriA, uriB []string, inputA, inputB map[string]*Parameter) error {
+	var errors = []error{}
+
+	// for each part
+	for pi, aPart := range uriA {
+		bPart := uriB[pi]
+
+		// no need for further check as it has been done earlier in the validation process
+		aIsCapture := len(aPart) > 1 && aPart[0] == '{'
+		bIsCapture := len(bPart) > 1 && bPart[0] == '{'
+
+		// both captures -> as we cannot check, consider a collision
+		if aIsCapture && bIsCapture {
+			errors = append(errors, fmt.Errorf("%w (path %s and %s)", errPatternCollision, aPart, bPart))
+			continue
+		}
+
+		// no capture -> check strict equality
+		if !aIsCapture && !bIsCapture {
+			if aPart == bPart {
+				errors = append(errors, fmt.Errorf("%w (same path '%s')", errPatternCollision, aPart))
+				continue
+			}
+		}
+
+		// A captures B -> check type (B is A ?)
+		if aIsCapture {
+			input, exists := inputA[aPart]
+
+			// fail if no type or no validator
+			if !exists || input.Validator == nil {
+				errors = append(errors, fmt.Errorf("%w (invalid type for %s)", errPatternCollision, aPart))
+				continue
+			}
+
+			// fail if not valid
+			if _, valid := input.Validator(bPart); valid {
+				errors = append(errors, fmt.Errorf("%w (%s captures '%s')", errPatternCollision, aPart, bPart))
+				continue
+			}
+
+			// B captures A -> check type (A is B ?)
+		} else if bIsCapture {
+			input, exists := inputB[bPart]
+
+			// fail if no type or no validator
+			if !exists || input.Validator == nil {
+				errors = append(errors, fmt.Errorf("%w (invalid type for %s)", errPatternCollision, bPart))
+				continue
+			}
+
+			// fail if not valid
+			if _, valid := input.Validator(aPart); valid {
+				errors = append(errors, fmt.Errorf("%w (%s captures '%s')", errPatternCollision, bPart, aPart))
+				continue
+			}
+		}
+
+		errors = append(errors, nil)
+
+	}
+
+	// at least 1 URI part not matching -> no collision
+	var firstError error
+	for _, err := range errors {
+		if err != nil && firstError == nil {
+			firstError = err
+		}
+
+		if err == nil {
+			return nil
+		}
+	}
+
+	return firstError
+}
+
 // SplitURL without empty sets
 func SplitURL(url string) []string {
 	trimmed := strings.Trim(url, " /\t\r\n")

internal/config/config_test.go

@@ -877,6 +877,36 @@ func TestMatchSimple(t *testing.T) {
 			"/a",
 			false,
 		},
+		{ // root url
+			`[ {
+					"method": "GET",
+					"path": "/a",
+					"info": "info",
+					"in": {}
+			} ]`,
+			"/",
+			false,
+		},
+		{
+			`[ {
+					"method": "GET",
+					"path": "/a",
+					"info": "info",
+					"in": {}
+			} ]`,
+			"/",
+			false,
+		},
+		{
+			`[ {
+					"method": "GET",
+					"path": "/",
+					"info": "info",
+					"in": {}
+			} ]`,
+			"/",
+			true,
+		},
 		{
 			`[ {
 					"method": "GET",
@@ -997,3 +1027,80 @@ func TestMatchSimple(t *testing.T) {
 	}
 
 }
+
+func TestFindPriority(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		Config       string
+		URL          string
+		MatchingDesc string
+	}{
+		{
+			`[
+				{ "method": "GET", "path": "/a", "info": "s1" },
+				{ "method": "GET", "path": "/",  "info": "s2" }
+			]`,
+			"/",
+			"s2",
+		},
+		{
+			`[
+				{ "method": "GET", "path": "/",  "info": "s2" },
+				{ "method": "GET", "path": "/a", "info": "s1" }
+			]`,
+			"/",
+			"s2",
+		},
+		{
+			`[
+				{ "method": "GET", "path": "/a", "info": "s1" },
+				{ "method": "GET", "path": "/",  "info": "s2" }
+			]`,
+			"/a",
+			"s1",
+		},
+		{
+			`[
+				{ "method": "GET", "path": "/a/b/c",  "info": "s1" },
+				{ "method": "GET", "path": "/a/b",    "info": "s2" }
+			]`,
+			"/a/b/c",
+			"s1",
+		},
+		{
+			`[
+				{ "method": "GET", "path": "/a/b/c",  "info": "s1" },
+				{ "method": "GET", "path": "/a/b",    "info": "s2" }
+			]`,
+			"/a/b/",
+			"s2",
+		},
+	}
+
+	for i, test := range tests {
+
+		t.Run(fmt.Sprintf("method.%d", i), func(t *testing.T) {
+			srv := &Server{}
+			srv.Types = append(srv.Types, builtin.AnyDataType{})
+			srv.Types = append(srv.Types, builtin.IntDataType{})
+			srv.Types = append(srv.Types, builtin.BoolDataType{})
+			err := srv.Parse(strings.NewReader(test.Config))
+
+			if err != nil {
+				t.Errorf("unexpected error: '%s'", err)
+				t.FailNow()
+			}
+
+			req := httptest.NewRequest(http.MethodGet, test.URL, nil)
+			service := srv.Find(req)
+			if service == nil {
+				t.Errorf("expected to find a service")
+				t.FailNow()
+			}
+			if service.Description != test.MatchingDesc {
+				t.Errorf("expected description '%s', got '%s'", test.MatchingDesc, service.Description)
+				t.FailNow()
+			}
+		})
+	}
+}

internal/config/errors.go

@@ -7,53 +7,53 @@ func (err cerr) Error() string {
 	return string(err)
 }
 
-// errRead - a problem ocurred when trying to read the configuration file
+// errRead - read error
 const errRead = cerr("cannot read config")
 
-// errUnknownMethod - invalid http method
+// errUnknownMethod - unknown http method
 const errUnknownMethod = cerr("unknown HTTP method")
 
-// errFormat - a invalid format has been detected
+// errFormat - invalid format
 const errFormat = cerr("invalid config format")
 
-// errPatternCollision - there is a collision between 2 services' patterns (same method)
+// errPatternCollision - collision between 2 services' patterns
 const errPatternCollision = cerr("pattern collision")
 
-// errInvalidPattern - a service pattern is malformed
+// errInvalidPattern - malformed service pattern
-const errInvalidPattern = cerr("must begin with a '/' and not end with")
+const errInvalidPattern = cerr("malformed service path: must begin with a '/' and not end with")
 
-// errInvalidPatternBraceCapture - a service pattern brace capture is invalid
+// errInvalidPatternBraceCapture - invalid brace capture
-const errInvalidPatternBraceCapture = cerr("invalid uri capturing braces")
+const errInvalidPatternBraceCapture = cerr("invalid uri parameter")
 
-// errUnspecifiedBraceCapture - a parameter brace capture is not specified in the pattern
+// errUnspecifiedBraceCapture - missing path brace capture
-const errUnspecifiedBraceCapture = cerr("capturing brace missing in the path")
+const errUnspecifiedBraceCapture = cerr("missing uri parameter")
 
-// errMandatoryRename - capture/query parameters must have a rename
+// errUndefinedBraceCapture - missing capturing brace definition
-const errMandatoryRename = cerr("capture and query parameters must have a 'name'")
+const errUndefinedBraceCapture = cerr("missing uri parameter definition")
 
-// errUndefinedBraceCapture - a parameter brace capture in the pattern is not defined in parameters
+// errMandatoryRename - capture/query parameters must be renamed
-const errUndefinedBraceCapture = cerr("capturing brace missing input definition")
+const errMandatoryRename = cerr("uri and query parameters must be renamed")
 
 // errMissingDescription - a service is missing its description
 const errMissingDescription = cerr("missing description")
 
-// errIllegalOptionalURIParam - an URI parameter cannot be optional
+// errIllegalOptionalURIParam - uri parameter cannot optional
-const errIllegalOptionalURIParam = cerr("URI parameter cannot be optional")
+const errIllegalOptionalURIParam = cerr("uri parameter cannot be optional")
 
-// errOptionalOption - an output is optional
+// errOptionalOption - cannot have optional output
 const errOptionalOption = cerr("output cannot be optional")
 
-// errMissingParamDesc - a parameter is missing its description
+// errMissingParamDesc - missing parameter description
 const errMissingParamDesc = cerr("missing parameter description")
 
-// errUnknownDataType - a parameter has an unknown datatype name
+// errUnknownDataType - unknown parameter datatype
-const errUnknownDataType = cerr("unknown data type")
+const errUnknownDataType = cerr("unknown parameter datatype")
 
-// errIllegalParamName - a parameter has an illegal name
+// errIllegalParamName - illegal parameter name
 const errIllegalParamName = cerr("illegal parameter name")
 
-// errMissingParamType - a parameter has an illegal type
+// errMissingParamType - missing parameter type
 const errMissingParamType = cerr("missing parameter type")
 
-// errParamNameConflict - a parameter has a conflict with its name/rename field
+// errParamNameConflict - name/rename conflict
-const errParamNameConflict = cerr("name conflict for parameter")
+const errParamNameConflict = cerr("parameter name conflict")

internal/config/parameter.go

@@ -11,33 +11,29 @@ type Parameter struct {
 	Description string `json:"info"`
 	Type        string `json:"type"`
 	Rename      string `json:"name,omitempty"`
-	// ExtractType is the type of data the datatype returns
+	Optional    bool
+	// ExtractType is the type the Validator will cast into
 	ExtractType reflect.Type
-	// Optional is set to true when the type is prefixed with '?'
+	// Validator is inferred from the "type" property
-	Optional bool
-
-	// Validator is inferred from @Type
 	Validator datatype.Validator
 }
 
 func (param *Parameter) validate(datatypes ...datatype.T) error {
-	// missing description
 	if len(param.Description) < 1 {
 		return errMissingParamDesc
 	}
 
-	// invalid type
 	if len(param.Type) < 1 || param.Type == "?" {
 		return errMissingParamType
 	}
 
-	// optional type transform
+	// optional type
 	if param.Type[0] == '?' {
 		param.Optional = true
 		param.Type = param.Type[1:]
 	}
 
-	// assign the datatype
+	// find validator
 	for _, dtype := range datatypes {
 		param.Validator = dtype.Build(param.Type, datatypes...)
 		param.ExtractType = dtype.Type()
@@ -48,6 +44,5 @@ func (param *Parameter) validate(datatypes ...datatype.T) error {
 	if param.Validator == nil {
 		return errUnknownDataType
 	}
-
 	return nil
 }

internal/config/service.go

@@ -22,15 +22,15 @@ type Service struct {
 	Input       map[string]*Parameter `json:"in"`
 	Output      map[string]*Parameter `json:"out"`
 
-	// references to url parameters
+	// Captures contains references to URI parameters from the `Input` map. The format
-	// format: '/uri/{param}'
+	// of these parameter names is "{paramName}"
 	Captures []*BraceCapture
 
-	// references to Query parameters
+	// Query contains references to HTTP Query parameters from the `Input` map.
-	// format: 'GET@paranName'
+	// Query parameters names are "GET@paramName", this map contains escaped names (e.g. "paramName")
 	Query map[string]*Parameter
 
-	// references for form parameters (all but Captures and Query)
+	// Form references form parameters from the `Input` map (all but Captures and Query).
 	Form map[string]*Parameter
 }
 
@@ -43,16 +43,12 @@ type BraceCapture struct {
 
 // Match returns if this service would handle this HTTP request
 func (svc *Service) Match(req *http.Request) bool {
-	// method
 	if req.Method != svc.Method {
 		return false
 	}
-
-	// check path
 	if !svc.matchPattern(req.RequestURI) {
 		return false
 	}
-
 	return true
 }
 
@@ -61,13 +57,12 @@ func (svc *Service) matchPattern(uri string) bool {
 	uriparts := SplitURL(uri)
 	parts := SplitURL(svc.Pattern)
 
-	// fail if size differ
 	if len(uriparts) != len(parts) {
 		return false
 	}
 
 	// root url '/'
-	if len(parts) == 0 {
+	if len(parts) == 0 && len(uriparts) == 0 {
 		return true
 	}