Bug 15126 - REST API: Use newer version of Swagger2
Summary: REST API: Use newer version of Swagger2
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Galen Charlton
URL:
Keywords: dependency
Depends on: 13799
Blocks: 15165 13903 16212
  Show dependency treegraph
 
Reported: 2015-11-04 14:02 UTC by Julian Maurice
Modified: 2017-12-07 22:17 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15126: Refactor spec file (21.66 KB, patch)
2015-11-04 14:04 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15126: x-mojo-controller deprecation (1.87 KB, patch)
2015-11-04 14:04 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15126: Update dependencies (860 bytes, patch)
2015-11-04 14:04 UTC, Julian Maurice
Details | Diff | Splinter Review
Dependency tree for Swagger2 0.60 (72.77 KB, image/png)
2015-11-04 14:16 UTC, Julian Maurice
Details
Swagger Minifier (3.17 KB, patch)
2016-02-09 02:13 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Swagger Minifier (3.60 KB, patch)
2016-02-09 19:45 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Example of a big swagger-spec split into bite-size chunks (6.63 KB, application/gzip)
2016-02-09 19:47 UTC, Olli-Antti Kivilahti
Details
Big splitted swagger2 spec. Example of the planned usage pattern. (92.87 KB, patch)
2016-02-10 12:53 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Swagger Minifier (7.05 KB, patch)
2016-02-10 12:54 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Newest Swagger-spec validated 100% and pushed to production yay! Includes the swaggerMinifier.pl (15.82 KB, application/gzip)
2016-02-19 10:39 UTC, Olli-Antti Kivilahti
Details
Bug 15126: Refactor spec file (21.66 KB, patch)
2016-04-01 12:21 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15126: x-mojo-controller deprecation (1.87 KB, patch)
2016-04-01 12:21 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15126: Update dependencies (911 bytes, patch)
2016-04-01 12:22 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15126: Refactor spec file (21.73 KB, patch)
2016-04-04 10:46 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15126: x-mojo-controller deprecation (1.94 KB, patch)
2016-04-04 10:46 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15126: Update dependencies (982 bytes, patch)
2016-04-04 10:46 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15126: Update patron definition (1.65 KB, patch)
2016-04-12 09:36 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15126: Refactor spec file (21.79 KB, patch)
2016-04-28 13:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15126: x-mojo-controller deprecation (5.18 KB, patch)
2016-04-28 13:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15126: Update dependencies (1.02 KB, patch)
2016-04-28 13:28 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15126: Update patron definition (1.71 KB, patch)
2016-04-28 13:28 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2015-11-04 14:02:15 UTC
As asked by Tomás on IRC, I will remove patches of bug 13799 that requires a newer version (> 0.31) of Swagger2 module, and put them here.
Comment 1 Julian Maurice 2015-11-04 14:04:48 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2015-11-04 14:04:51 UTC Comment hidden (obsolete)
Comment 3 Julian Maurice 2015-11-04 14:04:54 UTC Comment hidden (obsolete)
Comment 4 Julian Maurice 2015-11-04 14:16:45 UTC
Created attachment 44386 [details]
Dependency tree for Swagger2 0.60
Comment 5 Olli-Antti Kivilahti 2016-02-09 01:24:59 UTC
Hi!

I think your solution looks robust and extendable. I would push the separation of definitions a bit further.
This is a good article about how to split the Swagger2-spec:
    http://azimi.me/2015/07/16/split-swagger-into-smaller-files.html
and it is pretty much the same as you did.

How about we modified your suggestion so the structure looks like this:

.
├── swagger.json
├── info
│   └── index.json
├── definitions
│   └── index.json
│   └── error.json
│   └── patron.json
├── parameters
│   └── index.json
│   └── patron.json
└── paths
    ├── index.json
    └── patrons.json

where definitions/patron.json looks like this:
    "borrower": {
      "type": "object",
      "properties": {
        "borrowernumber": {
          "$ref": "#/definitions/borrowernumber"
        },
        "cardnumber": {
          "$ref": "#/definitions/cardnumber"
        },
        "surname": {
          "$ref": "#/definitions/surname"
        },
        "firstname": {
          "$ref": "#/definitions/firstname"
        },
        ...
      }
    },
    "cardnumber": {
      "description": "Library assigned ID number for Borrowers",
      "type": "string"
    },
    "borrowernumber": {
      "type": "integer",
      "description": "Borrower internal identifier"
    },
    "surname": {
      "description": "borrower's last name",
      "type": "string"
    },
    "surname": {
      "description": "borrower's first name",
      "type": "string"
    }

Now that there is a clear separation of logic (and release from insanely obscure git conflict resolutions with swagger.json), we should enforce that we define all parameters and definitions globally under #/parameters and #/definitions instead of defining them under object/path definitions.
This way we maximize code reuse.
In some cases it might not make much sense (if we only focus on today), like for "surname" and "firstname" in the above example, but some day in the near future somebody might want to reuse those components and then he would have to refactor those changes (or more likely just duplicate code because we are lazy). So as a future-proof solution I feel global definitions are the way to go.

We could even push this a bit further, by instead of defining these REST API with the restrictions of the Koha DB, we could use commonly acknowledged linked data schemas, like schema.org to define our objects.
https://schema.org/Person
This would make it much easier to share the same records, for example between multiple Koha installations. Mainly because of the unique identifier in these objects, like {"url": "https://renki.vaarakirjastot.fi/api/v1/patrons/43441"}
This is actually something which is easily enabled due to our awesome REST API and helps build more linked data systems between various API consumers.
So if somebody would take our Patron-object, it would have a undeniably unique identifier and wouldn't conflict with Patron-objects originating from other systems.

But for now we can focus on splitting our Swagger-spec.
Comment 6 Olli-Antti Kivilahti 2016-02-09 01:38:50 UTC
Also one thing regarding the structure of the api/v1/-directory:

Currently our api/v1/ looks like this:

.
├── hypnotoad.conf
├── hypnotoad.pid
├── app.cgi
├── swagger.json
├── doc
│   └── #Swagger-UI API discovery tool
│
├── auth_examples #REST API authentication code for different languages
│   │             #Discoverable from the Swagger-UI
│   └── ruby_example.rb.txt
│   └── ajax_example.sj.txt
#
#└── swagger  ##Could we add this?##
#    └── #swagger.json and subfolders and files

If you start adding the Swagger-spec linked files and subfolders here. The directory structure will get very confusing. How about we add a directory where to put the "swagger"-specification files?
I understand that the Swagger-UI might not be something which is aimed to be included in the official Koha, but somebody will certainly want to run some real API browsing/discovery service and this is a prefect place to do that. However Swagger-UI works really well and it is improving all the time with a lot of committers in already.
We could name the directory "swagger", or even better "OpenAPI", which is the new name of the Swagger2-specification
    https://github.com/OAI/OpenAPI-Specification
Comment 7 Olli-Antti Kivilahti 2016-02-09 02:13:11 UTC Comment hidden (obsolete)
Comment 8 Olli-Antti Kivilahti 2016-02-09 19:45:49 UTC Comment hidden (obsolete)
Comment 9 Olli-Antti Kivilahti 2016-02-09 19:47:45 UTC Comment hidden (obsolete)
Comment 10 Olli-Antti Kivilahti 2016-02-10 12:53:59 UTC Comment hidden (obsolete)
Comment 11 Olli-Antti Kivilahti 2016-02-10 12:54:46 UTC Comment hidden (obsolete)
Comment 12 Julian Maurice 2016-02-11 08:45:59 UTC
(In reply to Olli-Antti Kivilahti from comment #11)
> Created attachment 47844 [details] [review] [review]
> Swagger Minifier

This patch won't apply as it delete a file which doesn't exist in any other (non-obsoleted) patch. Please reformat your patch.

Also, if "Big splitted swagger2 spec. Example of the planned usage pattern." patch is not to be pushed, please include "[DO NOT PUSH]" in front of the patch title to make it obvious
Comment 13 Olli-Antti Kivilahti 2016-02-11 08:48:01 UTC
Thanks for taking a look!
Comment 14 Olli-Antti Kivilahti 2016-02-11 09:04:04 UTC
What I am curious of is whether or not the spliced swagger2-spec I posted makes sense?
Comment 15 Julian Maurice 2016-02-11 09:17:05 UTC
I haven't looked at it in detail, but having a filesystem structure that match swagger spec structure would certainly make sense
Comment 16 Olli-Antti Kivilahti 2016-02-19 10:39:36 UTC Comment hidden (obsolete)
Comment 17 Julian Maurice 2016-02-29 10:09:51 UTC
I just tested minifySwagger.pl and something bothers me : it resolves all "$ref", meaning the "minified" file will be larger than the sum of all files it comes from if the same "$ref" is used at least twice.

Is it possible resolve only "$ref" that point to different files ? So we keep the idea of a (small) single minified file and let the client resolve the other refs.
Comment 18 Benjamin Rokseth 2016-04-01 11:48:07 UTC
Comment on attachment 44373 [details] [review]
Bug 15126: x-mojo-controller deprecation

>From a1267ff354633ef7f491704f81d9ffe1590672a7 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Wed, 7 Oct 2015 14:49:17 +0000
>Subject: [PATCH] Bug 15126: x-mojo-controller deprecation
>
>Remove the use of soon to be deprecated x-mojo-controller from our
>specification and replace with the recommended operationId format.
>---
> Koha/REST/V1/{Patrons.pm => Patron.pm} | 6 +++---
> api/v1/swagger.json                    | 2 --
> 2 files changed, 3 insertions(+), 5 deletions(-)
> rename Koha/REST/V1/{Patrons.pm => Patron.pm} (96%)
>
>diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patron.pm
>similarity index 96%
>rename from Koha/REST/V1/Patrons.pm
>rename to Koha/REST/V1/Patron.pm
>index 6468bba..fee6c31 100644
>--- a/Koha/REST/V1/Patrons.pm
>+++ b/Koha/REST/V1/Patron.pm
>@@ -1,4 +1,4 @@
>-package Koha::REST::V1::Patrons;
>+package Koha::REST::V1::Patron;
> 
> # This file is part of Koha.
> #
>@@ -22,7 +22,7 @@ use Mojo::Base 'Mojolicious::Controller';
> use C4::Auth qw( haspermission );
> use Koha::Patrons;
> 
>-sub list_patrons {
>+sub list {
>     my ($c, $args, $cb) = @_;
> 
>     my $user = $c->stash('koha.user');
>@@ -35,7 +35,7 @@ sub list_patrons {
>     $c->$cb($patrons->unblessed, 200);
> }
> 
>-sub get_patron {
>+sub get {
>     my ($c, $args, $cb) = @_;
> 
>     my $user = $c->stash('koha.user');
>diff --git a/api/v1/swagger.json b/api/v1/swagger.json
>index 8f32781..e821c44 100644
>--- a/api/v1/swagger.json
>+++ b/api/v1/swagger.json
>@@ -16,7 +16,6 @@
>   "paths": {
>     "/patrons": {
>       "get": {
>-        "x-mojo-controller": "Koha::REST::V1::Patrons",
>         "operationId": "listPatrons",
>         "tags": ["patrons"],
>         "produces": [
>@@ -43,7 +42,6 @@
>     },
>     "/patrons/{borrowernumber}": {
>       "get": {
>-        "x-mojo-controller": "Koha::REST::V1::Patrons",
>         "operationId": "getPatron",
>         "tags": ["patrons"],
>         "parameters": [
>-- 
>1.9.1
Comment 19 Benjamin Rokseth 2016-04-01 12:21:30 UTC Comment hidden (obsolete)
Comment 20 Benjamin Rokseth 2016-04-01 12:21:52 UTC Comment hidden (obsolete)
Comment 21 Benjamin Rokseth 2016-04-01 12:22:02 UTC Comment hidden (obsolete)
Comment 22 Benjamin Rokseth 2016-04-01 12:26:10 UTC
Sorry for the spamming, I rebased the patches against master, since some changes were already pushed to master by bug #13799.
Comment 23 Benjamin Rokseth 2016-04-04 10:46:32 UTC Comment hidden (obsolete)
Comment 24 Benjamin Rokseth 2016-04-04 10:46:38 UTC Comment hidden (obsolete)
Comment 25 Benjamin Rokseth 2016-04-04 10:46:44 UTC Comment hidden (obsolete)
Comment 26 Olli-Antti Kivilahti 2016-04-04 11:10:57 UTC
(In reply to Julian Maurice from comment #17)
> I just tested minifySwagger.pl and something bothers me : it resolves all
> "$ref", meaning the "minified" file will be larger than the sum of all files
> it comes from if the same "$ref" is used at least twice.
> 
> Is it possible resolve only "$ref" that point to different files ? So we
> keep the idea of a (small) single minified file and let the client resolve
> the other refs.

There are some issue with this approach. Firstly, if you want to have reusable reference contents, they probably must reside under the "Definitions Object", and the "Definitions Object" only accepts Schemas/Objects, so we cannot define reusable primitive data types, such as biblionumber.
{ type => 'String',
  description => 'descr'
}

Atleast I didn't figure out how to do it.

Maybe you could add an extra level of nesting under the "Definitions Object"
"x-primitive-definitions": {
  "biblionumber": {
    type => 'Integer',
    description => 'descr'
  }
}

but that might break any automatic Swagger2-tools.

Also the way references are dereferenced is part of the core Mojolicious::Plugin::Swagger2-functionality and I am not so sure how the original author will like that change. I guess it would be nice to have and even doable, but it might be a really difficult algorithm to implement, which makes a complex system even more complex.

The reason I implemented this approach is that external Swagger2-tools (like Swagger2 UI) doesn't resolve those references well, or was it that they dont reference at all (at the point of writing).

I am not concerned about any repetition in the Swagger2-definition in Koha, tho you never know how it might affect us in the future.


Anyway. I am currently working on implementing CORS-support for the Swagger2-API, because we need it now. This certainly is a challenge :)


Also. We moved the
/definitions/index.json
to
/definitions.json

When you reference a definition, you had to write the reference like this:
"$ref": "/definitions/index.json#/borrowernumber"

now you can write the reference more easy:
"$ref": "/definitions.json#/borrowernumber"

Anyway you can find our newest Swagger2-definition from here:
https://github.com/KohaSuomi/kohasuomi/tree/kohasuomi3.16/api/v1/swagger
https://renki.vaarakirjastot.fi/api/v1/doc/


It took us quite some time to refactor to that model + a lot of testing :)
But we are more interested in following the community model. However our API is a product of months of work and continuous self-improvement so I can say with confidence that we are taking the right route.
Comment 27 Olli-Antti Kivilahti 2016-04-04 11:16:24 UTC
(In reply to Benjamin Rokseth from comment #24)
> Created attachment 49846 [details] [review] [review]
> Bug 15126: x-mojo-controller deprecation
> 
> Remove the use of soon to be deprecated x-mojo-controller from our
> specification and replace with the recommended operationId format.
> 
> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Also be advised that the new model of inferring the correct controller-subroutine -pair from the operationId is not without it's problems. Tho if you are willing to accept some clunkiness with the operationId, you can get by without the x-mojo-controller.

One issue I found out is that the operationId "getStory" doesn't translate to Stories::Get, but it translates to something like Storys::Get, which might be rather confusing.

This is not a stopper tho
Just saying, be careful.
use a lot of
export SWAGGER2_DEBUG=1
:)
Comment 28 Julian Maurice 2016-04-05 13:02:19 UTC
Olli, can we move the "Newest Swagger-spec validated 100% and pushed to production yay! Includes the swaggerMinifier.pl" attachment to another bug? This way we can continue the discussion on swagger-minification, while letting the patches here pass into master
Comment 29 Olli-Antti Kivilahti 2016-04-06 10:51:17 UTC
Sure.

Tho I stress that you should replace

definitions/index.json

with

definitions.json
Comment 30 Julian Maurice 2016-04-06 11:05:50 UTC
Comment on attachment 48226 [details]
Newest Swagger-spec validated 100% and pushed to production yay! Includes the swaggerMinifier.pl

Moved to bug 16212
Comment 31 Jonathan Druart 2016-04-12 08:35:05 UTC
Please provide an updated patch.
Comment 32 Julian Maurice 2016-04-12 09:36:51 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2016-04-13 14:19:28 UTC
Did someone try to package Mojolicious v6 and Swagger2 v0.59 for Jessie?
Comment 34 Jonathan Druart 2016-04-13 14:20:35 UTC
(In reply to Olli-Antti Kivilahti from comment #29)
> Sure.
> 
> Tho I stress that you should replace
> 
> definitions/index.json
> 
> with
> 
> definitions.json

What about that suggestion?
Comment 35 Julian Maurice 2016-04-13 14:33:15 UTC
(In reply to Jonathan Druart from comment #34)
> What about that suggestion?

Olli said:
> When you reference a definition, you had to write the reference like this:
> "$ref": "/definitions/index.json#/borrowernumber"
> 
> now you can write the reference more easy:
> "$ref": "/definitions.json#/borrowernumber"

But you can also write (Olli please correct me if I'm wrong)
  "$ref": "/definitions/borrowernumber.json"

So I don't see any benefit in moving index.json
Comment 36 Julian Maurice 2016-04-13 14:40:33 UTC
(In reply to Jonathan Druart from comment #33)
> Did someone try to package Mojolicious v6 and Swagger2 v0.59 for Jessie?

They are both packaged in Debian testing, so it shouldn't be too hard to make a backport for Jessie. But I'll let Galen answer that.
Adding Galen to the CC list.
Comment 37 Tomás Cohen Arazi 2016-04-19 19:16:38 UTC
(In reply to Julian Maurice from comment #36)
> (In reply to Jonathan Druart from comment #33)
> > Did someone try to package Mojolicious v6 and Swagger2 v0.59 for Jessie?
> 
> They are both packaged in Debian testing, so it shouldn't be too hard to
> make a backport for Jessie. But I'll let Galen answer that.
> Adding Galen to the CC list.

Any news on this one?
Comment 38 Jonathan Druart 2016-04-25 16:07:30 UTC
(In reply to Julian Maurice from comment #32)
> Created attachment 50142 [details] [review] [review]
> Bug 15126: Update patron definition

Julian, I think we need tests to make sure the swagger file will be always up-to-date with the DB structure. Any ideas how we could do that?
Comment 39 Julian Maurice 2016-04-26 06:51:23 UTC
(In reply to Jonathan Druart from comment #38)
> (In reply to Julian Maurice from comment #32)
> > Created attachment 50142 [details] [review] [review] [review]
> > Bug 15126: Update patron definition
> 
> Julian, I think we need tests to make sure the swagger file will be always
> up-to-date with the DB structure. Any ideas how we could do that?

An easy solution would be to parse the spec file with Swagger module, and for each path check that the keys are the same that the corresponding DB table.
But what if some paths return more (or less, or different) keys than the corresponding DB table (or if there is no corresponding DB table) ?
A proper solution would be to call each path and check that the keys in the response match with the spec, but it has to be done each time a path is added
Comment 40 Tomás Cohen Arazi 2016-04-26 13:11:33 UTC
(In reply to Jonathan Druart from comment #38)
> (In reply to Julian Maurice from comment #32)
> > Created attachment 50142 [details] [review] [review] [review]
> > Bug 15126: Update patron definition
> 
> Julian, I think we need tests to make sure the swagger file will be always
> up-to-date with the DB structure. Any ideas how we could do that?

Maybe that should be a coding guideline/QA requisite, that every endpoint has a proper set of unit tests, including this check on the swagger files. I'd vote for discussing this on the next dev meeting to gather broader opinions.
Comment 41 Jesse Weaver 2016-04-27 22:43:38 UTC
I verified that most of these changes work correctly in the process of testing bug 13903, so I'd be happy to put my Passed QA stamp on this, but I'm just now coming into this discussion and not sure where things stand. If Galen has a packages for the new Swagger today or tomorrow, are the other concerns on this bug resolved?
Comment 42 Kyle M Hall 2016-04-28 13:26:56 UTC
Created attachment 50916 [details] [review]
Bug 15126: Refactor spec file
Comment 43 Kyle M Hall 2016-04-28 13:27:40 UTC
Created attachment 50917 [details] [review]
Bug 15126: x-mojo-controller deprecation
Comment 44 Kyle M Hall 2016-04-28 13:28:21 UTC
Created attachment 50918 [details] [review]
Bug 15126: Update dependencies
Comment 45 Kyle M Hall 2016-04-28 13:28:45 UTC
Created attachment 50919 [details] [review]
Bug 15126: Update patron definition
Comment 46 Kyle M Hall 2016-04-28 13:30:28 UTC
Still needs input from Package Manager about dependencies.
Comment 47 Galen Charlton 2016-04-28 21:12:05 UTC
I have uploaded to the squeeze-dev/unstable slot the following backported packages:

libswagger2-perl 0.77-1~kohadev+1
libmojolicious-perl 6.15+dfsg-1~kohadev+1
libjson-validator-perl 0.67+dfsg-1~kohadev+1
libio-socket-socks-perl 0.67-1~kohadev+1

Please give it a whirl.
Comment 48 Galen Charlton 2016-04-28 21:30:11 UTC
Data point: seems to work for me on a Jessie box.
Comment 49 Martin Renvoize 2016-04-29 08:18:25 UTC
This is kinda the reason I was always a bit against using the Swagger2 and Mojolicious combination. Don't get me wrong, I love the combination for developeing against, but both are still fast moving projects, especially the swagger2 one which in the grand scheme of things is still itself fairly infant in it's release lifetime.

Since reporting this bug at Swagger 0.59.. we're now at 0.77 and still moving forward fairly quickly.

Still, I don't see any reason not to push this one.. but expect a new one opening up soon for the 0.77 release.. and another for the 0.80 and so on and so on.. ;)

Just to my pence
Comment 50 Kyle M Hall 2016-05-04 13:53:04 UTC
Pushed to master for Koha 16.05, thanks Martin and Julian!
Comment 51 Julian Maurice 2016-05-04 14:29:25 UTC
(In reply to Galen Charlton from comment #47)
> I have uploaded to the squeeze-dev/unstable slot the following backported
> packages:
> 
> libswagger2-perl 0.77-1~kohadev+1
> libmojolicious-perl 6.15+dfsg-1~kohadev+1
> libjson-validator-perl 0.67+dfsg-1~kohadev+1
> libio-socket-socks-perl 0.67-1~kohadev+1
> 
> Please give it a whirl.

I confirm that it works with Swagger 2 0.77 and Mojolicious 6.15. Should we update PerlDependencies.pm ?
libjson-validator-perl was installed automatically as a dependency, but libio-socket-socks-perl was not and it seems not needed. Why is it packaged ?
Comment 52 Galen Charlton 2016-05-04 14:39:34 UTC
(In reply to Julian Maurice from comment #51)
> libio-socket-socks-perl was not and it seems not needed. Why is it packaged ?

It's one of the recommends for libmojolicious-perl and I tossed it in, but it's not required as far as I can tell (and thus doesn't need to be added as a Koha perldep).
Comment 53 Jonathan Druart 2016-08-02 11:47:37 UTC
(In reply to Tomás Cohen Arazi from comment #40)
> (In reply to Jonathan Druart from comment #38)
> > (In reply to Julian Maurice from comment #32)
> > > Created attachment 50142 [details] [review] [review] [review] [review]
> > > Bug 15126: Update patron definition
> > 
> > Julian, I think we need tests to make sure the swagger file will be always
> > up-to-date with the DB structure. Any ideas how we could do that?
> 
> Maybe that should be a coding guideline/QA requisite, that every endpoint
> has a proper set of unit tests, including this check on the swagger files.
> I'd vote for discussing this on the next dev meeting to gather broader
> opinions.

This has never been done and the json file is not up-to-date: checkprevcheckout and updated_on are missing.
Comment 54 Tomás Cohen Arazi 2016-09-23 15:26:47 UTC
(In reply to Jonathan Druart from comment #53)
> (In reply to Tomás Cohen Arazi from comment #40)
> > (In reply to Jonathan Druart from comment #38)
> > > (In reply to Julian Maurice from comment #32)
> > > > Created attachment 50142 [details] [review] [review] [review] [review] [review]
> > > > Bug 15126: Update patron definition
> > > 
> > > Julian, I think we need tests to make sure the swagger file will be always
> > > up-to-date with the DB structure. Any ideas how we could do that?
> > 
> > Maybe that should be a coding guideline/QA requisite, that every endpoint
> > has a proper set of unit tests, including this check on the swagger files.
> > I'd vote for discussing this on the next dev meeting to gather broader
> > opinions.
> 
> This has never been done and the json file is not up-to-date:
> checkprevcheckout and updated_on are missing.

Will you fill a bug? :-D
Comment 55 Jonathan Druart 2016-09-27 07:59:21 UTC
(In reply to Tomás Cohen Arazi from comment #54)
> (In reply to Jonathan Druart from comment #53)
> > (In reply to Tomás Cohen Arazi from comment #40)
> > > (In reply to Jonathan Druart from comment #38)
> > > > (In reply to Julian Maurice from comment #32)
> > > > > Created attachment 50142 [details] [review] [review] [review] [review] [review] [review]
> > > > > Bug 15126: Update patron definition
> > > > 
> > > > Julian, I think we need tests to make sure the swagger file will be always
> > > > up-to-date with the DB structure. Any ideas how we could do that?
> > > 
> > > Maybe that should be a coding guideline/QA requisite, that every endpoint
> > > has a proper set of unit tests, including this check on the swagger files.
> > > I'd vote for discussing this on the next dev meeting to gather broader
> > > opinions.
> > 
> > This has never been done and the json file is not up-to-date:
> > checkprevcheckout and updated_on are missing.
> 
> Will you fill a bug? :-D

See bug 17032, already pushed now.