Wiki Spaces
Documentation
Projects
Resources
Get Help from Others
Q&A: Ask OpenMRS
Discussion: OpenMRS Talk
Real-Time: IRC Chat | Slack
This resource model is proposed for this project. Additional notes:
(1) All requests use the submission mechanism. A new priority level is added for synchronous execution.
(2) The former methods to GET all cohort definitions and dataset definitions have now become appropriately typed GETs to the Definition resource.
(3) The former methods which caused evaluations of cohort definitions and dataset definitions have now become appropriately typed POSTs to the Request resource, followed by GETs once the request has been completed.
(4) All requests and associated data migrate from cache to disk to trash per the mechanism in the reporting module.
(5) Enums are passed as the English text of their values.
(6) Rafal Korytkowski, please check to make sure key/value pairs can be handled in both JSON and XML. See (8) and (9) below.
(7) I'm thinking that rather than being subclass handlers, all 3 definitions and all 3 requests should be first-class resources. They would each contain all the current superclass fields (except Type) plus their own private fields. That way, we could at some later point incorporate their subclasses (e.g. AgeCohortDefinition, EncounterDatasetDefinition, PeriodIndicatorReportDefinition) into the model so they can be CRUDed through the REST interface.
(8) Re key-value pairs and (6) above, I am thinking that DatasetDefintions[] and Datasets[] can be represented as arrays of links, with the "rel" field being the key and the "uri" field being the uri of the resource.
(9) See the new note in the diagram about ColumnNames[]. DataRows[] can be as Darius proposes unless this causes some problem, in which case we should drop the column names and require that all rows have values for all columns. I fear that many datasets will be sparse, resulting in many NULLs, but that may be best for JSON. It is possible that the XML version may differ.
22 Comments
Mike Seaton
I'm confused as to why ReportRequest, CohortRequest, and DatasetRequest all behave differently. I would think that we would want to be able to:
If you wanted to take the Cohort within the EvaluatedCohort and persist this, you could use the core "cohort" resource for this. This is outside the scope of the reporting module.
Note, the reporting module does not currently have a notion of a DatasetRequest or a CohortRequest, but could potentially provide this under the hood by wrapping a DatasetDefinition or a CohortDefinition in a new ReportDefinition, processing it like a ReportRequest, and then parsing the appropriate EvaluatedDataset or EvaluatedCohort back out. But this is an implementation detail...
Roger Friedman
Mike, thanks for your quick response. Please note my "second thoughts" above.
Re your first paragraph, they don't all behave differently. Note that all the methods are at the superclass level, so they all work on all the subclasses. The difference between them lies in their non-common data elements. Re your capabilities, GET xDefinition, Definition UUIDs, POST xRequest, GET xRequest work exactly as you propose. Re DELETE, I didn't want to interfere with your caching/persisting mechanism, but if you want xRequest DELETE?purge, we can provide it easily within the REST framework. Re Request Resources, I have chosen to put the evaluated results into the xRequest resource. Re rendering, note that only ReportRequests have rendering information, and I was hoping reporting relied on RenderMode to indicate whether rendering was desired or not.
Re your second paragraph, I agree. That's the meaning of the note about a new CohortResource POST command.
Re your third paragraph, I was expecting to handle the embedding of cohort and dataset requests in report requests in the getter and setter methods of the data elements in the REST resource. The change I was looking for in Reporting was to add the SYNCHRONOUS Priority, with corresponding changes to the dequeueing process. The reason for wrapping the evaluated data in the request was to avoid having to distinguish between Cohorts and EvaluatedCohortDefinitions.
Sashrika Waidyarathna
Hi Roger and Mike,
I have few questions regarding this design. First, In the comment of the first picture says “GET is to be implemented.” . But in the existing module, it allows to list down the all resource definition(eg: GET /ws/rest/v1/reportingrest/cohortDefinition) . Or you mean something else?
Why don’t we add POST or DELET with ReportDefinition, DatasetDefinition, CohortDefinition? In the existing module it only allows to create ReportDefinition with the POST.
In the existing Module it doesn’t allow to list down all ReportRequest(GET command). I think we should provide that web service to all ReportRequest, CohortRequest and DatasetRequest. As you mentioned in the design.
Roger Friedman
Sashrika --
Perhaps my notation is confusing to you and Mike. What I am trying to show in the picture is 2 occurrences of an abstract class and 3 concrete subclasses, implemented as a Resource and 3 Subclass Handlers. Based on my "second thoughts", I would replace that with 2 occurrences of 3 classes implemented as Resources; the fields and methods of the old abstract class are added to those of each of the 3 old subclass handlers to produce the 3 new Resources. I should redraw the picture but I don't have time right now.
The notes are saying that only GET is implemented for Definitions because at least dataset definitions and cohort definitions rely on serialized objects – they are objects which contain other objects, and the contained objects don't have a uniform structure. Mike is working to rationalize these lower level objects, but until that is done, I don't think it's possible to have a REST interface for them. (In fact, that's why my "second thoughts" got rid of the subclasses; it may be that we need to implement the lower level objects as subclasses of the definitions.) If the lower level objects can't be built through REST, then the higher level objects (CohortDefinitions and DataDefinitions) can't either. You might be able to have a POST for a ReportDefinition, but the CohortDefintion and DatasetDefintions which it contains would have to be created through the API. To me, that didn't seem to add much value, but if you want to implement it, I don't have a problem.
Sashrika Waidyarathna
Oh there is no need to re-draw it Roger.
I understand what you are saying. I hope the structure in the picture is ok. We can have a Resource class and 3 Subclass Handlers as you said.
ReportDefinitionResource, CohortDefinitionResource,DatasetDefinitonResource are extended by BasedefinitionResource class in the existing module. You saying we can apply a same class hierarchy for ReportRequestResource, CohortRequestResource,DatasetRequestResource with a parent class. Am I correct?
I am agreeing with you Roger. Cohort definitions and Dataset definitions don’t have a uniform structure. It will be a more complex process to provide those functionalities.
I don’t understand what is “GET type={type}”. Can you please explain what it does? I also unable to run that command.
Roger Friedman
Yes, you clearly understand what I'm saying, now you have to question it.
Actually, in the case of definitions, I think it is best to have 3 types of resources. That way, we can use subclasses for the different subtypes of definitions within the 3 major types once Mike figures out a better representation for them.
In the case of requests, it may be better to use subclasses for the 3 major types. That way we can have a queue that handles the queueing and dequeueing of Requests, with subclass-specific evaluation calls.
We also need to remember that subclass handlers are the least tested aspect of the REST framework so we shouldn't push them too hard.
Sashrika Waidyarathna
Roger I have another question.
what is the purpose of GET ReportRequest, DatasetRequest, CohortRequest?
Is it for check whether a Report is completed or failed?
Then why do we need a GET DatasetRequest and a GET CohortRequest? Cohort queries and Dataset defintion don't have a status, don't they?
Roger Friedman
Sashrika, the idea is that a request calls for the evaluation of a definition, and, when complete, contains the result, so the GETs are to get the status and, if complete, the returned results. I know that dataset definitions can be evaluated individually and cohort definitions could be as well if they are not already. So the requests should have a status so we don't have to identify complete and incomplete requests by a distinction between empty sets and null objects (a debate which breaks out from time to time). BTW, I believe the current evaluate dataset definition call is synchronous, and I would have no objection during this early time to limiting the priority for dataset and cohort requests to synchronous. The important thing is that an evaluated cohort be different from a cohort because cohorts can have members added and removed individually while evaluated cohorts can only change by re-evaluation.
Lluis Martinez
Roger, I assume Reporting REST will invoke the existing Reporting API to evaluate reports/cohorts/datasets. And that this API already deals with (in)complete requests and returns the appropriate status. Or maybe I'm wrong and we need to improve the Reporting API as well? I hope it's not in the scope of this project.
Roger Friedman
Lluis, I have tried to conform to the existing Reporting object model and API as much as possible given that cohort and dataset definition evaluation requests are being added to report definition evaluation requests (a step made necessary by treating the request queue as the exposed resource, in that manner avoiding the issue of "action" calls and PUT). See the data model. At this point, the only change to the reporting module is the addition of a "synchronous" priority. Initially, I would expect that the ReportingRest module would handle cohort and dataset evaluation requests itself and would only run those synchronously (through the API). However, I would hope that in the longer term, the reporting module's request queue handler would be extended to allow all three types of requests and to handle synchronous execution itself.
Sashrika Waidyarathna
I stole your class diagram, modified it and pasted it on my project proposal
( Reporting REST web services Enhancements - Project Plan ) . I am going to start coding based on the design up to now. I hope to discuss further design problems here, as my implementation keep going.
Sashrika Waidyarathna
Hi Roger..,
I can't understand what is " <Subresource> ReportResource " . I unable to find any class relating to the "ReportResource" in reporting module. Can you please explain what does this mean?
Thank you.
Roger Friedman
Sashrika – Please see ReportDesign.hbm.xml in reporting-api/resources. The class name is ReportDesignResource. Ditto for ReportProcessorConfigurition
Sashrika Waidyarathna
Found it Roger.. Thank you.
I added Contents[] to the ReportResource Representation. It takes more than one minute to display the output. Because Contents[] contain thousands of bytes (it contains a byte array). Does it really useful to the users? f we remove the Contents[] from the representation, it displays the output within few milliseconds.
Lluis Martinez
Probably makes sense only for FULL representations, not DEFAULT ones. But it is still a problem because it can lead to an OutOfMemory exception. I believe report contents should be returned by streaming, but the way it is stored in DB and returned by REST it's hard to solve, and out of GSoC scope of course.
Sashrika Waidyarathna
Yes Lluis.. I added that to the FULL representation. User can wait for a minute if he really needs a full detail on this
Roger Friedman
I agree with the solution. As I understand it, and I could be totally wrong, a report resource is something like a file into which values from the report will be substituted or a BIRT report definition.
Sashrika Waidyarathna
Yes Roger.... It is a file. We can upload even a video file for that
Sashrika Waidyarathna
Roger..,
I can't understand the relationship between "Datasets[]" property and ReportRequest (in second picture). When I am going to evaluate a report in OpenMRS, it only ask me for the Base Cohort. And it also optional (If you have set the cohort definition when you crate a report definition, OpenMRS doesn't ask for an baseCohort when you going to evaluate it). But it doesn't ask for any Datasets[].
ReportRequest class also have no any notion about a Datasets[] attribute.
Am I getting this correctly?
Thank you...!
Roger Friedman
Sashrika, as I read the class definitions, a report request object contains a report definition object which in turn contains mapped data definition objects. After execution, a report object is created linked to the report request and containing a report data object, which in turn contains evaluated datasets. What I did when designing the REST API was to collapse everything between the request and the cohorts/datasets/parameters into the request. See Reporting Definition/Request Data Model.
Mike Seaton, any comments?
Sashrika Waidyarathna
https://github.com/openmrs/openmrs-module-reportingrest/pull/4
pull request made for the work upto now.
GET EvaluatedReport is not implemented yet. GET ReportRequest, DatasetRequest, CohortRequest were implemented, but they don't contain the result of the request. only contain the status of the request.
I am still working on this. your feedbacks are welcome
Sashrika Waidyarathna
More details on http://sashiwaidyarathna.blogspot.com/2013/07/reporting-rest-module-current-status.html