search

anz-bank / sysl

Sysl (pronounced "sizzle") is a system specification language
https://sysl.io
Apache License 2.0
121 stars 42 forks source link

sysl openapi spec importer behaviour assumes query params of different endpoints with the same name have the same schema, and non-deterministically picks a schema for them both to use #1059

Open anz-rfc opened 3 years ago

anz-rfc commented 3 years ago

Description

sysl openapi spec importer behaviour assumes query params of different endpoints with the same name have the same schema, and non-determinstically picks a schema for them both to use

Steps to Reproduce

The following example is hand-minimised from a real API spec.

  1. Create files specs/poc2.sysl and specs/items.yaml as follows:
$ cat specs/poc2.sysl 
import items.yaml as ItemCatalog

SomeEnterprise :: SomePlatform :: SomeService:
    @package="itembroker"

    DoSomething:
        ItemCatalog <- GET /items
        return ok <: string

$ cat specs/items.yaml 
swagger: '2.0'
info:
  title: Item Catalog
  version: v1
basePath: '/'
schemes:
  - http
paths:
  /items:
    get:
      operationId: Get ALL Items
      consumes:
        - application/json
      produces:
        - application/json
      parameters:
        - name: include
          required: false
          in: query
          type: array
          items:
            $ref: '#/definitions/edible_items'
      responses:
        '200':
          description: payload
          schema:
            $ref: '#/definitions/payload'
  '/items/{itemCode}':
    get:
      operationId: Get Specific Item
      consumes:
        - application/json
      produces:
        - application/json
      parameters:
        - name: itemCode
          required: true
          in: path
          type: string
        - name: include
          required: false
          in: query
          type: array
          items:
            $ref: '#/definitions/topological_items'
      responses:
        '200':
          description: payload
          schema:
            $ref: '#/definitions/payload'
definitions:
  edible_items:
    description: edible item types
    enum:
      - sour
      - sweet
      - cheesy
      - salty
      - spicy
    type: string
  payload:
    title: successful response payload
    type: object
    properties:
      matching_items:
        type: array
        items:
          type: object
          additionalProperties: true
  topological_items:
    description: topological item types
    enum:
      - sphere
      - donut
      - pretzel
      - lobster_pot
    type: string

observe that the Item Catalog API spec defines two endpoints -- each has a query parameter named "include" with a different schema.

  1. generate a sysl model.json a bunch of times using the importer:
$ sysl pb --mode json --root . specs/poc2.sysl --output z0.json
$ sysl pb --mode json --root . specs/poc2.sysl --output z1.json
$ sysl pb --mode json --root . specs/poc2.sysl --output z2.json
$ sysl pb --mode json --root . specs/poc2.sysl --output z3.json
$ sysl pb --mode json --root . specs/poc2.sysl --output z4.json
$ sysl pb --mode json --root . specs/poc2.sysl --output z5.json
  1. see if any outputs have different content (they should all be same if the importer was deterministic) and if so, diff a pair of them:
$ md5sum z*.jzon

eee186b9a8a0c791c532d13eda411951  z0.json
f3b8d71c4b8ad26aca138d3a619bb254  z1.json
17b33e2ff2470749f09c4905ca8d5b56  z2.json
f3b8d71c4b8ad26aca138d3a619bb254  z3.json
f3b8d71c4b8ad26aca138d3a619bb254  z4.json
f3b8d71c4b8ad26aca138d3a619bb254  z5.json
$ diff -U 5 --unified z0.json z2.json 
--- z0.json 2021-03-03 16:43:14.000000000 +1100
+++ z2.json 2021-03-03 16:43:19.000000000 +1100
@@ -242,11 +242,11 @@
          "include"
         ]
        },
        "ref": {
         "path": [
-         "topological_items"
+         "edible_items"
         ]
        }
       },
       "sourceContext": {
        "file": "specs/items.yaml",
@@ -443,6 +443,6 @@
      "col": 34
     }
    }
   }
  }
-}
+}
\ No newline at end of file

update: see comment below, this has a simpler example that is a valid swagger 2.0 document that can reproduce the same defect

Caveat -- issue with above example

according to swagger editor, the items.yaml spec isn't a valid swagger 2.0 document, there are two errors:

Structural error at paths./items.get.parameters.0.items
should NOT have additional properties
additionalProperty: $ref
Jump to line 21

Structural error at paths./items/{itemCode}.get.parameters.1.items
should NOT have additional properties
additionalProperty: $ref
Jump to line 44

Expected behavior

Actual behavior

Your Environment

$ sysl info
Build:
  Version      : v0.254.0
  Git Commit   : a5a64cd82c67781f94fe5dd75b846b6446b2783c
  Date         : 2020-10-25T23:53:41Z
  Go Version   : go1.14.8 darwin/amd64
  OS           : darwin/amd64

note: this problem appears present with sysl-go 0.175.0 and sysl-go 0.180.0, which internally use sysl version 0.258.0

anz-rfc commented 3 years ago

Same problem is reproducible where the input swagger 2 spec is a valid swagger 2 spec according to swagger editor.

here is a cleaned up sample spec that reproduces the issue:

swagger: '2.0'
info:
  title: Item Catalog
  version: v1
basePath: '/'
schemes:
  - http
paths:
  /items:
    get:
      operationId: Get ALL Items
      consumes:
        - application/json
      produces:
        - application/json
      parameters:
        - name: x
          in: query
          required: false
          type: array
          items:
            type: string
      responses:
        '200':
          description: payload
          schema:
            $ref: '#/definitions/payload'
  '/items/{itemCode}':
    get:
      operationId: Get Specific Item
      consumes:
        - application/json
      produces:
        - application/json
      parameters:
        - name: itemCode
          required: true
          in: path
          type: string
        - name: x
          in: query
          required: false
          type: array
          items:
            type: number
      responses:
        '200':
          description: payload
          schema:
            $ref: '#/definitions/payload'
definitions:
  payload:
    title: successful response payload
    type: object
    properties:
      matching_items:
        type: array
        items:
          type: object
          additionalProperties: true

both endpoints have an optional query parameter named x. one endpoint requires that x, if given, be an array of string. the other endpoint requires that x, if given, be an array of number.

when sysl importer is run it produces a model.json that arbitrarily picks one of the schemas and uses them for both endpoints, which is both nondeterministic and defective

$ sysl pb --mode json --root . specs/poc2.sysl --output z0.json
$ sysl pb --mode json --root . specs/poc2.sysl --output z1.json
$ sysl pb --mode json --root . specs/poc2.sysl --output z2.json
$ sysl pb --mode json --root . specs/poc2.sysl --output z3.json
$ sysl pb --mode json --root . specs/poc2.sysl --output z4.json
$ sysl pb --mode json --root . specs/poc2.sysl --output z5.json
$ md5sum z*.json
c8854a1487131eafed1e5fd0c1e336e7  z0.json
6524dbd714b1be7b222618b4c69a2add  z1.json
c8854a1487131eafed1e5fd0c1e336e7  z2.json
c8854a1487131eafed1e5fd0c1e336e7  z3.json
6524dbd714b1be7b222618b4c69a2add  z4.json
c8854a1487131eafed1e5fd0c1e336e7  z5.json
$ diff -U 10 --unified z0.json z1.json 
--- z0.json 2021-03-03 17:09:46.000000000 +1100
+++ z1.json 2021-03-03 17:09:48.000000000 +1100
@@ -277,21 +277,21 @@
        "col": 4
       },
       "end": {
        "line": 30,
        "col": 4
       }
      }
     },
     "x": {
      "sequence": {
-      "primitive": "FLOAT",
+      "primitive": "STRING",
       "sourceContext": {
        "file": "specs/items.yaml",
        "start": {
         "line": 36,
         "col": 4
        },
        "end": {
         "line": 38
        }
       }