Skip to content

Commit

Permalink
WIP implement validateResourceSchemaDefWithOptions
Browse files Browse the repository at this point in the history 
- refactor bodyParameterExists to no longer return error
- refactor tests with new bodyParameterExists signature
- add tests for validateResourceSchemaDefWithOptions
  • Loading branch information
@lillchan
lillchan committed Oct 28, 2019
1 parent 89fa2b2 commit 7384ccc
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 31 deletions.
40 changes: 25 additions & 15 deletions openapi/openapi_v2_spec_analyser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,30 +343,40 @@ func (specAnalyser *specV2Analyser) validateRootPath(resourcePath string) (strin

resourceRootPostSchemaDef, err := specAnalyser.getBodyParameterBodySchema(resourceRootPostOperation)
if err != nil {
//bodyParam := specAnalyser.bodyParameterExists(resourceRootPostOperation)
//if bodyParam == nil {
// TODO: Integrate specAnalyser.validateResourceSchemaDefWithOptions()
//}
return "", nil, nil, fmt.Errorf("resource root path '%s' POST operation validation error: %s", resourceRootPath, err)
}

return resourceRootPath, &resourceRootPathItem, resourceRootPostSchemaDef, nil
}

func (specAnalyser *specV2Analyser) validateResourceSchemaDefinition(schema *spec.Schema) error {
identifier := ""
func (specAnalyser *specV2Analyser) validateResourceSchemaDefWithOptions(schema *spec.Schema, shouldPropBeReadOnly bool) error {
containsIdentifier := false
for propertyName, property := range schema.Properties {
if propertyName == "id" {
identifier = propertyName
continue
containsIdentifier = true
} else if exists, useAsIdentifier := property.Extensions.GetBool(extTfID); exists && useAsIdentifier {
containsIdentifier = true
}
if exists, useAsIdentifier := property.Extensions.GetBool(extTfID); exists && useAsIdentifier {
identifier = propertyName
break
if shouldPropBeReadOnly {
if property.ReadOnly == false {
return fmt.Errorf("resource schema contains properties that are not just read only")
}
}
}
if identifier == "" {
if containsIdentifier == false {
return fmt.Errorf("resource schema is missing a property that uniquely identifies the resource, either a property named 'id' or a property with the extension '%s' set to true", extTfID)
}
return nil
}

func (specAnalyser *specV2Analyser) validateResourceSchemaDefinition(schema *spec.Schema) error {
return specAnalyser.validateResourceSchemaDefWithOptions(schema, false)
}

// postIsPresent checks if the given resource has a POST implementation returning true if the path is found
// in paths and the path exposes a POST operation
func (specAnalyser *specV2Analyser) postDefined(resourceRootPath string) bool {
Expand All @@ -377,22 +387,22 @@ func (specAnalyser *specV2Analyser) postDefined(resourceRootPath string) bool {
return true
}

func (SpecAnalyser *specV2Analyser) bodyParameterExists(resourceRootPostOperation *spec.Operation) (*spec.Parameter, error) {
func (SpecAnalyser *specV2Analyser) bodyParameterExists(resourceRootPostOperation *spec.Operation) *spec.Parameter {
if resourceRootPostOperation == nil {
return nil, fmt.Errorf("resource root operation does not have a POST operation")
return nil
}
for _, parameter := range resourceRootPostOperation.Parameters {
if parameter.In == "body" {
return &parameter, nil
return &parameter
}
}
return nil, fmt.Errorf("resource root operation missing the body parameter")
return nil
}

func (specAnalyser *specV2Analyser) getBodyParameterBodySchema(resourceRootPostOperation *spec.Operation) (*spec.Schema, error) {
bodyParameter, err := specAnalyser.bodyParameterExists(resourceRootPostOperation)
if err != nil {
return nil, err
bodyParameter := specAnalyser.bodyParameterExists(resourceRootPostOperation)
if bodyParameter == nil {
return nil, fmt.Errorf("resource root operation missing body parameter")
}

if bodyParameter.Schema == nil {
Expand Down
125 changes: 109 additions & 16 deletions openapi/openapi_v2_spec_analyser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,25 +172,22 @@ func Test_bodyParameterExists(t *testing.T) {
}
param := spec.Parameter{ParamProps: spec.ParamProps{In: "body", Schema: s}}
resourceRootPostOperation.Parameters = []spec.Parameter{param}
schema, err := specV2Analyser.bodyParameterExists(resourceRootPostOperation)
schema := specV2Analyser.bodyParameterExists(resourceRootPostOperation)
Convey("Then the schema returned should not be empty", func() {
So(schema, ShouldNotBeNil)
})
Convey("And the err returned should be nil", func() {
So(err, ShouldBeNil)
})
})
Convey("When bodyParameterExists is called with a nil arg", func() {
_, err := specV2Analyser.bodyParameterExists(nil)
Convey("Then the error returned should be the expected one", func() {
So(err.Error(), ShouldEqual, "resource root operation does not have a POST operation")
bodyParam := specV2Analyser.bodyParameterExists(nil)
Convey("Then the bodyParam returned should be nil", func() {
So(bodyParam, ShouldBeNil)
})
})
Convey("When bodyParameterExists is called with an empty Operation (no params)", func() {
resourceRootPostOperation := &spec.Operation{}
_, err := specV2Analyser.bodyParameterExists(resourceRootPostOperation)
Convey("Then the error returned should be the expected one", func() {
So(err.Error(), ShouldEqual, "resource root operation missing the body parameter")
bodyParam := specV2Analyser.bodyParameterExists(resourceRootPostOperation)
Convey("Then the bodyParam returned should be nil", func() {
So(bodyParam, ShouldBeNil)
})
})
Convey("When bodyParameterExists method is called with an operation that has multiple body parameters", func() {
Expand All @@ -212,10 +209,7 @@ func Test_bodyParameterExists(t *testing.T) {
},
},
}
schema, err := specV2Analyser.bodyParameterExists(operation)
Convey("Then the error should be nil", func() {
So(err, ShouldBeNil)
})
schema := specV2Analyser.bodyParameterExists(operation)
Convey("Then the schema returned should match the first body found", func() {
So(schema.ParamProps.Name, ShouldEqual, "first body")
})
Expand Down Expand Up @@ -248,14 +242,14 @@ func Test_getBodyParameterBodySchema(t *testing.T) {
Convey("When getBodyParameterBodySchema is called with a nil arg", func() {
_, err := specV2Analyser.getBodyParameterBodySchema(nil)
Convey("Then the error returned should be the expected one", func() {
So(err.Error(), ShouldEqual, "resource root operation does not have a POST operation")
So(err.Error(), ShouldEqual, "resource root operation missing body parameter")
})
})
Convey("When getBodyParameterBodySchema is called with an empty Operation (no params)", func() {
resourceRootPostOperation := &spec.Operation{}
_, err := specV2Analyser.getBodyParameterBodySchema(resourceRootPostOperation)
Convey("Then the error returned should be the expected one", func() {
So(err.Error(), ShouldEqual, "resource root operation missing the body parameter")
So(err.Error(), ShouldEqual, "resource root operation missing body parameter")
})
})
Convey("When getBodyParameterBodySchema is called with an Operation with OperationProps with a Parameter with an In:body ParamProp and NO Schema ParamProp", func() {
Expand Down Expand Up @@ -1315,6 +1309,105 @@ func TestValidateResourceSchemaDefinition(t *testing.T) {
})
}

func TestValidateResourceSchemaDefWithOptions(t *testing.T) {
Convey("Given an specV2Analyser", t, func() {
a := specV2Analyser{}
Convey("When validateResourceSchemaDefWithOptions method is called with a valid schema definition containing a property ID'", func() {
schema := &spec.Schema{
SchemaProps: spec.SchemaProps{
Properties: map[string]spec.Schema{
"id": {},
},
},
}
err := a.validateResourceSchemaDefWithOptions(schema, false)
Convey("Then error returned should be nil", func() {
So(err, ShouldBeNil)
})
})
Convey("When validateResourceSchemaDefWithOptions method is called with a valid schema definition missing an ID property but a different property acts as unique identifier'", func() {
schema := &spec.Schema{
SchemaProps: spec.SchemaProps{
Properties: map[string]spec.Schema{
"name": {
VendorExtensible: spec.VendorExtensible{
Extensions: spec.Extensions{
extTfID: true,
},
},
},
},
},
}
err := a.validateResourceSchemaDefWithOptions(schema, false)
Convey("Then error returned should be nil", func() {
So(err, ShouldBeNil)
})
})
Convey("When validateResourceSchemaDefWithOptions method is called with a valid schema definition with both a property that name 'id' and a different property with the 'x-terraform-id' extension'", func() {
schema := &spec.Schema{
SchemaProps: spec.SchemaProps{
Properties: map[string]spec.Schema{
"id": {},
"name": {
VendorExtensible: spec.VendorExtensible{
Extensions: spec.Extensions{
extTfID: true,
},
},
},
},
},
}
err := a.validateResourceSchemaDefWithOptions(schema, false)
Convey("Then error returned should be nil", func() {
So(err, ShouldBeNil)
})
})
Convey("When validateResourceSchemaDefWithOptions method is called with a NON valid schema definition due to missing unique identifier'", func() {
schema := &spec.Schema{
SchemaProps: spec.SchemaProps{
Properties: map[string]spec.Schema{
"name": {},
},
},
}
err := a.validateResourceSchemaDefWithOptions(schema, false)
Convey("And the error message should be", func() {
So(err.Error(), ShouldContainSubstring, "resource schema is missing a property that uniquely identifies the resource, either a property named 'id' or a property with the extension 'x-terraform-id' set to true")
})
})
Convey("When validateResourceSchemaDefWithOptions method is called with shouldReadyOnlyProps set to false and doesn't contain a mix of properties", func() {
schema := &spec.Schema{
SchemaProps: spec.SchemaProps{
Properties: map[string]spec.Schema{
"id": {SwaggerSchemaProps: spec.SwaggerSchemaProps{ReadOnly: true}},
"name": {SwaggerSchemaProps: spec.SwaggerSchemaProps{ReadOnly: false}},
},
},
}
err := a.validateResourceSchemaDefWithOptions(schema, false)
Convey("Then error returned should be nil", func() {
So(err, ShouldBeNil)
})
})
Convey("When validateResourceSchemaDefWithOptions method is called with shouldReadyOnlyProps set to true and contains not just read only props", func() {
schema := &spec.Schema{
SchemaProps: spec.SchemaProps{
Properties: map[string]spec.Schema{
"id": {SwaggerSchemaProps: spec.SwaggerSchemaProps{ReadOnly: true}},
"name": {SwaggerSchemaProps: spec.SwaggerSchemaProps{ReadOnly: false}},
},
},
}
err := a.validateResourceSchemaDefWithOptions(schema, true)
Convey("Then error returned should be as expected", func() {
So(err.Error(), ShouldEqual, "resource schema contains properties that are not just read only")
})
})
})
}

func TestValidateRootPath(t *testing.T) {
Convey("Given an specV2Analyser with a terraform compliant root path (and the schema has already been expanded)", t, func() {
swaggerContent := `swagger: "2.0"
Expand Down

0 comments on commit 7384ccc

Please sign in to comment.