Repository: anz-bank/sysl
Issue: Need validation before unmarshall response in restlib

When we generate a client file in golang with two expected kinds of response it will generate code like `responses := []interface{}{&A{}, &B{}}`
This response will pass into `restlib.DoHTTPRequest()`
In this method's

```go
for _, respInterface := range response {
	err = json.Unmarshal(respBody, respInterface)
	if err == nil {
		return makeHTTPResult(httpResponse, respBody, respInterface), nil
	}
} 
```

It didn't do the validation before `unmarshall`, so this `unmarshall` method never failed, it always marshall the `respBody` to the first `respInterface` come up from the for loop.
In this case, the response body will always return as &A{}.



Changed files:
src/sysl/importers/import_swagger.py, test/importers/test_import_swagger.py, src/sysl/importers/import_openapi.py, test/importers/test_import_openapi.py

diff --git a/src/sysl/importers/import_openapi.py b/src/sysl/importers/import_openapi.py
index 69972b64..1925ef17 100644
--- a/src/sysl/importers/import_openapi.py
+++ b/src/sysl/importers/import_openapi.py
@@ -164,6 +164,11 @@ class OpenApiTranslator:
             returnValues = OrderedDict()
 
             for rc, rspec in sorted(responses.iteritems()):
+
+                if rc == 'default' or rc.startswith('x-'):
+                    self.warn('default and x-* responses are not implemented')
+                    continue
+
                 schema = None
                 if rspec.get('content', None):
                     schema = rspec['content']
@@ -173,15 +178,26 @@ class OpenApiTranslator:
                             schema = schema['schema']
 
                 if schema is not None:
-                    if schema.get('type') == 'array':
-                        returnValues['sequence of ' + schema['items']['$ref'].rpartition('/')[2]] = True
-                    else:
-                        returnValues[schema['$ref'].rpartition('/')[2]] = True
-
-                if rc == 'default' or rc.startswith('x-'):
-                    self.warn('default responses and x-* responses are not implemented')
-
-            w(u'return {}', ', '.join(returnValues))
+                    varName = ""
+                    if rc.startswith('2'):
+                        varName = "ok <: "
+                    elif rc.startswith('4') or rc.startswith('5'):
+                        varName = "error <: "
+
+                    if varName != "":
+                        if schema.get('type') == 'array':
+                            returnValues[varName + 'sequence of ' + schema['items']['$ref'].rpartition('/')[2]] = True
+                        else:
+                            returnValues[varName + schema['$ref'].rpartition('/')[2]] = True
+
+            if len(returnValues) > 2:
+                self.error('invalid return value set:' + json.dumps(returnValues))
+
+            if len(returnValues) == 0:
+                w(u'return')
+            else:
+                for rv in returnValues:
+                    w(u'return {}', rv)
 
         for (path, api) in sorted(oaSpec['paths'].iteritems()):
             apiParams = api.pop('parameters') if 'parameters' in api else []
diff --git a/src/sysl/importers/import_swagger.py b/src/sysl/importers/import_swagger.py
index 87fe746f..5fdfcb8e 100644
--- a/src/sysl/importers/import_swagger.py
+++ b/src/sysl/importers/import_swagger.py
@@ -234,16 +234,33 @@ class SwaggerTranslator:
 
                         returnValues = OrderedDict()
                         for key in sorted(responses):
+                            if key == 'default' or key.startswith('x-'):
+                                self.warn('default and x-* responses are not implemented')
+                                continue
+
                             schema = responses.get(key, {}).get('schema')
+
                             if schema is not None:
-                                if schema.get('type') == 'array':
-                                    returnValues['sequence of ' + schema['items']['$ref'].rpartition('/')[2]] = True
-                                else:
-                                    returnValues[schema['$ref'].rpartition('/')[2]] = True
-                            if key.startswith('x-'):
-                                self.warn('x-* responses are not implemented')
-
-                        w(u'return {}'.format(', '.join(returnValues)))
+                                varName = ""
+                                if key.startswith('2'):
+                                    varName = "ok <: "
+                                elif key.startswith('4') or key.startswith('5'):
+                                    varName = "error <: "
+
+                                if varName != "":
+                                    if schema.get('type') == 'array':
+                                        returnValues[varName + 'sequence of ' + schema['items']['$ref'].rpartition('/')[2]] = True
+                                    else:
+                                        returnValues[varName + schema['$ref'].rpartition('/')[2]] = True
+
+                        if len(returnValues) > 2:
+                            self.error('invalid return value set:' + json.dumps(returnValues))
+
+                        if len(returnValues) == 0:
+                            w(u'return')
+                        else:
+                            for rv in returnValues:
+                                w(u'return {}', rv)
 
     def writeDefs(self, swag, w):
         w()
diff --git a/test/importers/test_import_openapi.py b/test/importers/test_import_openapi.py
index 73ff9d23..a05b0be1 100644
--- a/test/importers/test_import_openapi.py
+++ b/test/importers/test_import_openapi.py
@@ -25,6 +25,49 @@ def getOutputString(input):
     return str(w), logger
 
 
+def test_importing_simple_openapi_with_error_type():
+    output, _ = getOutputString(r"""
+"openapi": "3.0"
+info:
+  title: Simple
+paths:
+  /test:
+    get:
+      responses:
+        400:
+          description: "client error"
+          content:
+              application/json:
+                schema:
+                  $ref: "#/components/schemas/SimpleObj"
+components:
+  schemas:
+    SimpleObj:
+
+      type: object
+      properties:
+        name:
+          type: string
+""")
+    assert r"""
+ "Simple" [package=""]:
+    @description =:
+        | No description.
+
+    /test:
+        GET:
+            | No description.
+            return error <: SimpleObj
+
+    #---------------------------------------------------------------------------
+    # definitions
+
+    !type SimpleObj:
+        name <: string?:
+            @json_tag = "name"
+""" in output
+
+
 def test_importing_simple_openapi_with_json_tags():
     output, _ = getOutputString(r"""
 "openapi": "3.0"
@@ -57,7 +100,7 @@ components:
     /test:
         GET:
             | No description.
-            return SimpleObj
+            return ok <: SimpleObj
 
     #---------------------------------------------------------------------------
     # definitions
@@ -352,7 +395,7 @@ paths:
         /goat/delete-goat:
             POST ?goat_id=string:
                 | Delete a goat.
-                return Acknowledgement
+                return ok <: Acknowledgement
 
     #---------------------------------------------------------------------------
     # definitions
@@ -439,7 +482,7 @@ paths:
         /goat/get-goats:
             GET:
                 | Gotta get goats.
-                return sequence of Goat
+                return ok <: sequence of Goat
 
     #---------------------------------------------------------------------------
     # definitions
@@ -667,7 +710,7 @@ paths:
           description: 'here be default response'
       summary: Check goat status
 """)
-    expected_warnings = ['default responses and x-* responses are not implemented']
+    expected_warnings = ['default and x-* responses are not implemented']
     assert logger.warnings == expected_warnings
 
 
@@ -692,7 +735,7 @@ paths:
           description: 'here be an x-banana response'
       summary: Check goat status
 """)
-    expected_warnings = ['default responses and x-* responses are not implemented']
+    expected_warnings = ['default and x-* responses are not implemented']
     assert logger.warnings == expected_warnings
 
 
diff --git a/test/importers/test_import_swagger.py b/test/importers/test_import_swagger.py
index 4f1e6aa7..68b89582 100644
--- a/test/importers/test_import_swagger.py
+++ b/test/importers/test_import_swagger.py
@@ -280,7 +280,7 @@ EXAMPLE_SWAGGER_SPEC_WITH_ENDPOINT_PATH_EXPECTED_SYSL = r"""
         /goat/delete-goat:
             POST ?goat_id=string:
                 | Delete a goat.
-                return Acknowledgement
+                return ok <: Acknowledgement
 
     #---------------------------------------------------------------------------
     # definitions
@@ -356,7 +356,7 @@ EXAMPLE_SWAGGER_SPEC_WITH_ENDPOINT_RETURNING_ARRAY_OF_DEFINED_OBJECT_TYPE_EXPECT
         /goat/get-goats:
             GET:
                 | Gotta get goats.
-                return sequence of Goat
+                return ok <: sequence of Goat
 
     #---------------------------------------------------------------------------
     # definitions
@@ -977,6 +977,30 @@ def getOutputString(input):
     return str(w), logger
 
 
+def test_importing_simple_swagger_with_json_tags():
+    output, _ = getOutputString(r"""
+swagger: "2.0"
+info:
+  title: Simple
+paths:
+  /test:
+    get:
+      responses:
+        400:
+          description: client error
+          schema:
+            $ref: '#/definitions/SimpleObj'
+definitions:
+  SimpleObj:
+
+    type: object
+    properties:
+      name:
+        type: string
+""")
+    assert 'return error <: SimpleObj' in output
+
+
 def test_importing_simple_swagger_with_json_tags():
     output, _ = getOutputString(SIMPLE_SWAGGER_EXAMPLE)
     assert 'name <: string?:\\n            @json_tag = "name"' in output
@@ -1072,13 +1096,13 @@ def test_import_of_swagger_path_with_error_response():
 
 def test_import_of_swagger_path_with_default_response_is_not_implemented():
     _, logger = getOutputString(EXAMPLE_SWAGGER_SPEC_WITH_ENDPOINT_PATH_WITH_DEFAULT_RESPONSE)
-    expected_warnings = []
+    expected_warnings = ['default and x-* responses are not implemented']
     assert logger.warnings == expected_warnings
 
 
 def test_import_of_swagger_path_with_x_dash_whatever_response_is_not_implemented():
     _, logger = getOutputString(EXAMPLE_SWAGGER_SPEC_WITH_ENDPOINT_PATH_WITH_X_DASH_WHATEVER_RESPONSE)
-    expected_warnings = ['x-* responses are not implemented']
+    expected_warnings = ['default and x-* responses are not implemented']
     assert logger.warnings == expected_warnings