RESTWS-562 Talk Thread - Adding support for datatypes

restws
Tags: #<Tag:0x00007fbad4930a58>

(Gayan Weerakutti) #1

As mentioned in the issue: RESTWS-562, the data type of a property of a swagger definition is always string which needs improvement.

Properties of a resource are set manually, by just passing a string as the property name. See:

So if we are to pass the corresponding data type of a property, it has to be done manually as well, right? Which means we’d have to update all the resource classes…?

@pascal @bholagabbar


(Rafal Korytkowski) #2

Ideally datatypes of properties would be discovered looking at supportedClass…


(Burke Mamlin) #3

+1 for auto-detecting datatype, even if it is done in a build step instead of real time, to avoid manual maintenance. Anything that is a complicated type could become “Object”.

<aside>Personally, I’d prefer if we built documentation at build time instead of computing it every time the page is loaded. Why should I have to wait or waste precious server cycles when 99% of the time nothing has changed. If/when a new module is added, it can contribute static build files too.</aside>


(Gayan Weerakutti) #4

I’d like to suggest using swagger-core library to facilitate auto-conversion of data types and to convert resource classes to swagger models. swagger-core got handy functions for these type of work.

Currently REST module maintains its own set of model classes. So for example type of a property is represented by DefinitionProperty class which got type as its only property and is not the ideal representation of an actual swagger property.

There are more to it than just a type. Property can have a type, type with format or just $ref. Trying to replicate these would be really cumbersome IMO.

Here I wrote a test case to generate swagger spec using swagger-core (passing Patient.class as a definition). It really simplifies the generation of swagger spec:

Here, Swagger, Model… are model classes provided by the swagger-core.

The above method generates the following spec. Please see the complete swagger.json to see how the definitions were defined.

This would prevent…

  • the need to maintain our own set swagger model classes.
  • having to manually convert data types.
  • having to make changes to the existing swagger model classes.

But before I could begin the implementation, I’d really like hear you guys’ opinion on this.

@raff @burke @bholagabbar @pascal


(Pascal Brandt) #5

Do the serialized objects produced by Swagger match the JSON objects returned and accepted by our API? I think we might do some manual serialization with Representations.


(Darius Jazayeri) #6

@gayanw, note that our REST API does not map exactly to our Java API (and this is intentional), so it’s not enough to just automagically read the Patient object.

For example here it shows a creator property, but actually in our REST API this would be auditInfo.creator.

So, it has to be done “manually” in some way.


(Gayan Weerakutti) #7

@pascal is this what darius said below…? or could you please point me to an example?


(Gayan Weerakutti) #8

@darius Yeah I see the problem there. Thanks for the clarification.

In that case we’ll be adding the properties in each class manually as the way it is being already done. I’ll manage a way to pass other attributes as necessary and not just the property names. I guess it’s okay to use swagger-core as helper library? . Update I’ve already started doing some implementation. I’ll update you with the results once ready


(Darius Jazayeri) #9

FYI a problem you may run into is that the actual behavior of the REST API is poorly documented and a bit inconsistent. E.g. see this: org.openmrs.module.webservices.rest.web.response.ConversionException: patient on class org.openmrs.Encounter


(Gayan Weerakutti) #10

Operations and their responses can have only one type of schema. We can document either full representation or the default and not both. Our existing swagger includes full representations,

So for the schema of a operation (get, post) I’ll be using the default representation. And for their reference properties, I’ll use the ref representation.

    "uuid": "6261cf3c-b921-4db5-b0b8-6686c241948d",
    "display": "Gayan Weerakutti",
    "gender": "M",
    "preferredName": {
        "display": "Gayan Weerakutti",
        "links": [
            {
                "rel": "self",
                "uri": "http://localhost:8080/openmrs/ws/rest/v1/person/6261cf3c-b921-4db5-b0b8-6686c241948d/name/8cb7f7bc-522f-4bc4-8ab2-322e50509a28"
            }
        ],
        "uuid": "8cb7f7bc-522f-4bc4-8ab2-322e50509a28"
    },
    "resourceVersion": "1.11",
    "voided": false
}

So for example for a person resource we’ll be showing the properties of its default representation. And for its reference properties such as `“preferredName”, the reference representation will be used.

In the Swagger definitions, we’ll have different representations, ref, and default for the same resource. This makes sense as one can just look into the model definitions section if they want to know the full representation.


(Gayan Weerakutti) #11

I’m not sure how to find the exact schema of a resource. For GET operations I could make a request, then could check the returned response to find its structure.

But this isn’t option for POST, and POST_UPDATE operations.

Take for example: POST /form/{parent-uuid}/formfield/{uuid}

FormFieldResource1_8.java does not define a getUpdatableProperties method nor its test cases demonstrate how to update the formfiled resource. So if I to define the updatable properties for that resource, how should I approach to find them?


(Gayan Weerakutti) #12

I’ve been working on RESTWS-562. Now for post operations in the swagger doc, full representations will be shown as its model schema. In that case however the example resource would be quite large. See the resource object for creating a Visit: http://jsoneditoronline.org/?id=367e9d5624e93ec563cc32ecae5a99ee

But we can also document with either full representation or default (uuids instead of full objects), depending on the resource if needed. But then I’d need to decide which resources to document with full and which ones to document with default.

What would you recommend?



(Daniel Kayiwa) #13

I thought the full representation really means FULL and hence large. Not shortened versions. Not so?


(Gayan Weerakutti) #14

Yeah right. First pic shows the model version of POST /visit whereas the second pic shows the Example Value of POST /visit.

Both are for the FULL representation.


(Daniel Kayiwa) #15

Then in that case i would not worry about the FULL representation being large.