|
Description
Lari Taskula
2016-06-09 13:59:37 UTC
(In reply to Lari Taskula from comment #0) > It's great that Bug 15126 splits Swagger specification over multiple files. > However, the problem of still exists as swagger.json grows larger with each > new path. We should do the same split as Bug 15126 did, but for paths and > parameters. I propose the following structure for paths: > > . > ├── swagger.json > ├── definitions > │ └── index.json > │ └── error.json > │ └── patron.json > ├── parameters > │ └── index.json > │ └── patron.json > └── paths > ├── index.json > └── patrons.json > > Also Swagger should specify the required permissions for each resource. We > can do this by adding "x-koha-permission"-field for each path, and using > this field when checking for the needed permissions. This way required > permissions are well-documented and anyone can instantly find out which > permissions they need. Agree with this one, patching swagger.json for each api update/enhancement is cumbersome and prone for errors. As it seems you have already implemented this, please submit patch and I will review asap. Thanks for the interest! Unfortunately, I am having some issues with this approach. The specifications won't validate through Swagger-UI validator (online.swagger.io/validator/debug?url=url_to_your_swagger.json). I read a little about the issue, and to my understanding the structure I described above does not seem to meet Swagger2 specification. It seems that the Paths Object does not take a $ref, but Path Item Object, which can take a $ref. This means that we would not meet the Swagger2 specification if we attempted something like this in swagger.json:
..
"paths": {
"$ref": "paths/index.json"
},
..
In fact the same error happens when running the current master version (the definitions/index.json in Bug 15126) against online.swagger.io-validator. This is the error message I get with the online.swagger.io-validator:
{"messages":["attribute definitions.$ref is not of type `object`"],"schemaValidationMessages":[{"level":"error","domain":"validation","keyword":"type","message":"instance type (string) does not match any allowed primitive type (allowed: [\"object\"])","schema":{"loadingURI":"http://swagger.io/v2/schema.json#","pointer":"/definitions/schema"},"instance":{"pointer":"/definitions/$ref"}}]}
An option would be to keep the proposed structure for development, and use the minifySwagger.pl-script (Bug 16212) to have a valid Swagger specification in the minified swagger.min.json.
Created attachment 52375 [details] [review] Bug 16699: Split parameters and paths in Swagger 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 :)) Created attachment 52376 [details] [review] Bug 16699: Add borrowernumberQueryParam for reusability The borrowernumber as a query parameter should be defined in parameters.json in order to allow its reusability. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate swagger.min.json in online.swagger.io/validator/debug?url=url_to+ _your_swagger_min_json or your local swagger-api/validator-badge validator 4. Observe that validation passes Created attachment 52377 [details] [review] Bug 16699: Fix mixed-up indentation from 2-4 spaces to 2 spaces These definitions had indentation of 4 spaces, while rest of the specification uses 2 spaces. This patch simply maintains the consistency in indentations and provides no other modifications to code. Created attachment 52378 [details] [review] Bug 16699: Support multiple types in primitive definitions Currently it is not possible to define multiple types for primitive definitions in /definitions/*. If you try to use the following "firstname": { "type": ["string", "null"], "description": "patron's first name" } in definitions.json, online.swagger.io validator will not validate it: {"messages":["attribute definitions.firstname.type is not of type `string`"]} One way to get around this issue is to extend definitions with custom "x-primitives" object, where we will define all reusable primitive definitions. To test: 1. Add the "firstname" example above to definitions.json 2. Run minifySwagger.pl 3. Validate your specification 4. Observe that error with description mentioned above is given 5. Apply patch 6. Repeat step 2 and 3 7. Observe that validation passes Created attachment 52379 [details] [review] Bug 16699: Reference new x-primitives in currently defined objects Since we have defined some basic x-primitives in x-primitives.json, we can now start to reuse them in our currently defined objects. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate your Swagger specifications 4. Observe that validation passes Created attachment 52380 [details] [review] Bug 16699: Move Swagger-related files to api/v1/swagger This patch separates Swagger-specifications and the minifySwagger.pl from other api-files by moving specifications & minifier into api/v1/swagger. The patches above with Bug 16212 is my proposal for splitting the Swagger spec. This passes online.swagger.io/validator validations and passes all current REST tests in master. I think it is essential to split the specification yet also maintain its validity against the swagger.io-validator. I'm open to ideas and criticism :) I will soon provide the patch for defining the required permissions in Swagger paths spec. The second part of this Bug duplicated Bug 14868. Moving the permission definitions in Swagger spec to Bug 14868. Created attachment 52580 [details] [review] Bug 16699: Move Swagger-related files to api/v1/swagger This patch separates Swagger-specifications and the minifySwagger.pl from other api-files by moving specifications & minifier into api/v1/swagger. Created attachment 52581 [details] [review] Bug 16699: Move Swagger-related files to api/v1/swagger This patch separates Swagger-specifications and the minifySwagger.pl from other api-files by moving specifications & minifier into api/v1/swagger. Created attachment 52956 [details] [review] Bug 16699: Split parameters and paths in Swagger 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 Buug 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 Buug 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 :)) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Created attachment 52957 [details] [review] [SIGNED-OFF] Bug 16699: Split parameters and paths in Swagger 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 Buug 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 Buug 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 :)) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Created attachment 52958 [details] [review] [SIGNED-OFF] Bug 16699: Add borrowernumberQueryParam for reusability The borrowernumber as a query parameter should be defined in parameters.json in order to allow its reusability. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate swagger.min.json in online.swagger.io/validator/debug?url=url_to+ _your_swagger_min_json or your local swagger-api/validator-badge validator 4. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Created attachment 52959 [details] [review] [SIGNED-OFF] Bug 16699: Fix mixed-up indentation from 2-4 spaces to 2 spaces These definitions had indentation of 4 spaces, while rest of the specification uses 2 spaces. This patch simply maintains the consistency in indentations and provides no other modifications to code. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Created attachment 52960 [details] [review] [SIGNED-OFF] Bug 16699: Support multiple types in primitive definitions Currently it is not possible to define multiple types for primitive definitions in /definitions/*. If you try to use the following "firstname": { "type": ["string", "null"], "description": "patron's first name" } in definitions.json, online.swagger.io validator will not validate it: {"messages":["attribute definitions.firstname.type is not of type `string`"]} One way to get around this issue is to extend definitions with custom "x-primitives" object, where we will define all reusable primitive definitions. To test: 1. Add the "firstname" example above to definitions.json 2. Run minifySwagger.pl 3. Validate your specification 4. Observe that error with description mentioned above is given 5. Apply patch 6. Repeat step 2 and 3 7. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Created attachment 52961 [details] [review] [SIGNED-OFF] Bug 16699: Reference new x-primitives in currently defined objects Since we have defined some basic x-primitives in x-primitives.json, we can now start to reuse them in our currently defined objects. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate your Swagger specifications 4. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Created attachment 52962 [details] [review] [SIGNED-OFF] Bug 16699: Move Swagger-related files to api/v1/swagger This patch separates Swagger-specifications and the minifySwagger.pl from other api-files by moving specifications & minifier into api/v1/swagger. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Created attachment 52974 [details] [review] [SIGNED-OFF] Bug 16699: Split parameters and paths in Swagger 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 Buug 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 Buug 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 :)) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52975 [details] [review] [SIGNED-OFF] Bug 16699: Add borrowernumberQueryParam for reusability The borrowernumber as a query parameter should be defined in parameters.json in order to allow its reusability. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate swagger.min.json in online.swagger.io/validator/debug?url=url_to+ _your_swagger_min_json or your local swagger-api/validator-badge validator 4. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52976 [details] [review] [SIGNED-OFF] Bug 16699: Fix mixed-up indentation from 2-4 spaces to 2 spaces These definitions had indentation of 4 spaces, while rest of the specification uses 2 spaces. This patch simply maintains the consistency in indentations and provides no other modifications to code. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52978 [details] [review] [SIGNED-OFF] Bug 16699: Support multiple types in primitive definitions Currently it is not possible to define multiple types for primitive definitions in /definitions/*. If you try to use the following "firstname": { "type": ["string", "null"], "description": "patron's first name" } in definitions.json, online.swagger.io validator will not validate it: {"messages":["attribute definitions.firstname.type is not of type `string`"]} One way to get around this issue is to extend definitions with custom "x-primitives" object, where we will define all reusable primitive definitions. To test: 1. Add the "firstname" example above to definitions.json 2. Run minifySwagger.pl 3. Validate your specification 4. Observe that error with description mentioned above is given 5. Apply patch 6. Repeat step 2 and 3 7. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52979 [details] [review] [SIGNED-OFF] Bug 16699: Reference new x-primitives in currently defined objects Since we have defined some basic x-primitives in x-primitives.json, we can now start to reuse them in our currently defined objects. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate your Swagger specifications 4. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52980 [details] [review] [SIGNED-OFF] Bug 16699: Move Swagger-related files to api/v1/swagger This patch separates Swagger-specifications and the minifySwagger.pl from other api-files by moving specifications & minifier into api/v1/swagger. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52986 [details] [review] [SIGNED-OFF] Bug 16699: Split parameters and paths in Swagger 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 Buug 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 Buug 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 :)) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52987 [details] [review] [SIGNED-OFF] Bug 16699: Add borrowernumberQueryParam for reusability The borrowernumber as a query parameter should be defined in parameters.json in order to allow its reusability. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate swagger.min.json in online.swagger.io/validator/debug?url=url_to+ _your_swagger_min_json or your local swagger-api/validator-badge validator 4. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52988 [details] [review] [SIGNED-OFF] Bug 16699: Fix mixed-up indentation from 2-4 spaces to 2 spaces These definitions had indentation of 4 spaces, while rest of the specification uses 2 spaces. This patch simply maintains the consistency in indentations and provides no other modifications to code. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52989 [details] [review] [SIGNED-OFF] Bug 16699: Support multiple types in primitive definitions Currently it is not possible to define multiple types for primitive definitions in /definitions/*. If you try to use the following "firstname": { "type": ["string", "null"], "description": "patron's first name" } in definitions.json, online.swagger.io validator will not validate it: {"messages":["attribute definitions.firstname.type is not of type `string`"]} One way to get around this issue is to extend definitions with custom "x-primitives" object, where we will define all reusable primitive definitions. To test: 1. Add the "firstname" example above to definitions.json 2. Run minifySwagger.pl 3. Validate your specification 4. Observe that error with description mentioned above is given 5. Apply patch 6. Repeat step 2 and 3 7. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52990 [details] [review] [SIGNED-OFF] Bug 16699: Reference new x-primitives in currently defined objects Since we have defined some basic x-primitives in x-primitives.json, we can now start to reuse them in our currently defined objects. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate your Swagger specifications 4. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Created attachment 52991 [details] [review] [SIGNED-OFF] Bug 16699: Move Swagger-related files to api/v1/swagger This patch separates Swagger-specifications and the minifySwagger.pl from other api-files by moving specifications & minifier into api/v1/swagger. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. > Currently it is not possible to define multiple types for primitive definitions > in /definitions/*. If you try to use the following > "firstname": { > "type": ["string", "null"], > "description": "patron's first name" mojo swagger2 validate accepts this, and it works, so what is the problem? Perhaps we should consider online swagger validator faulty? If you remove agree on my objections to bug 16212 and remove the auto minifying I would be happy to sign off. Created attachment 54299 [details] [review] [SIGNED-OFF] Bug 16699: Split parameters and paths in Swagger 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 Buug 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 Buug 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 :)) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Created attachment 54300 [details] [review] [SIGNED-OFF] Bug 16699: Add borrowernumberQueryParam for reusability The borrowernumber as a query parameter should be defined in parameters.json in order to allow its reusability. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate swagger.min.json in online.swagger.io/validator/debug?url=url_to+ _your_swagger_min_json or your local swagger-api/validator-badge validator 4. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Created attachment 54301 [details] [review] [SIGNED-OFF] Bug 16699: Fix mixed-up indentation from 2-4 spaces to 2 spaces These definitions had indentation of 4 spaces, while rest of the specification uses 2 spaces. This patch simply maintains the consistency in indentations and provides no other modifications to code. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Created attachment 54302 [details] [review] [SIGNED-OFF] Bug 16699: Support multiple types in primitive definitions Currently it is not possible to define multiple types for primitive definitions in /definitions/*. If you try to use the following "firstname": { "type": ["string", "null"], "description": "patron's first name" } in definitions.json, online.swagger.io validator will not validate it: {"messages":["attribute definitions.firstname.type is not of type `string`"]} One way to get around this issue is to extend definitions with custom "x-primitives" object, where we will define all reusable primitive definitions. To test: 1. Add the "firstname" example above to definitions.json 2. Run minifySwagger.pl 3. Validate your specification 4. Observe that error with description mentioned above is given 5. Apply patch 6. Repeat step 2 and 3 7. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Created attachment 54303 [details] [review] [SIGNED-OFF] Bug 16699: Reference new x-primitives in currently defined objects Since we have defined some basic x-primitives in x-primitives.json, we can now start to reuse them in our currently defined objects. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate your Swagger specifications 4. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Created attachment 54304 [details] [review] [SIGNED-OFF] Bug 16699: Move Swagger-related files to api/v1/swagger This patch separates Swagger-specifications and the minifySwagger.pl from other api-files by moving specifications & minifier into api/v1/swagger. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Rebased against 16271 which is now in master. Removed patched t/api/v1/minifier.t which should be moved to bug #17102 Please test and get this moved further, as it is a pain to rebase swagger definitions as is today ... Created attachment 54311 [details] [review] Bug 16699: Remove requirement from borrowernumberQueryParam borrowernumberQueryParam shouldn't be required as also changed in Bug 16271. To test: 1. Don't apply the patch yet, but first minify Swagger and run t/db_dependent/api/v1/holds.t 2. Observe that some tests fail with response code 400 when expecting 200. 3. Apply patch and minify Swagger 4. Run t/db_dependent/api/v1/holds.t 5. Observe that tests pass. (In reply to Benjamin Rokseth from comment #39) > Rebased against 16271 which is now in master. > > Removed patched t/api/v1/minifier.t which should be moved to bug #17102 > > Please test and get this moved further, as it is a pain to rebase swagger > definitions as is today ... Thanks Benjamin! And I agree, it can be surprisingly time consuming! Created attachment 54372 [details] [review] [SIGNED-OFF] Bug 16699: Remove requirement from borrowernumberQueryParam borrowernumberQueryParam shouldn't be required as also changed in Bug 16271. To test: 1. Don't apply the patch yet, but first minify Swagger and run t/db_dependent/api/v1/holds.t 2. Observe that some tests fail with response code 400 when expecting 200. 3. Apply patch and minify Swagger 4. Run t/db_dependent/api/v1/holds.t 5. Observe that tests pass. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Created attachment 54884 [details] [review] Bug 16699: Split parameters and paths in Swagger 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 Buug 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 Buug 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 :)) Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 54885 [details] [review] Bug 16699: Add borrowernumberQueryParam for reusability The borrowernumber as a query parameter should be defined in parameters.json in order to allow its reusability. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate swagger.min.json in online.swagger.io/validator/debug?url=url_to+ _your_swagger_min_json or your local swagger-api/validator-badge validator 4. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 54886 [details] [review] Bug 16699: Fix mixed-up indentation from 2-4 spaces to 2 spaces These definitions had indentation of 4 spaces, while rest of the specification uses 2 spaces. This patch simply maintains the consistency in indentations and provides no other modifications to code. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 54887 [details] [review] Bug 16699: Support multiple types in primitive definitions Currently it is not possible to define multiple types for primitive definitions in /definitions/*. If you try to use the following "firstname": { "type": ["string", "null"], "description": "patron's first name" } in definitions.json, online.swagger.io validator will not validate it: {"messages":["attribute definitions.firstname.type is not of type `string`"]} One way to get around this issue is to extend definitions with custom "x-primitives" object, where we will define all reusable primitive definitions. To test: 1. Add the "firstname" example above to definitions.json 2. Run minifySwagger.pl 3. Validate your specification 4. Observe that error with description mentioned above is given 5. Apply patch 6. Repeat step 2 and 3 7. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 54888 [details] [review] Bug 16699: Reference new x-primitives in currently defined objects Since we have defined some basic x-primitives in x-primitives.json, we can now start to reuse them in our currently defined objects. To test: 1. Apply patch 2. Run minifySwagger.pl 3. Validate your Swagger specifications 4. Observe that validation passes Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 54889 [details] [review] Bug 16699: Move Swagger-related files to api/v1/swagger This patch separates Swagger-specifications and the minifySwagger.pl from other api-files by moving specifications & minifier into api/v1/swagger. Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com> My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 54890 [details] [review] Bug 16699: Remove requirement from borrowernumberQueryParam borrowernumberQueryParam shouldn't be required as also changed in Bug 16271. To test: 1. Don't apply the patch yet, but first minify Swagger and run t/db_dependent/api/v1/holds.t 2. Observe that some tests fail with response code 400 when expecting 200. 3. Apply patch and minify Swagger 4. Run t/db_dependent/api/v1/holds.t 5. Observe that tests pass. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 54891 [details] [review] Bug 16699: (QA followup) Move minified swagger file into the swagger/ dir Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 54892 [details] [review] Bug 16699: (QA followup) Move minified swagger file into the swagger/ dir Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Pushed to master for 16.11, thanks Lari! |