Compare commits

...

4 Commits

Author SHA1 Message Date
Adrien Marquès 8cebf52405 add internal/config test for the Browse() method
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
2019-11-21 22:00:48 +01:00
Adrien Marquès aada9edff5 test internal/config parsing and illegal/missing fields 2019-11-21 21:48:24 +01:00
Adrien Marquès 52b42e479a parameter name conflict: on rename conflicting rename, return original name 2019-11-21 21:34:25 +01:00
Adrien Marquès 19e203c364 refactor internal/config
- create cerr (constant errors) with wrapped context (service name, method, parameter name)
- fix comments numbering
- remove duplicate check
2019-11-21 21:29:23 +01:00
4 changed files with 569 additions and 35 deletions

View File

@ -0,0 +1,522 @@
package config
import (
"fmt"
"net/http"
"strings"
"testing"
)
func TestLegalServiceName(t *testing.T) {
tests := []struct {
Raw string
Error error
}{
{
`{
"/": {
"invalid/service-name": {
}
}
}`,
ErrFormat.Wrap(ErrIllegalServiceName.WrapString("invalid/service-name")),
},
{
`{
"/": {
"invalid/service/name": {
}
}
}`,
ErrFormat.Wrap(ErrIllegalServiceName.WrapString("invalid/service/name")),
},
{
`{
"/": {
"invalid-service-name": {
}
}
}`,
ErrFormat.Wrap(ErrIllegalServiceName.WrapString("invalid-service-name")),
},
{
`{
"/": {
"valid.service_name": {
}
}
}`,
nil,
},
}
for i, test := range tests {
t.Run(fmt.Sprintf("service.%d", i), func(t *testing.T) {
_, err := Parse(strings.NewReader(test.Raw))
if err == nil && test.Error != nil {
t.Errorf("expected an error: '%s'", test.Error.Error())
t.FailNow()
}
if err != nil && test.Error == nil {
t.Errorf("unexpected error: '%s'", err.Error())
t.FailNow()
}
if err != nil && test.Error != nil {
if err.Error() != test.Error.Error() {
t.Errorf("expected the error '%s' (got '%s')", test.Error.Error(), err.Error())
t.FailNow()
}
}
})
}
}
func TestAvailableMethods(t *testing.T) {
reader := strings.NewReader(`{
"GET": { "info": "info" },
"POST": { "info": "info" },
"PUT": { "info": "info" },
"DELETE": { "info": "info" }
}`)
srv, err := Parse(reader)
if err != nil {
t.Errorf("unexpected error (got '%s')", err)
t.FailNow()
}
if srv.Method(http.MethodGet) == nil {
t.Errorf("expected method GET to be available")
t.Fail()
}
if srv.Method(http.MethodPost) == nil {
t.Errorf("expected method POST to be available")
t.Fail()
}
if srv.Method(http.MethodPut) == nil {
t.Errorf("expected method PUT to be available")
t.Fail()
}
if srv.Method(http.MethodDelete) == nil {
t.Errorf("expected method DELETE to be available")
t.Fail()
}
if srv.Method(http.MethodPatch) != nil {
t.Errorf("expected method PATH to be UNavailable")
t.Fail()
}
}
func TestParseEmpty(t *testing.T) {
reader := strings.NewReader(`{}`)
_, err := Parse(reader)
if err != nil {
t.Errorf("unexpected error (got '%s')", err)
t.FailNow()
}
}
func TestParseJsonError(t *testing.T) {
reader := strings.NewReader(`{
"GET": {
"info": "info
},
}`) // trailing ',' is invalid JSON
_, err := Parse(reader)
if err == nil {
t.Errorf("expected error")
t.FailNow()
}
}
func TestParseMissingMethodDescription(t *testing.T) {
tests := []struct {
Raw string
Error error
}{
{ // missing description
`{
"GET": {
}
}`,
ErrFormat.Wrap(ErrMissingMethodDesc.WrapString("GET /")),
},
{ // empty description
`{
"GET": {
"info": ""
}
}`,
ErrFormat.Wrap(ErrMissingMethodDesc.WrapString("GET /")),
},
{ // valid description
`{
"GET": {
"info": "a"
}
}`,
nil,
},
{ // valid description
`{
"GET": {
"info": "some description"
}
}`,
nil,
},
}
for i, test := range tests {
t.Run(fmt.Sprintf("method.%d", i), func(t *testing.T) {
_, err := Parse(strings.NewReader(test.Raw))
if err == nil && test.Error != nil {
t.Errorf("expected an error: '%s'", test.Error.Error())
t.FailNow()
}
if err != nil && test.Error == nil {
t.Errorf("unexpected error: '%s'", err.Error())
t.FailNow()
}
if err != nil && test.Error != nil {
if err.Error() != test.Error.Error() {
t.Errorf("expected the error '%s' (got '%s')", test.Error.Error(), err.Error())
t.FailNow()
}
}
})
}
}
func TestParseParameters(t *testing.T) {
tests := []struct {
Raw string
Error error
ErrorAlternative error
}{
{ // invalid param name prefix
`{
"GET": {
"info": "info",
"in": {
"_param1": {}
}
}
}`,
ErrFormat.Wrap(ErrIllegalParamName.WrapString("GET / {_param1}")),
nil,
},
{ // invalid param name suffix
`{
"GET": {
"info": "info",
"in": {
"param1_": {}
}
}
}`,
ErrFormat.Wrap(ErrIllegalParamName.WrapString("GET / {param1_}")),
nil,
},
{ // missing param description
`{
"GET": {
"info": "info",
"in": {
"param1": {}
}
}
}`,
ErrFormat.Wrap(ErrMissingParamDesc.WrapString("GET / {param1}")),
nil,
},
{ // empty param description
`{
"GET": {
"info": "info",
"in": {
"param1": {
"info": ""
}
}
}
}`,
ErrFormat.Wrap(ErrMissingParamDesc.WrapString("GET / {param1}")),
nil,
},
{ // missing param type
`{
"GET": {
"info": "info",
"in": {
"param1": {
"info": "valid"
}
}
}
}`,
ErrFormat.Wrap(ErrMissingParamType.WrapString("GET / {param1}")),
nil,
},
{ // empty param type
`{
"GET": {
"info": "info",
"in": {
"param1": {
"info": "valid",
"type": ""
}
}
}
}`,
ErrFormat.Wrap(ErrMissingParamType.WrapString("GET / {param1}")),
nil,
},
{ // valid description + valid type
`{
"GET": {
"info": "info",
"in": {
"param1": {
"info": "valid",
"type": "valid"
}
}
}
}`,
nil,
nil,
},
{ // name conflict with rename
`{
"GET": {
"info": "info",
"in": {
"param1": { "info": "valid", "type": "valid" },
"param2": { "info": "valid", "type": "valid", "name": "param1" }
}
}
}`,
// 2 possible errors as map order is not deterministic
ErrFormat.Wrap(ErrParamNameConflict.WrapString("GET / {param1}")),
ErrFormat.Wrap(ErrParamNameConflict.WrapString("GET / {param2}")),
},
{ // rename conflict with name
`{
"GET": {
"info": "info",
"in": {
"param1": { "info": "valid", "type": "valid", "name": "param2" },
"param2": { "info": "valid", "type": "valid" }
}
}
}`,
// 2 possible errors as map order is not deterministic
ErrFormat.Wrap(ErrParamNameConflict.WrapString("GET / {param1}")),
ErrFormat.Wrap(ErrParamNameConflict.WrapString("GET / {param2}")),
},
{ // rename conflict with rename
`{
"GET": {
"info": "info",
"in": {
"param1": { "info": "valid", "type": "valid", "name": "conflict" },
"param2": { "info": "valid", "type": "valid", "name": "conflict" }
}
}
}`,
// 2 possible errors as map order is not deterministic
ErrFormat.Wrap(ErrParamNameConflict.WrapString("GET / {param1}")),
ErrFormat.Wrap(ErrParamNameConflict.WrapString("GET / {param2}")),
},
{ // both renamed with no conflict
`{
"GET": {
"info": "info",
"in": {
"param1": { "info": "valid", "type": "valid", "name": "freename" },
"param2": { "info": "valid", "type": "valid", "name": "freename2" }
}
}
}`,
nil,
nil,
},
}
for i, test := range tests {
t.Run(fmt.Sprintf("method.%d", i), func(t *testing.T) {
_, err := Parse(strings.NewReader(test.Raw))
if err == nil && test.Error != nil {
t.Errorf("expected an error: '%s'", test.Error.Error())
t.FailNow()
}
if err != nil && test.Error == nil {
t.Errorf("unexpected error: '%s'", err.Error())
t.FailNow()
}
if err != nil && test.Error != nil {
if err.Error() != test.Error.Error() && err.Error() != test.ErrorAlternative.Error() {
t.Errorf("expected the error '%s' (got '%s')", test.Error.Error(), err.Error())
t.FailNow()
}
}
})
}
}
func TestBrowseSimple(t *testing.T) {
tests := []struct {
Raw string
Path []string
BrowseDepth int
ValidDepth bool
}{
{ // false positive -1
`{
"/" : {
"parent": {
"/": {
"subdir": {}
}
}
}
}`,
[]string{"parent", "subdir"},
1,
false,
},
{ // false positive +1
`{
"/" : {
"parent": {
"/": {
"subdir": {}
}
}
}
}`,
[]string{"parent", "subdir"},
3,
false,
},
{
`{
"/" : {
"parent": {
"/": {
"subdir": {}
}
}
}
}`,
[]string{"parent", "subdir"},
2,
true,
},
{ // unknown path
`{
"/" : {
"parent": {
"/": {
"subdir": {}
}
}
}
}`,
[]string{"x", "y"},
2,
false,
},
{ // unknown path
`{
"/" : {
"parent": {
"/": {
"subdir": {}
}
}
}
}`,
[]string{"parent", "y"},
1,
true,
},
{ // Warning: this case is important to understand the precedence of service paths over
// the value of some variables. Here if we send a string parameter in the GET method that
// unfortunately is equal to 'subdir', it will call the sub-service /parent/subdir' instead
// of the service /parent with its parameter set to the value 'subdir'.
`{
"/" : {
"parent": {
"/": {
"subdir": {}
},
"GET": {
"info": "valid-desc",
"in": {
"some-value": {
"info": "valid-desc",
"type": "valid-type"
}
}
}
}
}
}`,
[]string{"parent", "subdir"},
2,
true,
},
}
for i, test := range tests {
t.Run(fmt.Sprintf("method.%d", i), func(t *testing.T) {
srv, err := Parse(strings.NewReader(test.Raw))
if err != nil {
t.Errorf("unexpected error: '%s'", err)
t.FailNow()
}
_, depth := srv.Browse(test.Path)
if test.ValidDepth {
if depth != test.BrowseDepth {
t.Errorf("expected a depth of %d (got %d)", test.BrowseDepth, depth)
t.FailNow()
}
} else {
if depth == test.BrowseDepth {
t.Errorf("expected a depth NOT %d (got %d)", test.BrowseDepth, depth)
t.FailNow()
}
}
})
}
}

27
internal/config/errors.go Normal file
View File

@ -0,0 +1,27 @@
package config
import "git.xdrm.io/go/aicra/internal/cerr"
// ErrRead - a problem ocurred when trying to read the configuration file
const ErrRead = cerr.Error("cannot read config")
// ErrFormat - a invalid format has been detected
const ErrFormat = cerr.Error("invalid config format")
// ErrIllegalServiceName - an illegal character has been found in a service name
const ErrIllegalServiceName = cerr.Error("service must not contain any slash '/' nor '-' symbols")
// ErrMissingMethodDesc - a method is missing its description
const ErrMissingMethodDesc = cerr.Error("missing method description")
// ErrMissingParamDesc - a parameter is missing its description
const ErrMissingParamDesc = cerr.Error("missing parameter description")
// ErrIllegalParamName - a parameter has an illegal name
const ErrIllegalParamName = cerr.Error("illegal parameter name (must not begin/end with '_')")
// ErrMissingParamType - a parameter has an illegal type
const ErrMissingParamType = cerr.Error("missing parameter type")
// ErrParamNameConflict - a parameter has a conflict with its name/rename field
const ErrParamNameConflict = cerr.Error("name conflict for parameter")

View File

@ -1,7 +1,6 @@
package config package config
import ( import (
"fmt"
"strings" "strings"
) )
@ -10,7 +9,7 @@ func (methodDef *Method) checkAndFormat(servicePath string, httpMethod string) e
// 1. fail on missing description // 1. fail on missing description
if len(methodDef.Description) < 1 { if len(methodDef.Description) < 1 {
return fmt.Errorf("missing %s.%s description", servicePath, httpMethod) return ErrMissingMethodDesc.WrapString(httpMethod + " " + servicePath)
} }
// 2. stop if no parameter // 2. stop if no parameter
@ -22,16 +21,16 @@ func (methodDef *Method) checkAndFormat(servicePath string, httpMethod string) e
// 3. for each parameter // 3. for each parameter
for pName, pData := range methodDef.Parameters { for pName, pData := range methodDef.Parameters {
// check name // 3.1. check name
if strings.Trim(pName, "_") != pName { if strings.Trim(pName, "_") != pName {
return fmt.Errorf("invalid name '%s' must not begin/end with '_'", pName) return ErrIllegalParamName.WrapString(httpMethod + " " + servicePath + " {" + pName + "}")
} }
if len(pData.Rename) < 1 { if len(pData.Rename) < 1 {
pData.Rename = pName pData.Rename = pName
} }
// 5. Check for name/rename conflict // 3.2. Check for name/rename conflict
for paramName, param := range methodDef.Parameters { for paramName, param := range methodDef.Parameters {
// ignore self // ignore self
@ -39,39 +38,34 @@ func (methodDef *Method) checkAndFormat(servicePath string, httpMethod string) e
continue continue
} }
// 1. Same rename field // 3.2.1. Same rename field
if pData.Rename == param.Rename { if pData.Rename == param.Rename {
return fmt.Errorf("rename conflict for %s.%s parameter '%s'", servicePath, httpMethod, pData.Rename) return ErrParamNameConflict.WrapString(httpMethod + " " + servicePath + " {" + pName + "}")
} }
// 2. Not-renamed field matches a renamed field // 3.2.2. Not-renamed field matches a renamed field
if pName == param.Rename { if pName == param.Rename {
return fmt.Errorf("name conflict for %s.%s parameter '%s'", servicePath, httpMethod, pName) return ErrParamNameConflict.WrapString(httpMethod + " " + servicePath + " {" + pName + "}")
} }
// 3. Renamed field matches name // 3.2.3. Renamed field matches name
if pData.Rename == paramName { if pData.Rename == paramName {
return fmt.Errorf("name conflict for %s.%s parameter '%s'", servicePath, httpMethod, pName) return ErrParamNameConflict.WrapString(httpMethod + " " + servicePath + " {" + pName + "}")
} }
} }
// 6. Manage invalid type // 3.3. Fail on missing description
if len(pData.Type) < 1 {
return fmt.Errorf("invalid type for %s.%s parameter '%s'", servicePath, httpMethod, pName)
}
// 7. Fail on missing description
if len(pData.Description) < 1 { if len(pData.Description) < 1 {
return fmt.Errorf("missing description for %s.%s parameter '%s'", servicePath, httpMethod, pName) return ErrMissingParamDesc.WrapString(httpMethod + " " + servicePath + " {" + pName + "}")
} }
// 8. Fail on missing type // 3.4. Manage invalid type
if len(pData.Type) < 1 { if len(pData.Type) < 1 {
return fmt.Errorf("missing type for %s.%s parameter '%s'", servicePath, httpMethod, pName) return ErrMissingParamType.WrapString(httpMethod + " " + servicePath + " {" + pName + "}")
} }
// 9. Set optional + type // 3.5. Set optional + type
if pData.Type[0] == '?' { if pData.Type[0] == '?' {
pData.Optional = true pData.Optional = true
pData.Type = pData.Type[1:] pData.Type = pData.Type[1:]

View File

@ -2,20 +2,11 @@ package config
import ( import (
"encoding/json" "encoding/json"
"fmt"
"io" "io"
"net/http" "net/http"
"strings" "strings"
"git.xdrm.io/go/aicra/internal/cerr"
) )
// ErrRead - a problem ocurred when trying to read the configuration file
const ErrRead = cerr.Error("cannot read config")
// ErrFormat - a invalid format has been detected
const ErrFormat = cerr.Error("invalid config format")
// Parse builds a service from a json reader and checks for most format errors. // Parse builds a service from a json reader and checks for most format errors.
func Parse(r io.Reader) (*Service, error) { func Parse(r io.Reader) (*Service, error) {
receiver := &Service{} receiver := &Service{}
@ -87,20 +78,20 @@ func (svc *Service) checkAndFormat(servicePath string) error {
} }
} }
// 1. stop if no child */ // 2. stop if no child */
if svc.Children == nil || len(svc.Children) < 1 { if svc.Children == nil || len(svc.Children) < 1 {
return nil return nil
} }
// 2. for each service */ // 3. for each service */
for childService, ctl := range svc.Children { for childService, ctl := range svc.Children {
// 3. invalid name */ // 3.1. invalid name */
if strings.ContainsAny(childService, "/-") { if strings.ContainsAny(childService, "/-") {
return fmt.Errorf("service '%s' must not contain any slash '/' nor '-' symbols", childService) return ErrIllegalServiceName.WrapString(childService)
} }
// 4. check recursively */ // 3.2. check recursively */
err := ctl.checkAndFormat(childService) err := ctl.checkAndFormat(childService)
if err != nil { if err != nil {
return err return err