From eb4417e39f6cce37bc90fe09a82c4efa458e7d7b Mon Sep 17 00:00:00 2001
From: Lari Taskula <larit@student.uef.fi>
Date: Thu, 9 Jun 2016 16:13:53 +0300
Subject: [PATCH] Bug 16699: Split parameters and paths in Swagger
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Parameters and paths should be split in our Swagger specification, because
otherwise swagger.json would become messy with all the paths and their
further specification in the same file. Also parameters should be split
for the same reason.

Instead of using index.json for definitions, parameters and paths, we define
new files "definitions.json", "parameters.json" and "paths.json" in order to
simplify the references. If we kept using index.json and try to reference
"/definitions/error.json" from "/paths/holds.json", reference would be
"../definitions/index.json#/error" instead of now simplified version,
"../definitions.json#/error".

Here is the proposed structure:

.
├── swagger.json
├── definitions.json
├── paths.json
├── parameters.json
├── definitions
│   └── error.json
│   └── patron.json
├── parameters
│   └── patron.json
├── paths
│   └── patrons.json
├── minifySwagger.pl
└── swagger.min.js

The swagger.json paths, definitions and parameters will look as follows:
...
  "paths": {
    "$ref": "paths.json"
  },
  "definitions": {
    "$ref": "definitions.json"
  },
  "parameters": {
    "$ref": "parameters.json"
  }
...

A problem with splitting specification into multiple files directly from
swagger.json (e.g. "paths": { "$ref": "paths.json" }) is that it is not
following the Swagger specification and an error will be thrown by the
Swagger-UI default validator (online.swagger.io/validator).

To overcome this problem, we use the minifySwagger.pl script from Bug 16212.
This allows the developers to work with the structure introduced in this patch
thus allowing developers to split the specification nicely, and still have a
valid Swagger specification in the minified swagger.min.json.

To test:
-2: Apply the minifier-patch in Bug 16212.
-1: Make sure you can validate your specification with Swagger2 validator at
    online.swagger.io/validator/debug?url=url_to_swaggerjson, or install it
    locally from https://github.com/swagger-api/validator-badge.

1. Don't apply this patch yet, but first validate swagger.json
   with swagger.io-validator (or your local version, if you installed it)
2. Observe that validation errors are given
3. Run minifySwagger.pl
4. Validate swagger.min.json with the validator you used in step 1
5. Observe that validation passes and we overcame the invalid specification
   problem in swagger.min.json
6. Apply this patch
7. Run minifySwagger.pl
8. Repeat step 4
9. Observe that validation passes with new structure
10. Run REST tests at t/db_dependents/api/v1

(11. Study the new structure of our Swagger specifications :))
---
 api/v1/definitions.json       |  14 +++
 api/v1/definitions/index.json |   6 --
 api/v1/parameters.json        |   8 ++
 api/v1/parameters/hold.json   |   9 ++
 api/v1/parameters/patron.json |   9 ++
 api/v1/paths.json             |  14 +++
 api/v1/paths/holds.json       | 181 ++++++++++++++++++++++++++++++++
 api/v1/paths/patrons.json     |  61 +++++++++++
 api/v1/swagger.json           | 236 +-----------------------------------------
 9 files changed, 299 insertions(+), 239 deletions(-)
 create mode 100644 api/v1/definitions.json
 delete mode 100644 api/v1/definitions/index.json
 create mode 100644 api/v1/parameters.json
 create mode 100644 api/v1/parameters/hold.json
 create mode 100644 api/v1/parameters/patron.json
 create mode 100644 api/v1/paths.json
 create mode 100644 api/v1/paths/holds.json
 create mode 100644 api/v1/paths/patrons.json

diff --git a/api/v1/definitions.json b/api/v1/definitions.json
new file mode 100644
index 0000000..e4d7427
--- /dev/null
+++ b/api/v1/definitions.json
@@ -0,0 +1,14 @@
+{
+  "patron": {
+    "$ref": "definitions/patron.json"
+  },
+  "holds": {
+    "$ref": "definitions/holds.json"
+  },
+  "hold": {
+    "$ref": "definitions/hold.json"
+  },
+  "error": {
+    "$ref": "definitions/error.json"
+  }
+}
diff --git a/api/v1/definitions/index.json b/api/v1/definitions/index.json
deleted file mode 100644
index 3f69544..0000000
--- a/api/v1/definitions/index.json
+++ /dev/null
@@ -1,6 +0,0 @@
-{
-    "patron": { "$ref": "patron.json" },
-    "holds": { "$ref": "holds.json" },
-    "hold": { "$ref": "hold.json" },
-    "error": { "$ref": "error.json" }
-}
diff --git a/api/v1/parameters.json b/api/v1/parameters.json
new file mode 100644
index 0000000..9778ac7
--- /dev/null
+++ b/api/v1/parameters.json
@@ -0,0 +1,8 @@
+{
+  "borrowernumberPathParam": {
+    "$ref": "parameters/patron.json#/borrowernumberPathParam"
+  },
+  "holdIdPathParam": {
+    "$ref": "parameters/hold.json#/holdIdPathParam"
+  }
+}
diff --git a/api/v1/parameters/hold.json b/api/v1/parameters/hold.json
new file mode 100644
index 0000000..d8177e1
--- /dev/null
+++ b/api/v1/parameters/hold.json
@@ -0,0 +1,9 @@
+{
+  "holdIdPathParam": {
+    "name": "reserve_id",
+    "in": "path",
+    "description": "Internal hold identifier",
+    "required": true,
+    "type": "integer"
+  }
+}
diff --git a/api/v1/parameters/patron.json b/api/v1/parameters/patron.json
new file mode 100644
index 0000000..285ee04
--- /dev/null
+++ b/api/v1/parameters/patron.json
@@ -0,0 +1,9 @@
+{
+  "borrowernumberPathParam": {
+    "name": "borrowernumber",
+    "in": "path",
+    "description": "Internal patron identifier",
+    "required": true,
+    "type": "integer"
+  }
+}
diff --git a/api/v1/paths.json b/api/v1/paths.json
new file mode 100644
index 0000000..f00b1b4
--- /dev/null
+++ b/api/v1/paths.json
@@ -0,0 +1,14 @@
+{
+  "/holds": {
+    "$ref": "paths/holds.json#/~1holds"
+  },
+  "/holds/{reserve_id}": {
+    "$ref": "paths/holds.json#/~1holds~1{reserve_id}"
+  },
+  "/patrons": {
+    "$ref": "paths/patrons.json#/~1patrons"
+  },
+  "/patrons/{borrowernumber}": {
+    "$ref": "paths/patrons.json#/~1patrons~1{borrowernumber}"
+  }
+}
diff --git a/api/v1/paths/holds.json b/api/v1/paths/holds.json
new file mode 100644
index 0000000..06c9af5
--- /dev/null
+++ b/api/v1/paths/holds.json
@@ -0,0 +1,181 @@
+{
+  "/holds": {
+    "get": {
+      "operationId": "listHolds",
+      "tags": ["borrowers", "holds"],
+      "parameters": [{
+          "name": "borrowernumber",
+          "in": "query",
+          "description": "Internal borrower identifier",
+          "required": true,
+          "type": "integer"
+        }
+      ],
+      "produces": ["application/json"],
+      "responses": {
+        "200": {
+          "description": "A list of holds",
+          "schema": {
+            "$ref": "../definitions.json#/holds"
+          }
+        },
+        "404": {
+          "description": "Borrower not found",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        }
+      }
+    },
+    "post": {
+      "operationId": "addHold",
+      "tags": ["borrowers", "holds"],
+      "parameters": [{
+          "name": "body",
+          "in": "body",
+          "description": "A JSON object containing informations about the new hold",
+          "required": true,
+          "schema": {
+            "type": "object",
+            "properties": {
+              "borrowernumber": {
+                "description": "Borrower internal identifier",
+                "type": "integer"
+              },
+              "biblionumber": {
+                "description": "Biblio internal identifier",
+                "type": "integer"
+              },
+              "itemnumber": {
+                "description": "Item internal identifier",
+                "type": "integer"
+              },
+              "branchcode": {
+                "description": "Pickup location",
+                "type": "string"
+              },
+              "expirationdate": {
+                "description": "Hold end date",
+                "type": "string",
+                "format": "date"
+              }
+            }
+          }
+        }
+      ],
+      "consumes": ["application/json"],
+      "produces": ["application/json"],
+      "responses": {
+        "201": {
+          "description": "Created hold",
+          "schema": {
+            "$ref": "../definitions.json#/hold"
+          }
+        },
+        "400": {
+          "description": "Missing or wrong parameters",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "403": {
+          "description": "Hold not allowed",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "404": {
+          "description": "Borrower not found",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "500": {
+          "description": "Internal error",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        }
+      }
+    }
+  },
+  "/holds/{reserve_id}": {
+    "put": {
+      "operationId": "editHold",
+      "tags": ["holds"],
+      "parameters": [{
+          "$ref": "../parameters.json#/holdIdPathParam"
+        }, {
+          "name": "body",
+          "in": "body",
+          "description": "A JSON object containing fields to modify",
+          "required": true,
+          "schema": {
+            "type": "object",
+            "properties": {
+              "priority": {
+                "description": "Position in waiting queue",
+                "type": "integer",
+                "minimum": 1
+              },
+              "branchcode": {
+                "description": "Pickup location",
+                "type": "string"
+              },
+              "suspend_until": {
+                "description": "Suspend until",
+                "type": "string",
+                "format": "date"
+              }
+            }
+          }
+        }
+      ],
+      "consumes": ["application/json"],
+      "produces": ["application/json"],
+      "responses": {
+        "200": {
+          "description": "Updated hold",
+          "schema": {
+            "$ref": "../definitions.json#/hold"
+          }
+        },
+        "400": {
+          "description": "Missing or wrong parameters",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "404": {
+          "description": "Hold not found",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        }
+      }
+    },
+    "delete": {
+      "operationId": "deleteHold",
+      "tags": ["holds"],
+      "parameters": [{
+          "$ref": "../parameters.json#/holdIdPathParam"
+        }
+      ],
+      "produces": ["application/json"],
+      "responses": {
+        "200": {
+          "description": "Successful deletion",
+          "schema": {
+            "type": "object"
+          }
+        },
+        "404": {
+          "description": "Hold not found",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/api/v1/paths/patrons.json b/api/v1/paths/patrons.json
new file mode 100644
index 0000000..f7c6400
--- /dev/null
+++ b/api/v1/paths/patrons.json
@@ -0,0 +1,61 @@
+{
+  "/patrons": {
+    "get": {
+      "operationId": "listPatrons",
+      "tags": ["patrons"],
+      "produces": [
+          "application/json"
+      ],
+      "responses": {
+        "200": {
+          "description": "A list of patrons",
+          "schema": {
+            "type": "array",
+            "items": {
+              "$ref": "../definitions.json#/patron"
+            }
+          }
+        },
+        "403": {
+          "description": "Access forbidden",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        }
+      }
+    }
+  },
+  "/patrons/{borrowernumber}": {
+    "get": {
+      "operationId": "getPatron",
+      "tags": ["patrons"],
+      "parameters": [{
+          "$ref": "../parameters.json#/borrowernumberPathParam"
+        }
+      ],
+      "produces": [
+          "application/json"
+      ],
+      "responses": {
+        "200": {
+          "description": "A patron",
+          "schema": {
+            "$ref": "../definitions.json#/patron"
+          }
+        },
+        "403": {
+          "description": "Access forbidden",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "404": {
+          "description": "Patron not found",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/api/v1/swagger.json b/api/v1/swagger.json
index 1488aba..362780a 100644
--- a/api/v1/swagger.json
+++ b/api/v1/swagger.json
@@ -14,242 +14,12 @@
   },
   "basePath": "/api/v1",
   "paths": {
-    "/patrons": {
-      "get": {
-        "operationId": "listPatrons",
-        "tags": ["patrons"],
-        "produces": [
-          "application/json"
-        ],
-        "responses": {
-          "200": {
-            "description": "A list of patrons",
-            "schema": {
-              "type": "array",
-              "items": {
-                "$ref": "#/definitions/patron"
-              }
-            }
-          },
-          "403": {
-            "description": "Access forbidden",
-            "schema": {
-              "$ref": "#/definitions/error"
-            }
-          }
-        }
-      }
-    },
-    "/patrons/{borrowernumber}": {
-      "get": {
-        "operationId": "getPatron",
-        "tags": ["patrons"],
-        "parameters": [
-          {
-            "$ref": "#/parameters/borrowernumberPathParam"
-          }
-        ],
-        "produces": [
-          "application/json"
-        ],
-        "responses": {
-          "200": {
-            "description": "A patron",
-            "schema": {
-              "$ref": "#/definitions/patron"
-            }
-          },
-          "403": {
-            "description": "Access forbidden",
-            "schema": {
-              "$ref": "#/definitions/error"
-            }
-          },
-          "404": {
-            "description": "Patron not found",
-            "schema": {
-              "$ref": "#/definitions/error"
-            }
-          }
-        }
-      }
-    },
-    "/holds": {
-      "get": {
-        "operationId": "listHolds",
-        "tags": ["borrowers", "holds"],
-        "parameters": [
-          {
-            "name": "borrowernumber",
-            "in": "query",
-            "description": "Internal borrower identifier",
-            "required": true,
-            "type": "integer"
-          }
-        ],
-        "produces": ["application/json"],
-        "responses": {
-          "200": {
-            "description": "A list of holds",
-            "schema": { "$ref": "#/definitions/holds" }
-          },
-          "404": {
-            "description": "Borrower not found",
-            "schema": { "$ref": "#/definitions/error" }
-          }
-        }
-      },
-      "post": {
-        "operationId": "addHold",
-        "tags": ["borrowers", "holds"],
-        "parameters": [
-          {
-            "name": "body",
-            "in": "body",
-            "description": "A JSON object containing informations about the new hold",
-            "required": true,
-            "schema": {
-              "type": "object",
-              "properties": {
-                "borrowernumber": {
-                  "description": "Borrower internal identifier",
-                  "type": "integer"
-                },
-                "biblionumber": {
-                  "description": "Biblio internal identifier",
-                  "type": "integer"
-                },
-                "itemnumber": {
-                  "description": "Item internal identifier",
-                  "type": "integer"
-                },
-                "branchcode": {
-                  "description": "Pickup location",
-                  "type": "string"
-                },
-                "expirationdate": {
-                  "description": "Hold end date",
-                  "type": "string",
-                  "format": "date"
-                }
-              }
-            }
-          }
-        ],
-        "consumes": ["application/json"],
-        "produces": ["application/json"],
-        "responses": {
-          "201": {
-            "description": "Created hold",
-            "schema": { "$ref": "#/definitions/hold" }
-          },
-          "400": {
-            "description": "Missing or wrong parameters",
-            "schema": { "$ref": "#/definitions/error" }
-          },
-          "403": {
-            "description": "Hold not allowed",
-            "schema": { "$ref": "#/definitions/error" }
-          },
-          "404": {
-            "description": "Borrower not found",
-            "schema": { "$ref": "#/definitions/error" }
-          },
-          "500": {
-            "description": "Internal error",
-            "schema": { "$ref": "#/definitions/error" }
-          }
-        }
-      }
-    },
-    "/holds/{reserve_id}": {
-      "put": {
-        "operationId": "editHold",
-        "tags": ["holds"],
-        "parameters": [
-          { "$ref": "#/parameters/holdIdPathParam" },
-          {
-            "name": "body",
-            "in": "body",
-            "description": "A JSON object containing fields to modify",
-            "required": true,
-            "schema": {
-              "type": "object",
-              "properties": {
-                "priority": {
-                  "description": "Position in waiting queue",
-                  "type": "integer",
-                  "minimum": 1
-                },
-                "branchcode": {
-                  "description": "Pickup location",
-                  "type": "string"
-                },
-                "suspend_until": {
-                  "description": "Suspend until",
-                  "type": "string",
-                  "format": "date"
-                }
-              }
-            }
-          }
-        ],
-        "consumes": ["application/json"],
-        "produces": ["application/json"],
-        "responses": {
-          "200": {
-            "description": "Updated hold",
-            "schema": { "$ref": "#/definitions/hold" }
-          },
-          "400": {
-            "description": "Missing or wrong parameters",
-            "schema": { "$ref": "#/definitions/error" }
-          },
-          "404": {
-            "description": "Hold not found",
-            "schema": { "$ref": "#/definitions/error" }
-          }
-        }
-      },
-      "delete": {
-        "operationId": "deleteHold",
-        "tags": ["holds"],
-        "parameters": [
-          { "$ref": "#/parameters/holdIdPathParam" }
-        ],
-        "produces": ["application/json"],
-        "responses": {
-          "200": {
-            "description": "Successful deletion",
-            "schema": {
-              "type": "object"
-            }
-          },
-          "404": {
-            "description": "Hold not found",
-            "schema": { "$ref": "#/definitions/error" }
-          }
-        }
-      }
-    }
+    "$ref": "paths.json"
   },
   "definitions": {
-    "$ref": "./definitions/index.json"
+    "$ref": "definitions.json"
   },
   "parameters": {
-    "borrowernumberPathParam": {
-      "name": "borrowernumber",
-      "in": "path",
-      "description": "Internal patron identifier",
-      "required": true,
-      "type": "integer"
-    },
-    "holdIdPathParam": {
-      "name": "reserve_id",
-      "in": "path",
-      "description": "Internal hold identifier",
-      "required": true,
-      "type": "integer"
-    }
+    "$ref": "parameters.json"
   }
 }
-- 
1.9.1