Ticket #57 (closed enhancement: worksforme)

Opened 4 years ago

Last modified 3 years ago

enhance REST services

Reported by: business@… Owned by: j.saito@…
Priority: critical Milestone: 0.5.1
Component: General Version:
Keywords: Cc:
Product: Operating system:
URL: Hardware:

Description

Enhance the REST services as specified on the dbnp osxeu site (under modules). Update the methods according to their specification (including the change from 'externalID' to 'token'), and add the not yet implemented methods getStudy etc. These should give back all the information about domain and template fields.
Probably the most efficient way to do this is create a method in TemplateEntity? which iterates over all domain and template fields and renders their values as JSON.

Also, proper error messages should be returned when something is not found, e.g. no assay is found with the specified code. I think the best way to this is to return a 404 Not Found page with the error message in the body.

Change History

  Changed 4 years ago by business@…

  • component set to GSCF in general

Most of this is done now, are the error messages in place?

  Changed 4 years ago by j.saito@…

Currently working on the Error handling.

Implementing one test as proof of concept. The rest is just coding for about one or two days.

  Changed 4 years ago by business@…

Also, the REST methods need to be stabilized. If something is not there, they shouldn't give an error. For example, when a Sample does not have a material set, currently the getSamples REST call fails with the error:
2010-10-11 17:48:43,782 [TP-Processor11] ERROR errors.GrailsExceptionResolver? - Cannot get property 'name' on null object
java.lang.NullPointerException?: Cannot get property 'name' on null object

at RestController?$_closure5_closure13.doCall(RestController?.groovy:160)
at RestController?$_closure5.doCall(RestController?.groovy:159)
at RestController?$_closure5.doCall(RestController?.groovy)
at net.bull.javamelody.JspWrapper?.invoke(JspWrapper?.java:116)
at net.bull.javamelody.JdbcWrapper?$DelegatingInvocationHandler?.invoke(JdbcWrapper?.java:250)
at $Proxy71.forward(Unknown Source)
at org.apache.shiro.web.servlet.ShiroFilter?.executeChain(ShiroFilter?.java:687)
at org.apache.shiro.web.servlet.ShiroFilter?.doFilterInternal(ShiroFilter?.java:616)
at org.apache.shiro.web.servlet.OncePerRequestFilter?.doFilter(OncePerRequestFilter?.java:81)
at net.bull.javamelody.MonitoringFilter?.doFilter(MonitoringFilter?.java:388)
at org.apache.jk.server.JkCoyoteHandler?.invoke(JkCoyoteHandler?.java:190)
at org.apache.jk.common.HandlerRequest?.invoke(HandlerRequest?.java:291)
at org.apache.jk.common.ChannelSocket?.invoke(ChannelSocket?.java:774)
at org.apache.jk.common.ChannelSocket?.processConnection(ChannelSocket?.java:703)
at org.apache.jk.common.ChannelSocket?$SocketConnection?.runIt(ChannelSocket?.java:896)
at java.lang.Thread.run(Thread.java:619)

  Changed 4 years ago by business@…

  • milestone set to 0.5.1
  • reporter changed from kees.vanbochove@… to business@…

  Changed 4 years ago by j.saito@…

The getSample error is an instance of an unsystematic null pointer exception. This kind of error should generally be handled and I remove it together with two more similar instances.

Besides I have tried to introduce Exception handling for errors in the Communication manager. So far, I only committed exceptions for something going wrong. I have been working on a more fine grained distinction between server side errors and client side errors related to REST. however, these errors require some sort of convention for delivering from the server side to the client side. Therefore, they require a convention to be agreed to by NMCDSP and I have not committed these Exceptions.

Next, I would like to roll the CommunicationManager? and the REST exceptions into a plugin.

  Changed 4 years ago by business@…

We can pretty much set the standard here. So I propose if an invalid request is made (e.g. a study that doesn't exist) we should return a 404 with the error message in the HTML body. See also  http://microformats.org/wiki/rest/urls.

  Changed 4 years ago by business@…

Aside from the exception handling, we today also agreed that the REST methods for Authentication Phase 1 (see  http://www.dbnp.org/modules) should handle security slightly differently. Instead of just supplying username and password with each call, we will just use the session id of the browser sessions as a means to identify the user. In that way, the module doesn't have to handle logins anymore, it can just redirect to GSCF to login (and use the isUser method to check if a user has already logged in to GSCF in the current session). This has also been updated in the documentation, see for the changes: http://dbnp.org/modules/@@history?one=25&two=24.

follow-up: ↓ 9   Changed 3 years ago by business@…

Because now a new requirement from NMC is that the information in the REST call 'getSample' should also be available in bulk, e.g. for all samples in an assay, we have a dilemma. If we extend 'getSample' to also accept multiple identifiers or even the convention 'no identifier gives you all the samples', we will end up with 2 REST methods that give you information about the samples in an assay. And that can be confusing, which is not what we want.

So, please update documentation + code to remove getStudy, getAssay and getSample, and merge their functionality in getStudies, getAssays and getSamples. The signature of getStudy, getAssay and getSample can remain the same.

in reply to: ↑ 8   Changed 3 years ago by business@…

Replying to business@…:
Sorry, that last line should read: The signature of getStudies, getAssays and getSamples can remain the same.

  Changed 3 years ago by business@…

Also, change 'moduleURL' to 'consumer' (which is already done in some places in the code) and update the documentation at dbnp.org/modules accordingly.

  Changed 3 years ago by j.saito@…

@Kees

I'm actually not sure whether this is really a good idea. Please confirm this.

OAuth dinstinguishes between: consumer, consumer key and consumer secret.

If I understand this correctly, the code that has been added as consumer referes to the consumer key. However, the moduleURL referes to something I would desribe as and ID to refer to the actual consumer (i.e., the server), not it's key or secret. Please check this.

In any case, I made the change locally and I can check it in if you confirm.

  Changed 3 years ago by j.saito@…

Hm, I read the documentation again. Actually, consumer probably referes to the consumer key. So the, everything should be okay. I'll check this in.

follow-up: ↓ 14   Changed 3 years ago by m.s.vanvliet@…

GSCF is missing 3 REST Methods (which used to work):

getStudy(studyToken) Provide detailed information about a certain study
getAssay(assayToken) Provide detailed information about a certain assay
getSample(assayToken, sampleToken) Provide detailed information about a certain sample, or all samples in the assay if sampleToken is omitted

in reply to: ↑ 13 ; follow-up: ↓ 15   Changed 3 years ago by j.saito@…

Replying to m.s.vanvliet@…:

GSCF is missing 3 REST Methods (which used to work):

getStudy(studyToken) Provide detailed information about a certain study
getAssay(assayToken) Provide detailed information about a certain assay
getSample(assayToken, sampleToken) Provide detailed information about a certain sample, or all samples in the assay if sampleToken is omitted

Actually, I merged the functionality of these three "missing" REST resources with other REST resources by request in this ticket (see. 5 entries above). I had assumed that the request was made in communication with you! The functionality is documented in the code. Furtherdocumentation on dbnp.dorg will follow within this week.

getStudies now returns all information that would otherwise be returned one by one in getStudy.
Same for getAssays and getSample. Please let me know whether this is okay for you.

in reply to: ↑ 14   Changed 3 years ago by m.s.vanvliet@…

Replying to j.saito@…:

Replying to m.s.vanvliet@…:

GSCF is missing 3 REST Methods (which used to work):

getStudy(studyToken) Provide detailed information about a certain study
getAssay(assayToken) Provide detailed information about a certain assay
getSample(assayToken, sampleToken) Provide detailed information about a certain sample, or all samples in the assay if sampleToken is omitted



Actually, I merged the functionality of these three "missing" REST resources with other REST resources by request in this ticket (see. 5 entries above). I had assumed that the request was made in communication with you! The functionality is documented in the code. Furtherdocumentation on dbnp.dorg will follow within this week.

getStudies now returns all information that would otherwise be returned one by one in getStudy.
Same for getAssays and getSample. Please let me know whether this is okay for you.


Fine with me... Thanks for the update.

follow-up: ↓ 17   Changed 3 years ago by m.s.vanvliet@…

The REST call getSamples(assayToken, sampleToken) does not return the Template data!

in reply to: ↑ 16 ; follow-up: ↓ 18   Changed 3 years ago by j.saito@…

Replying to m.s.vanvliet@…:

The REST call getSamples(assayToken, sampleToken) does not return the Template data!

Hm, i was following the specs that we had made. The query actually gives back a selection of fields from Sample, Event, and Subject.

From which Classes do you need the template fields: Sample, Event, and/or Subject? It's easy to add, so please just let me know.

in reply to: ↑ 17 ; follow-up: ↓ 20   Changed 3 years ago by m.s.vanvliet@…

Replying to j.saito@…:

Replying to m.s.vanvliet@…:

The REST call getSamples(assayToken, sampleToken) does not return the Template data!


Hm, i was following the specs that we had made. The query actually gives back a selection of fields from Sample, Event, and Subject.

From which Classes do you need the template fields: Sample, Event, and/or Subject? It's easy to add, so please just let me know.

For the Pilot I need at least the Sample TemplateFields?... but the Event and Subject would also help,... What would be the alternative way to fetch the Template/Meta? data of the Sample, Event or Subject?

follow-up: ↓ 21   Changed 3 years ago by m.s.vanvliet@…

REST returns 401's again... something changed with the authorization?

in reply to: ↑ 18 ; follow-up: ↓ 22   Changed 3 years ago by j.saito@…

Replying to m.s.vanvliet@…:

Replying to j.saito@…:

Replying to m.s.vanvliet@…:

The REST call getSamples(assayToken, sampleToken) does not return the Template data!


Hm, i was following the specs that we had made. The query actually gives back a selection of fields from Sample, Event, and Subject.

From which Classes do you need the template fields: Sample, Event, and/or Subject? It's easy to add, so please just let me know.


For the Pilot I need at least the Sample TemplateFields?... but the Event and Subject would also help,... What would be the alternative way to fetch the Template/Meta? data of the Sample, Event or Subject?

For the prototype i added just the TemplateFields? for Sample.

However, there is one exception. For the field named 'material', i keep returning just the name instead of pulling the whole object. If you prefer to have the whole object I can add it.

In the long run we have to decide on a suitable convention.

The specs changed. Originally, we had two REST resources per domain object, one compact the other extended. E.g., getSamples (returning only sampleTokens) and getSample (returning the complete or even extended object). Later, the specs changed to merge the compact and extended resources (e.g., getSample was mareged into getSamples). If we now need getSubjects and getEvents with full template fields, then we should make a clean decision on whether or not to go back to the standard with two calls per domain object (e.g., getSamples and getSample). In the SAM module, the template fields for Events and Subjects are not needed.

in reply to: ↑ 19   Changed 3 years ago by j.saito@…

Replying to m.s.vanvliet@…:

REST returns 401's again... something changed with the authorization?

Hm, I have my old version of the authorization and that did not change, I believe. At least it is still working.

in reply to: ↑ 20   Changed 3 years ago by j.saito@…

Replying to j.saito@…:

Replying to m.s.vanvliet@…:

Replying to j.saito@…:

Replying to m.s.vanvliet@…:

The REST call getSamples(assayToken, sampleToken) does not return the Template data!


Hm, i was following the specs that we had made. The query actually gives back a selection of fields from Sample, Event, and Subject.

From which Classes do you need the template fields: Sample, Event, and/or Subject? It's easy to add, so please just let me know.


For the Pilot I need at least the Sample TemplateFields?... but the Event and Subject would also help,... What would be the alternative way to fetch the Template/Meta? data of the Sample, Event or Subject?



For the prototype i added just the TemplateFields? for Sample.

However, there is one exception. For the field named 'material', i keep returning just the name instead of pulling the whole object. If you prefer to have the whole object I can add it.



In the long run we have to decide on a suitable convention.

The specs changed. Originally, we had two REST resources per domain object, one compact the other extended. E.g., getSamples (returning only sampleTokens) and getSample (returning the complete or even extended object). Later, the specs changed to merge the compact and extended resources (e.g., getSample was mareged into getSamples). If we now need getSubjects and getEvents with full template fields, then we should make a clean decision on whether or not to go back to the standard with two calls per domain object (e.g., getSamples and getSample). In the SAM module, the template fields for Events and Subjects are not needed.

I have some additional thoughts.

Events have so far not been supposed to be transfered between modules. Therefore, the domain class does not even have any kind of token (external ID for interchange). Please make sure whether we really need them. It is difficult to make a clear cut in what domain objects should be accessible to the non-GSCF modules. If we allow Events, we need to also start thinking about SamplingEvents? and Event groups. SAM does not need any of the three, but for other modules that might be different than we have had in mind so far.

For the time being, the best place to put event information maybe in the sample, but I don't know whether this suits the needs of NMCDSP. Please let me know.

To get started, I now added the template fields of Subject and Event in getSamples.
I am not sure whether this is where you want the data from. For the prototype this might be acceptable, but maybe you want own REST calls to access the full Subject and Event object.

Here is an example of one list entry returned in a list by the enriched getSamples REST call:

key: startTime -> value: 4 days, 6 hours
key: Sample measured volume -> value: null
key: eventObject -> value: [startTime:367200, duration:0, Sample Protocol:null, Sample volume:4.5]
key: event -> value: Blood extraction
key: subject -> value: 6
key: name -> value: 6_A
key: Remarks -> value: null
key: subjectObject -> value: [Age:30, Run-in-food:null, species:[id:2, name:Homo sapiens, accession:9606, class:dbnp.data.Term, ontology:[id:1, class:Ontology]], Diastolic blood pressure:null, DOB:1965-01-09T23:02:00Z, Heart rate:null, Race:null, Height:1.7541354986068989, Weight:43.15085677657785, name:6, Gender:[id:2, name:Female, parent:[id:1, class:TemplateField], class:dbnp.studycapturing.TemplateFieldListItem], BMI:23.545355342988223, Systolic blood pressure:null, Hip circumference:null, Waist circumference:null]
key: Text on vial -> value: T15.166614127028332
key: material -> value: blood plasma

Maybe we can start discussing from here.

  Changed 3 years ago by business@…

  • status changed from accepted to closed
  • resolution set to worksforme
Note: See TracTickets for help on using tickets.