[Bug 17008] New: REST API: Correct data types in Swagger
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Bug ID: 17008 Summary: REST API: Correct data types in Swagger Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Web services Assignee: koha-bugs@lists.koha-community.org Reporter: larit@student.uef.fi QA Contact: testopia@bugs.koha-community.org Currently some data types are oddly defined in Swagger. For example borrowernumber is defined as a string, even though it and many other *numbers should be integers/numbers. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <larit@student.uef.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |17032 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17032 [Bug 17032] REST API tests: Make sure Swagger object definition is up-to-date with database -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 --- Comment #1 from Lari Taskula <larit@student.uef.fi> --- Created attachment 54486 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54486&action=edit Bug 17008: Test to compare Swagger property types against column data types This patch adds a test that iterates through each definition-object in Swagger and compares their properties' data types to corresponding Koha-object's column's data type. For this to succeed, we need a SQL data type => Swagger data type mapping. See "_sql_to_swagger_data_type_mapping" in definitions.t. SQL data types currently defined in the mapping are extracted with a simple script that iterates through each column from each schema's resultset and storing the data type, so the mapping covers at least all currently defined data types and some manually added ones as well. This test will also check if the column is nullable and will complain unless property has also "null" defined in its type. To test: 1. Run t/db_dependent/api/v1/swagger/definitions.t 2. Observe that tests pass (if not, open a Bug to fix issues) 3. Modify api/v1/definitions/patron.json cardnumber to be type "integer". 4. Run the test again. 5. Observe that test fails and will complain about incorrect data type. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <larit@student.uef.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54486|0 |1 is obsolete| | --- Comment #2 from Lari Taskula <larit@student.uef.fi> --- Created attachment 54502 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54502&action=edit Bug 17008: Test to compare Swagger property types against column data types -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 --- Comment #3 from Lari Taskula <larit@student.uef.fi> --- Created attachment 54508 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54508&action=edit Bug 17008: Add "swaggerize" for Koha::Object and Koha::Objects This patch adds new subroutine "swaggerize" for Koha::Object and Koha::Objects. By matching column data types with corresponding Swagger data type, we can e.g. numify values in order to be able to return borrowernumber:123 instead of borrowernumber:"123". -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <larit@student.uef.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54502|0 |1 is obsolete| | --- Comment #4 from Lari Taskula <larit@student.uef.fi> --- Created attachment 54509 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54509&action=edit Bug 17008: Test to compare Swagger property types against column data types This patch adds a test that iterates through each definition-object in Swagger and compares their properties' data types to corresponding Koha-object's column's data type by using Koha::Util::Swagger::column_type_to_swagger_type(). This test will also check if the column is nullable and will complain unless property has also "null" defined in its type. To test: 1. Run t/db_dependent/api/v1/swagger/definitions.t 2. Observe that tests pass (if not, open a Bug to fix issues) 3. Modify api/v1/definitions/patron.json cardnumber to be type "integer". 4. Run the test again. 5. Observe that test fails and will complain about incorrect data type. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 --- Comment #5 from Lari Taskula <larit@student.uef.fi> --- Created attachment 54510 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54510&action=edit Bug 17008: Fix current data types This patch changes current Swagger definitions for patrons and holds to have data types corresponding to column data types in their database tables. To test: 1. Run t/db_dependent/api/v1/swagger/definitions.t 2. Note that test has a lot of failures. 3. Apply this patch. 4. Run t/db_dependent/api/v1/swagger/definitions.t 5. Note that tests pass. 6. Run REST of the tests in t/db_dependent/api/v1/* -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <larit@student.uef.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <larit@student.uef.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54508|0 |1 is obsolete| | --- Comment #6 from Lari Taskula <larit@student.uef.fi> --- Created attachment 54561 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54561&action=edit Bug 17008: Add "swaggerize" for Koha::Object and Koha::Objects This patch adds new subroutine "swaggerize" for Koha::Object and Koha::Objects. By matching column data types with corresponding Swagger data type, we can e.g. numify values in order to be able to return borrowernumber:123 instead of borrowernumber:"123". Before this patch, we usually returned an unblessed representation of the Koha-object in controller by calling e.g. $c->$cb($patron->unblessed, 200); This would cause the returned JSON to have { "borrowernumber": "123" ... }, or in other words everything as strings (apart from undefs, which would be null). Now, we also return an unblessed representation of the Koha-object, but where numbers are actually numified simply by calling e.g. $c->$cb($patron->swaggerize, 200); Here our returned JSON will have { "borrowernumber": 123 ... } in "integer" data type of Swagger, as well as every other column that is defined to be an integer. Decimals, doubles and floats should have "number" data type and will also be numified for the returned JSON. To test: 1. Apply this patch and run t/db_dependent/Koha/Util/Swagger.t 2. Apply REST of the patches and follow their test plan for seeing how this works in practice -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <larit@student.uef.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54510|0 |1 is obsolete| | --- Comment #7 from Lari Taskula <larit@student.uef.fi> --- Created attachment 54562 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54562&action=edit Bug 17008: Fix current data types This patch changes current Swagger definitions for patrons and holds to have data types corresponding to column data types in their database tables. To test: 1. GET http://yourlibrary/api/v1/patrons/YYY where YYY is existing borrowernumber 2. Observe that numbers / integers are in string data type. 3. Apply this patch. 4. Repeat step 1. 5. Observe that numbers / integers are now actually numbers / integers. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <larit@student.uef.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54509|0 |1 is obsolete| | --- Comment #8 from Lari Taskula <larit@student.uef.fi> --- Created attachment 54563 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54563&action=edit Bug 17008: Test to compare Swagger property types against column data types This patch adds a test that iterates through each definition-object in Swagger and compares their properties' data types to corresponding Koha-object's column's data type by using Koha::Util::Swagger::column_type_to_swagger_type(). This test will also check if the column is nullable and will complain unless property has also "null" defined in its type. To test: 1. Run t/db_dependent/api/v1/swagger/definitions.t 2. Observe that tests pass (if not, open a Bug to fix issues) 3. Modify api/v1/definitions/patron.json cardnumber to be type "integer". 4. Run the test again. 5. Observe that test fails and will complain about incorrect data type. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <larit@student.uef.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |larit@student.uef.fi |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Status|Needs Signoff |Patch doesn't apply --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- CONFLICT (modify/delete): api/v1/definitions/patron.json deleted in 096078a5084dfb5d5fad9ce4e62ecef94140958a and modified in Bug 17008: Fix current data types. Version Bug 17008: Fix current data types of api/v1/definitions/patron.json left in tree. CONFLICT (modify/delete): api/v1/definitions/hold.json deleted in 096078a5084dfb5d5fad9ce4e62ecef94140958a and modified in Bug 17008: Fix current data types. Version Bug 17008: Fix current data types of api/v1/definitions/hold.json left in tree. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54561|0 |1 is obsolete| | --- Comment #10 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 56247 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56247&action=edit Bug 17008: Add "swaggerize" for Koha::Object and Koha::Objects -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54562|0 |1 is obsolete| | --- Comment #11 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 56248 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56248&action=edit Bug 17008: Fix current data types This patch changes current Swagger definitions for patrons and holds to have data types corresponding to column data types in their database tables. To test: 1. GET http://yourlibrary/api/v1/patrons/YYY where YYY is existing borrowernumber 2. Observe that numbers / integers are in string data type. 3. Apply this patch and minify swagger.json perl misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json -d api/v1/swagger/swagger.min.json 4. Repeat step 1. 5. Observe that numbers / integers are now actually numbers / integers. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #54563|0 |1 is obsolete| | --- Comment #12 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 56249 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56249&action=edit Bug 17008: Test to compare Swagger property types against column data types -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #13 from Lari Taskula <lari.taskula@jns.fi> --- Rebased on top of master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56248|0 |1 is obsolete| | --- Comment #14 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 56250 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56250&action=edit Bug 17008: Fix current data types Fixed nullability for "branchcode". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56249|0 |1 is obsolete| | --- Comment #15 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 56251 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56251&action=edit Bug 17008: Test to compare Swagger property types against column data types -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56247|0 |1 is obsolete| | --- Comment #16 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 56279 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56279&action=edit Bug 17008: Add "swaggerize" for Koha::Object and Koha::Objects This patch adds new subroutine "swaggerize" for Koha::Object and Koha::Objects. By matching column data types with corresponding Swagger data type, we can e.g. numify values in order to be able to return borrowernumber:123 instead of borrowernumber:"123". Before this patch, we usually returned an unblessed representation of the Koha-object in controller by calling e.g. $c->$cb($patron->unblessed, 200); This would cause the returned JSON to have { "borrowernumber": "123" ... }, or in other words everything as strings (apart from undefs, which would be null). Now, we also return an unblessed representation of the Koha-object, but where numbers are actually numified simply by calling e.g. $c->$cb($patron->swaggerize, 200); Here our returned JSON will have { "borrowernumber": 123 ... } in "integer" data type of Swagger, as well as every other column that is defined to be an integer. Decimals, doubles and floats should have "number" data type and will also be numified for the returned JSON. To test: 1. Apply this patch and run t/db_dependent/Koha/Util/Swagger.t 2. Apply REST of the patches and follow their test plan for seeing how this works in practice -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56250|0 |1 is obsolete| | --- Comment #17 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 56280 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56280&action=edit Bug 17008: Fix current data types This patch changes current Swagger definitions for patrons and holds to have data types corresponding to column data types in their database tables. To test: 1. GET http://yourlibrary/api/v1/patrons/YYY where YYY is existing borrowernumber 2. Observe that numbers / integers are in string data type. 3. Apply this patch 4. Repeat step 1. 5. Observe that numbers / integers are now actually numbers / integers. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56251|0 |1 is obsolete| | --- Comment #18 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 56281 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56281&action=edit Bug 17008: Test to compare Swagger property types against column data types This patch adds a test that iterates through each definition-object in Swagger and compares their properties' data types to corresponding Koha-object's column's data type by using Koha::Util::Swagger::column_type_to_swagger_type(). This test will also check if the column is nullable and will complain unless property has also "null" defined in its type. To test: 1. Run t/db_dependent/api/v1/swagger/definitions.t 2. Observe that tests pass (if not, open a Bug to fix issues) 3. Modify api/v1/swagger/definitions/patron.json cardnumber to be type "integer". 4. Run the test again. 5. Observe that test fails and will complain about incorrect data type. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 --- Comment #19 from Lari Taskula <lari.taskula@jns.fi> --- Removed minifying step from test plans after Bug 17432. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56279|0 |1 is obsolete| | Attachment #56280|0 |1 is obsolete| | Attachment #56281|0 |1 is obsolete| | --- Comment #20 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 57352 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57352&action=edit [SIGNED-OFF] Bug 17008: Add "swaggerize" for Koha::Object and Koha::Objects This patch adds new subroutine "swaggerize" for Koha::Object and Koha::Objects. By matching column data types with corresponding Swagger data type, we can e.g. numify values in order to be able to return borrowernumber:123 instead of borrowernumber:"123". Before this patch, we usually returned an unblessed representation of the Koha-object in controller by calling e.g. $c->$cb($patron->unblessed, 200); This would cause the returned JSON to have { "borrowernumber": "123" ... }, or in other words everything as strings (apart from undefs, which would be null). Now, we also return an unblessed representation of the Koha-object, but where numbers are actually numified simply by calling e.g. $c->$cb($patron->swaggerize, 200); Here our returned JSON will have { "borrowernumber": 123 ... } in "integer" data type of Swagger, as well as every other column that is defined to be an integer. Decimals, doubles and floats should have "number" data type and will also be numified for the returned JSON. To test: 1. Apply this patch and run t/db_dependent/Koha/Util/Swagger.t 2. Apply REST of the patches and follow their test plan for seeing how this works in practice Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 --- Comment #21 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 57353 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57353&action=edit [SIGNED-OFF] Bug 17008: Fix current data types This patch changes current Swagger definitions for patrons and holds to have data types corresponding to column data types in their database tables. To test: 1. GET http://yourlibrary/api/v1/patrons/YYY where YYY is existing borrowernumber 2. Observe that numbers / integers are in string data type. 3. Apply this patch 4. Repeat step 1. 5. Observe that numbers / integers are now actually numbers / integers. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 --- Comment #22 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 57354 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57354&action=edit [SIGNED-OFF] Bug 17008: Test to compare Swagger property types against column data types This patch adds a test that iterates through each definition-object in Swagger and compares their properties' data types to corresponding Koha-object's column's data type by using Koha::Util::Swagger::column_type_to_swagger_type(). This test will also check if the column is nullable and will complain unless property has also "null" defined in its type. To test: 1. Run t/db_dependent/api/v1/swagger/definitions.t 2. Observe that tests pass (if not, open a Bug to fix issues) 3. Modify api/v1/swagger/definitions/patron.json cardnumber to be type "integer". 4. Run the test again. 5. Observe that test fails and will complain about incorrect data type. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 --- Comment #23 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 57355 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57355&action=edit Bug 17008: followup - fix cityid type -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |tomascohen@gmail.com --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I'm failing this one: - Fixing data types and providing and extension for Koha::Object to handle some inconsistencies are different concerns. And bug 17926 provides a more simple way of doing it. - If you still feel we need to make Koha::Object handle data transformations suitable for output on the REST api, please file a new bug. We still need to openly discuss two things: mistakes in our DB model, and the fact that user facing REST apis don't need to be tied to the underlying model. - Many of the type fixes are correct, but we need to make booleans be booleans, and this patch fails to do so, making them integers. I still think we could make use of the column-to-swagger table, but I'm not sure how to deal with Booleans (which are wrongly mapped to integers currently in most of the cases, as there are only a dozen cases in which they are really integers). I'm open to discuss this on separate bugs that don't catch-them-all. So I'm closing this one, and available to help resurrect the ideas behind this bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17008 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |WONTFIX Status|Failed QA |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org