On Thu, Apr 26, 2012 at 8:32 AM, Friedman, Roger (CDC/CGH/DGHA) (CTR) < [email protected]> wrote:
> Burke --**** > > Regardless of whether it's Darius or me, each of the posts 1 and 3 > would need a t=<type> parameter. **** > > In my design, t is always virtual, it is the classname of the object; > this avoids issues like having to use a global variable or predefined value > or concept map to know the value of order type for drug order. It also > leaves order type available for distinctions that might be important to the > facility without affecting the data model (say a narcotic drug order). I'm > not sure where Darius is at on this; at one point he required a > discriminator field with a known name (registered?), but I think he has > become more virtual. The point is, you can't change the value of a > discriminator field, so why include it in the POST body? I do return it in > the body on GET, as does Darius. > You can't change the concept of an order either, so would that move the URL as well? I don't like the idea of putting some of the properties in the URL and others in the data. You're right that you can't change the type of order; however, you must specify it when creating an order. I'm also not thrilled about clients needing to know the Java class name for every order type. There are not an infinite number of orderables. Most implementations will be able to get by with two up front (drugs & tests). Even for an enterprise-level inpatient system, you can get by with only a dozen or so (e.g., activities, precautions, diet, call orders, drugs, lab tests, radiology tests, procedures, nursing orders, referrals, education/handouts). Actually, each of these basic types could be represented by a concept class and that class name could be used. > **** > > Also, Darius and I differ about privileges and validation at posts 1 > and 3. In Darius' design, both commands would require the privileges to > create an order and would validate per order's requirements. In my design, > post 1 would require the privileges to create a drug order and would > validate per drug order's requirements, while post 3 would require the > privileges to create a referral order and would validate per referral > order's requirements. > There should be a top level privilege for ordering in general. My assumption in posting the orders was that the REST document received would be passed to the order type-specific handler, so permissions & validation could be type-specific as you describe – i.e., we would have separate validators for each basic type of order. > ** > > At get 4, both designs would use the default rep associated with the > subclass of the object. However, suppose you wanted to display all the > types of order with just the order fields and some combination of > subclass-specific fields in a details column. In my design, you could > define a named representation (say, COMPRESSED) that would be defined > differently for different subclasses; you could even define it at a parent > level (say drug order) and it could be used by children (like HIV drug > order). I don't believe you can do this in Darius' design. > Again, I think I agree with you here. I was assuming that the generation of representation for each order would be delegated to it's type-specific handler, so if you asked for a FOOBAR representation of all the patient's orders, the API would iterate through the orders, asking for the representation for each from its type-specific handler. @Darius – is that not what you're doing? i.e., were you creating something that would not defer calls to type-specific handlers? > **** > > With Darius, at step 0 (before the drug order is built in), you have > to create a resource and controller for the order superclass; you have to > create a handler, and a controller if you want custom searches, for drug > order. The superclass resource will contain all the methods of the current > design plus the non-final declaration. > Yes. The idea is that drug orders are provided for you. So "you" will not have to do these steps. > Darius depends on each resource and handler having the variable lists > created by RESTWS-226, which I do not. The drug order handler will contain > its registration plus most of the methods of a resource (no save, delete or > purge). At step 2, you would need to build another handler and possibly a > controller.**** > > With me, at step 0 you have to create a resource and controller for > the hierarchy, a resource and controller for the superclass (order), and a > resource and controller for the subclass (drug order). The controllers are > one-liners unless you want to add custom searches; they are just like the > current design. The hierarchy resource could also be a one-liner, with the > actual delegations taking place in an abstract class using generics. The > resource for the superclass and the subclass would look just like the > current design. At step 2, you would create another controller and > resource for referral order and that would also look just like the current > design. > If you're creating a separate resource for each type, then doesn't that imply a different URL for each order type? > **** > > In our discussion, Darius and I identified some situations (as I > recall relating to validation) where it would be necessary under his design > to override the supplied core resource with a user-defined resource that > extended the core resource. This could not be done independently for more > than one subclass, and would require a custom build. This would not be > necessary with my design. > Can you provide an concrete example? I'm presuming each order type has an associated handler/validator. -Burke > **** > > ** ** > > *From:* [email protected] [mailto:[email protected]] *On Behalf Of *Burke > Mamlin > *Sent:* Wednesday, April 25, 2012 5:08 PM > > *To:* [email protected] > *Subject:* Re: [OPENMRS-DEV] An Alternative Approach to Subclass > Implementation**** > > ** ** > > Ok, so if I want to create a drug order and a referral order for patient > with uuid 123-456, then request all orders for the patient, it would look > something like this (simplifying the order details for a simpler example)? > **** > > ** ** > > *1. Create a drug order for my patient; they're built-in:***** > > POST order**** > > { "patient":"123-456", "type":"drugorder", "order":"penicillin 500 mg > orally twice daily for 2 weeks" }**** > > ** ** > > *2. Install a module to add support for referral orders:***** > > Install a referrals module that adds a referralorder type of order. > Presumably, by introducing a new concept class for referral order, along > with a handler for the new order type and, of course, a ReferralOrder class > that extends Order.**** > > ** ** > > *3. Create a referral order for my patient:***** > > POST order**** > > { "patient":"123-456", "type":"referralorder", "order":"cardiology > referral please evaluate for rheumatic heart disease" }**** > > ** ** > > *4. Find all the active orders for my patient:***** > > GET order?patient=123-456**** > > [**** > > { "patient":"123-456", "type":"drugorder", "order":"penicillin 500 mg > orally twice daily for 2 weeks", "uri":"…" },**** > > { "patient":"123-456", "type":"referralorder", "order":"cardiology > referral please evaluate for rheumatic heart disease", "uri":"…" }**** > > ]**** > > ** ** > > Is that close?**** > > ** ** > > -Burke**** > > ** ** > > On Tue, Apr 24, 2012 at 9:21 PM, Darius Jazayeri <[email protected]> > wrote:**** > > On RESTWS-243 <https://tickets.openmrs.org/browse/RESTWS-243> I've > attached a patch that contains my implementation that I'm now ready to > commit. (It's what Roger was responding to in his email.)**** > > ** ** > > I'm going to hold off on committing until after tomorrow's design call in > case we have time to discuss this a bit.**** > > ** ** > > Basically what I've done is allow resources to indicate that they support > subclasses, and if they do, they'll have all appropriate > DelegatingSubclassHandlers registered with them via spring.**** > > ** ** > > The idea is that we're hiding subclasses from the consumer of the REST > API. As far as they're concerned there is a single "order" resource, which > has documents of different types (e.g. "order", "drugorder", etc.) Behind > the scenes each type is a subclass, but in REST that's not visible, rather > they're exposed as different document types. **** > > ** ** > > DelegatingSubclassHandler encapsulates the methods subclasses need to > specify for their owning resource.**** > > ** ** > > Here is the functionality that my approach supports:**** > > ** ** > > *GET order***** > > - gets all orders regardless of type**** > - each document in the list may have different types**** > - the top-level resource does the db query and delegates to the > appropriate handler to convert each result**** > > *GET order?t=drugorder***** > > - gets all orders whose type is drugorder**** > - the subclass handler does the db query**** > - In my implementation GET order?t=order (i.e. get only things that > are order stubs) is not allowed**** > > *GET order?patient=<uuid>***** > > - gets all orders for the given patient**** > - each document in the list may have different types**** > > *GET order?patient=<uuid>&t=drugorder***** > > - gets all orders for the given patient whose type is drugorder**** > - if the subclass handler defines a suitable method, we use it, > otherwise the top-level resource queries by patient, and manually filters > out by type**** > > *GET order/<uuid>***** > > - returns a specific order**** > - representations are defined by the type, e.g. { "type": "drugorder", > "dose": "100", ... }**** > > *POST order***** > > - creates a new order**** > - you must specify the type in the content, e.g. { "type": > "drugorder", "dose": "100", ... }**** > - allowed properties are defined by the type**** > > *POST order/<uuid>***** > > - modifies an existing order**** > - you are not allowed to change the type**** > - allowed properties are defined by the type**** > > *DELETE order/<uuid>***** > > - voids an order**** > - handled by the top-level resource, not the subclass handler**** > > *DELETE order/<uuid>?purge=true***** > > - purges an order**** > - handled by the top-level resource, not the subclass handler**** > > @Roger, can you please phrase your counterproposal in terms of the > additional functionality that you want to expose beyond these methods?**** > > ** ** > > -Darius**** > > ** ** > > On Mon, Apr 23, 2012 at 9:54 AM, Burke Mamlin <[email protected]> > wrote:**** > > Roger,**** > > ** ** > > Could you provide some concrete examples?**** > > ** ** > > -Burke**** > > ** ** > > On Mon, Apr 23, 2012 at 8:07 AM, Friedman, Roger (CDC/CGH/DGHA) (CTR) < > [email protected]> wrote:**** > > AN ALTERNATIVE APPROACH TO SUBCLASS IMPLEMENTATION**** > > In this approach, subclass resources remain resources, just are mapped via > the superclass. Substantially all operations are delegated to the > appropriate subclass resource. This is motivated by simplicity, validation > and representation considerations. It is also motivated by a privilege > issue I just realized – the people authorized to create/update a lab order > are likely different from those allowed to create/update a drug order. I > have assumed that the privilege of updating a parent class field in > subclass record requires only parent class privileges**** > > I don't believe this alternative requires much in the way of changes to > existing code other than mappings; adding the controller and resource for > each superclass; some changes in asRepresentation where it finds the right > representation to use; and some changes where updates are going on to allow > the possibility that the fields to be looked for in the request body may > come from an object which is a parent class of the object being updated. It > eliminates the need for the two variable lists used by Darius (although > they could still be used if it was thought they added value). I still think > it is a good idea to treat ActiveLists as an abstract class so that the > AllergyList and the ProblemList appear to be two different superclasses.** > ** > > All members of a class hierarchy request a mapping to > /<superclass>?t=<subclass>. The superclass also requests a mapping to > /<superclass>?!t. I have tried to make all the calls of /<superclass>?!t > work the same as calls to /<superclass>?t=<superclass>, if I have missed > please let me know. It might be worthwhile to extend Resource to > SuperclassResource to make the coding of the type-free calls easier. All > representations include a t virtual field which contains the actual > subclass of the object being represented.**** > > 1. GET <superclass>?t=<subclass>&v=<rep>, GET <superclass>?!t**** > > Purpose is to get all records of a particular subclass in a formatted > representation. Spring routes this to the subclass controller. The subclass > resource uses getAll to get a (polymorphic) list of objects. > asRepresentation determines the class of each object and looks for the > requested representation in that subclass resource; if it exists, it is > used; if not, the parent class resource (if any) is searched for a > representation; this continues until the t subclass has been searched, at > which point a rep does not exist error is thrown.**** > > 2. GET <superclass>?t=<subclass>&v=<rep>&q=<search param>, GET > <superclass>?!t&v=<rep>&q=<search param>**** > > Purpose is to use the standard query for a subclass to get a formatted > representation. Spring routes this to the subclass controller. The subclass > resource uses doSearch to get a (polymorphic) list of objects. > asRepresentation determines the class of each object and looks for the > requested representation in that subclass resource; if it exists, it is > used; if not, the parent class resource (if any) is searched for a > representation; this continues until the t subclass has been searched, at > which point a rep does not exist error is thrown.**** > > 3. GET <superclass>?t=<subclass>&v=<rep>&<custom search>=<search param>*** > * > > Purpose is to do a custom search that is defined at some level of the > class hierarchy. Spring routes this to the subclass controller. The > subclass controller creates a resource for its type and delegates the > search to it. The resource uses service methods to do the search to produce > a (polymorphic) list of objects. asRepresentation determines the class of > each object and looks for the requested representation in that subclass > resource; if it exists, it is used; if not, the parent class resource (if > any) is searched for a representation; this continues until the t subclass > has been searched, at which point a rep does not exist error is thrown.*** > * > > 4. GET <superclass>/<uuid>?!t&v=<rep>**** > > Purpose is to get all records of a particular subclass in a formatted > representation. Spring routes this to the superclass controller. The > superclass resource uses getByUuid to get an object. asRepresentation > determines the class of each object and looks for the requested > representation in that subclass resource; if it exists, it is used; if not, > the parent class resource (if any) is searched for a representation; this > continues until the t subclass has been searched, at which point a rep does > not exist error is thrown.**** > > 5. GET <superclass>/<uuid>?t=<subclass>&v=<rep>**** > > Purpose is to use a representation from a parent class. Like 5, except > after getting by Uuid, the subclass resource coerces the result to be its > type. asRepresentation determines the class of each object and looks for > the requested representation in that subclass resource; if it exists, it is > used; if not, the parent class resource (if any) is searched for a > representation; this continues until the supertype has been searched, at > which point a rep does not exist error is thrown.**** > > 6. POST <superclass>?t=<subclass> {body} > > Purpose is to create a new object of type <subclass>. The subclass > resource creates a new object whose fields are extracted from the body (no > need to know. The subclass resource save method uses services method to > save the object; validation takes place in the save method. > > 7. POST <superclass>/<uuid>?!t {body}**** > > Purpose is to update an object of any subclass. The type is determined and > the operation is delegated to the resource of the actual type.**** > > 8. POST <superclass/<uuid>?t=<subclass> {body}**** > > Purpose is to update an object using only parent-level fields. This is to > avoid additional privileges at the actual subclass level. The type is > deermined and the operation is delegated to the resource of the declared > type. Throws an error if the actual type is not a subclass of the type > declared. **** > > 9. DELETE <superclass>/<uuid>?!t&!purge, DELETE > <superclass>/<uuid>?t=<subclass>&!purge**** > > Purpose is to delete an object of any subclass. The type is determined and > the operation is delegated to the resource of the actual type. The > subclass, if present, must match the actual type of the resource.**** > > 10. DELETE <superclass>/<uuid>?purge&!t, DELETE <superclass>/<uuid>?purge* > *** > > Purpose is to purge an object of any subclass. The type is determine and > the operation is delegated to the resource of the actual type. The > subclass, if present, must match the actual type of the resource.**** > ------------------------------ > > Click here to > unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from > OpenMRS Developers' mailing list > **** > > ** ** > ------------------------------ > > Click here to > unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from > OpenMRS Developers' mailing list > **** > > ** ** > ------------------------------ > > Click here to > unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from > OpenMRS Developers' mailing list > **** > > ** ** > ------------------------------ > > Click here to > unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from > OpenMRS Developers' mailing list > **** > _________________________________________ To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [email protected] with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l]
