Repository: anz-bank/sysl
Pull request #965 (2020-07-26 12:51:41)

More OpenAPI import fixes:
* handle referenced arrays better
* Handle OneOf keyword in objects



codecov[bot]: # [Codecov](https://codecov.io/gh/anz-bank/sysl/pull/965?src=pr&el=h1) Report
> Merging [#965](https://codecov.io/gh/anz-bank/sysl/pull/965?src=pr&el=desc) into [master](https://codecov.io/gh/anz-bank/sysl/commit/b37ce6254cec4a15899c576f3f02b5f367c9b1cd&el=desc) will **increase** coverage by `1.18%`.
> The diff coverage is `88.12%`.

```diff
@@            Coverage Diff             @@
##           master     #965      +/-   ##
==========================================
+ Coverage   83.35%   84.53%   +1.18%     
==========================================
  Files          73       75       +2     
  Lines       10609    10503     -106     
==========================================
+ Hits         8843     8879      +36     
+ Misses       1432     1313     -119     
+ Partials      334      311      -23     
```


| [Impacted Files](https://codecov.io/gh/anz-bank/sysl/pull/965?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/pbutil/output.go](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree#diff-cGtnL3BidXRpbC9vdXRwdXQuZ28=) | `76.00% <ø> (-0.48%)` | :arrow_down: |
| [pkg/syslwrapper/app.go](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree#diff-cGtnL3N5c2x3cmFwcGVyL2FwcC5nbw==) | `93.44% <ø> (-0.05%)` | :arrow_down: |
| [pkg/syslwrapper/test\_helper.go](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree#diff-cGtnL3N5c2x3cmFwcGVyL3Rlc3RfaGVscGVyLmdv) | `57.89% <ø> (-1.42%)` | :arrow_down: |
| [pkg/importer/importer.go](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree#diff-cGtnL2ltcG9ydGVyL2ltcG9ydGVyLmdv) | `63.15% <63.15%> (ø)` | |
| [pkg/mod/gomod.go](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree#diff-cGtnL21vZC9nb21vZC5nbw==) | `73.68% <66.66%> (+1.27%)` | :arrow_up: |
| [pkg/mod/module.go](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree#diff-cGtnL21vZC9tb2R1bGUuZ28=) | `66.66% <72.72%> (+0.95%)` | :arrow_up: |
| [pkg/parse/parse.go](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree#diff-cGtnL3BhcnNlL3BhcnNlLmdv) | `85.34% <73.07%> (+1.37%)` | :arrow_up: |
| [pkg/importer/openapi.go](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree#diff-cGtnL2ltcG9ydGVyL29wZW5hcGkuZ28=) | `85.96% <85.18%> (-3.48%)` | :arrow_down: |
| [pkg/parse/listener\_impl.go](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree#diff-cGtnL3BhcnNlL2xpc3RlbmVyX2ltcGwuZ28=) | `89.62% <86.48%> (+0.11%)` | :arrow_up: |
| [pkg/importer/swagger.go](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree#diff-cGtnL2ltcG9ydGVyL3N3YWdnZXIuZ28=) | `81.48% <88.88%> (+1.88%)` | :arrow_up: |
| ... and [22 more](https://codecov.io/gh/anz-bank/sysl/pull/965/diff?src=pr&el=tree-more) | |


Patch:
diff --git a/pkg/importer/openapi.go b/pkg/importer/openapi.go
index 5c9579b3..5357725c 100644
--- a/pkg/importer/openapi.go
+++ b/pkg/importer/openapi.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/url"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"sort"
 	"strings"
@@ -75,7 +76,7 @@ func (o *openapiv3) Load(file string) (string, error) {
 	if err != nil {
 		spec, _ = loader.LoadSwaggerFromData([]byte(file)) // nolint: errcheck
 	}
-	return o.convertSpec(spec, "")
+	return o.convertSpec(spec)
 }
 
 func (o *openapiv3) pushName(n string) func() {
@@ -86,9 +87,19 @@ func (o *openapiv3) pushName(n string) func() {
 }
 func (o *openapiv3) popName() { o.nameStack = o.nameStack[:len(o.nameStack)-1] }
 
-func (o *openapiv3) convertSpec(spec *openapi3.Swagger, basepath string) (string, error) {
+func orderedKeys(mapObj interface{}) []string {
+	var typeNames []string
+	for _, k := range reflect.ValueOf(mapObj).MapKeys() {
+		typeNames = append(typeNames, k.String())
+	}
+	sort.Strings(typeNames)
+	return typeNames
+}
+
+func (o *openapiv3) convertSpec(spec *openapi3.Swagger) (string, error) {
 	o.types = TypeList{}
-	for name, ref := range spec.Components.Schemas {
+	for _, name := range orderedKeys(spec.Components.Schemas) {
+		ref := spec.Components.Schemas[name]
 		if _, found := o.types.Find(name); !found {
 			if ref.Value == nil {
 				o.types.Add(NewStringAlias(name))
@@ -276,12 +287,22 @@ func makeSizeSpec(min uint64, max *uint64) *sizeSpec {
 func (o *openapiv3) buildField(name string, prop *openapi3.SchemaRef) Field {
 	f := Field{Name: name}
 	typeName := typeNameFromSchemaRef(prop)
+
+	if prop.Ref != "" {
+		f.Type = nameOnlyType(typeName)
+		return f
+	}
+
 	defer o.pushName(name)()
-	f.Type = o.typeAliasForSchema(prop)
-	isArray := prop.Value.Type == ArrayTypeName
-	if isArray {
+
+	if prop.Value.Type == ArrayTypeName && prop.Value.Items.Ref != "" {
+		f.Type = &Array{Items: nameOnlyType(typeNameFromSchemaRef(prop.Value.Items))}
 		f.SizeSpec = makeSizeSpec(prop.Value.MinItems, prop.Value.MaxItems)
+		return f
 	}
+
+	f.Type = o.typeAliasForSchema(prop)
+	isArray := prop.Value.Type == ArrayTypeName
 	if typeName == ObjectTypeName {
 		if isArray {
 			prop = prop.Value.Items
@@ -340,6 +361,14 @@ func (o *openapiv3) loadTypeSchema(name string, schema *openapi3.Schema) Type {
 			Attributes: getAttrs(schema),
 		}
 
+		// AllOf means this object is composed of all of the sub-schemas (and potentially addiitonal properties)
+		for _, subschema := range schema.AllOf {
+			subType := o.loadTypeSchema("", subschema.Value)
+			if subObj, ok := subType.(*StandardType); ok {
+				obj.Properties = append(obj.Properties, subObj.Properties...)
+			}
+		}
+
 		for fname, prop := range schema.Properties {
 			f := o.buildField(fname, prop)
 			f.Optional = !contains(fname, schema.Required)
diff --git a/pkg/importer/swagger.go b/pkg/importer/swagger.go
index 77e21e2f..9a1d7d62 100644
--- a/pkg/importer/swagger.go
+++ b/pkg/importer/swagger.go
@@ -46,7 +46,7 @@ func (l *OpenAPI2Importer) Load(oas2spec string) (string, error) {
 	}
 	l.openAPI2Spec = oas2spec
 	l.basePath = basePath
-	return l.convertSpec(oas3spec, basePath)
+	return l.convertSpec(oas3spec)
 }
 
 // Set the AppName of the imported app


Test patch:
diff --git a/pkg/importer/tests-openapi/petstore-expanded.sysl b/pkg/importer/tests-openapi/petstore-expanded.sysl
index 0d137881..1e8d19af 100644
--- a/pkg/importer/tests-openapi/petstore-expanded.sysl
+++ b/pkg/importer/tests-openapi/petstore-expanded.sysl
@@ -41,20 +41,20 @@ testapp "Swagger Petstore" [package="package_foo"]:
             | lorem. Pellentesque vitae felis pretium, pulvinar elit eu, euismod
             | sapien.
             return error <: Error
-            return ok <: sequence of EXTERNAL_Pet
+            return ok <: sequence of Pet
 
     /pets/{id<:int64}:
         GET:
             | Returns a user based on a single ID, if the user does not have
             | access to the pet
             return error <: Error
-            return ok <: EXTERNAL_Pet
+            return ok <: Pet
 
     /pets:
         POST (NewPetRequest <: NewPet [mediatype="application/json", ~body]):
             | Creates a new pet in the store. Duplicates are allowed
             return error <: Error
-            return ok <: EXTERNAL_Pet
+            return ok <: Pet
 
     /pets/{id<:int64}:
         DELETE:
@@ -77,8 +77,13 @@ testapp "Swagger Petstore" [package="package_foo"]:
         tag <: string?:
             @json_tag = "tag"
 
-    !alias EXTERNAL_Pet:
-        string
+    !type Pet:
+        id <: int64:
+            @json_tag = "id"
+        name <: string:
+            @json_tag = "name"
+        tag <: string?:
+            @json_tag = "tag"
 
     !alias tags:
         sequence of string
diff --git a/pkg/importer/tests-swagger/petstore-expanded.sysl b/pkg/importer/tests-swagger/petstore-expanded.sysl
index 836b13fc..de6ec2a9 100644
--- a/pkg/importer/tests-swagger/petstore-expanded.sysl
+++ b/pkg/importer/tests-swagger/petstore-expanded.sysl
@@ -18,20 +18,20 @@ testapp "Swagger Petstore" [package="package_foo"]:
         GET ?tags={tags}?&limit=int32?:
             | Returns all pets from the system that the user has access to
             return error <: ErrorModel
-            return ok <: sequence of EXTERNAL_Pet
+            return ok <: sequence of Pet
 
     /pets/{id<:int64}:
         GET:
             | Returns a user based on a single ID, if the user does not have
             | access to the pet
             return error <: ErrorModel
-            return ok <: EXTERNAL_Pet
+            return ok <: Pet
 
     /pets:
         POST (NewPetRequest <: NewPet [mediatype="application/json", ~body]):
             | Creates a new pet in the store.  Duplicates are allowed
             return error <: ErrorModel
-            return ok <: EXTERNAL_Pet
+            return ok <: Pet
 
     /pets/{id<:int64}:
         DELETE:
@@ -54,8 +54,13 @@ testapp "Swagger Petstore" [package="package_foo"]:
         tag <: string?:
             @json_tag = "tag"
 
-    !alias EXTERNAL_Pet:
-        string
+    !type Pet:
+        id <: int64:
+            @json_tag = "id"
+        name <: string:
+            @json_tag = "name"
+        tag <: string?:
+            @json_tag = "tag"
 
     !alias tags:
         sequence of string
diff --git a/pkg/parse/tests/openapi3.sysl.golden.textpb b/pkg/parse/tests/openapi3.sysl.golden.textpb
index 5b7503a6..69f82074 100644
--- a/pkg/parse/tests/openapi3.sysl.golden.textpb
+++ b/pkg/parse/tests/openapi3.sysl.golden.textpb
@@ -141,7 +141,7 @@ apps: {
         }
         stmt: {
           ret: {
-            payload: "ok <: sequence of Preferences_obj"
+            payload: "ok <: Preferences"
           }
         }
         rest_params: {