>From a code-writing perspective, it actually *is* important that it's a subclass. Particularly, this approach depends on the fact that we have an OrderService.getOrders(...) that returns a list of different kinds (subclasses) of orders.
I agree that we should not mention "subclass" anywhere in the externally-facing WS docs (and that's why I prefer this approach to the subclass-links-to-superclass approach: this way give us "different kinds of orders"). But I feel like it's important for the developer writing the code behind the scenes to know that subclasses are involved. Maybe this is just a javadoc issue. -Darius On Thu, Apr 19, 2012 at 11:20 AM, Burke Mamlin <[email protected]>wrote: > Can we remove "subclass" from any of this? That term is not intuitive and > has no clinical meaning – e.g., instead use something like > registerOrderType or registerOrderClass, depending on how we decide to > decide to model different types of orders. If we follow the path laid out > during the design call and start out just supporting different types as > orderables using concept classes, then I'd favor registerOrderClass or > registerOrderableClass. > > -Burke > > On Thu, Apr 19, 2012 at 1:29 PM, Darius Jazayeri > <[email protected]>wrote: > >> I just replied on the ticket... >> >> @Roger, I really really don't like the idea of ignoring extra properties. >> Trying to update a resource with a property it doesn't support should _not_ >> fail silently, it should give you a BAD REQUEST. >> >> From my proposal above, I think we _should_ include a discriminator >> property when you're doing a POST to create, e.g: >> {code} >> { >> type: "org.openmrs.module.lab.LabModuleOrder", >> startDate: "2011-03-04", >> labModuleSpecimen: "<uuid>" >> } >> ...will be delegated to the registered handler for LabModuleOrder, rather >> than being handled directly by OrderResource. >> {code} >> >> The one revision I'd make to what I said before is that we should hide >> the fully-qualified classnames from the end-user of REST. The module should >> do something like >> {code} >> registerSubclass(OrderResource.class, "LabModuleOrder", new >> LabModuleOrderSubclassHandler()); >> {code} >> >> -Darius >> >> On Thu, Apr 19, 2012 at 10:15 AM, Friedman, Roger (CDC/CGH/DGHA) (CTR) < >> [email protected]> wrote: >> >>> That's why I'd like to see subclasses appear as "flat"; you'd still >>> have to update up the tree, but you could use the same body and each layer >>> would pick off the variables it needs. As for create, Hibernate would take >>> care of the splitting and only the subclass would be exposed, so there's no >>> need for multiple updates. But this means that there would be no errors >>> for specifying "extra" variables.**** >>> >>> ** ** >>> >>> *From:* [email protected] [mailto:[email protected]] *On Behalf Of *Darius >>> Jazayeri >>> *Sent:* Thursday, April 19, 2012 1:01 PM >>> >>> *To:* [email protected] >>> *Subject:* Re: [OPENMRS-DEV] REST WS and subclasses**** >>> >>> ** ** >>> >>> Burke, if we incorporate Jeff Wishnie's off-list suggestion to us about >>> making the "Subclass links to Superclass" also include a link from the >>> superclass to the subclass, we could do something like this:**** >>> >>> ** ** >>> >>> GET order ->**** >>> >>> [**** >>> >>> {**** >>> >>> // order properties here**** >>> >>> links: [**** >>> >>> { rel: "self": uri: "order/<uuid>" },**** >>> >>> { rel: "more-details": uri: "drugorder/<uuid>" }**** >>> >>> ]**** >>> >>> },**** >>> >>> {**** >>> >>> // order properties here**** >>> >>> links: [**** >>> >>> { rel: "self": uri: "order/<uuid2>" },**** >>> >>> { rel: "more-details": uri: "laborder/<uuid2>" }**** >>> >>> ]**** >>> >>> }**** >>> >>> ]**** >>> >>> ** ** >>> >>> GET drugorder ->**** >>> >>> {**** >>> >>> // only DrugOrder-specific properties here**** >>> >>> order: { ref representation of the superclass },**** >>> >>> links: [**** >>> >>> { rel: "self": uri: "drugorder/<uuid>" }**** >>> >>> ]**** >>> >>> }**** >>> >>> ** ** >>> >>> Personally I prefer my originally-proposed approach #3 instead of this, >>> though. It seems like it would be easier for clients to put all a DrugOrder >>> properties (own and inherited) in a single document, rather than having >>> dosage in one document and startDate and concept in a linked document. As I >>> phrased it before, it makes sense to me that if subclasses "inherit the >>> *meaning* of the superclass, not just its implementation details" then >>> it makes sense to expose them all through a single resource...**** >>> >>> ** ** >>> >>> (But I'm no REST expert, so...)**** >>> >>> ** ** >>> >>> -Darius**** >>> >>> ** ** >>> >>> ** ** >>> >>> On Thu, Apr 19, 2012 at 9:29 AM, Burke Mamlin <[email protected]> >>> wrote:**** >>> >>> Orders are often approached generically (e.g. getting a list of active >>> orders; creating order sets that contain meds, tests, educational >>> materials, referrals, etc.; etc.), all orders share a base of properties & >>> workflows, and we want to allow for modules to add additional types. By >>> treating all orders types as "orders", we allow consumers to treat >>> different types differently when they know about them, but also fall back >>> to treating them as generic orders for those types they don't understand. >>> **** >>> >>> ** ** >>> >>> For example, if I want to generate a list of all active orders, I can go >>> to one place, get this list, and show the names of them, who ordered them, >>> etc. rather than fetching the active list from each resource (drugorder, >>> laborder, radiologyorder, nursingorder, referralorder, dietorder, >>> activityorder, precautionorder, and callorder) and then be forced to >>> refactor my code when I want to include Roger's latest whirlygigorder.** >>> ** >>> >>> ** ** >>> >>> We could model all order types as separate resources, if we can still >>> have a centralized mechanism for dealing with orders (e.g., fetching all >>> active orders) and ensure that all of these resources have some shared part >>> of their representation that was guaranteed to not only have the same >>> properties, but follow the same business logic (e.g., all orders must have >>> an orderer, the definition of "active" needs to be applied the same across >>> all orders, etc.)**** >>> >>> ** ** >>> >>> -Burke**** >>> >>> ** ** >>> >>> On Thu, Apr 19, 2012 at 11:08 AM, Saptarshi Purkayastha < >>> [email protected]> wrote:**** >>> >>> The Approach#3 is extremely complex and means that someone needs to >>> understand the type of "what"**** >>> >>> ** ** >>> >>> A client does not have to understand the types, just resources and so to >>> me the approach #1 is generalizable and concise to understand as a general >>> pattern when having to communicate with OpenMRS web services. Again why >>> would it be a problem to use Approach#1 with drugorder or laborder >>> resource??**** >>> >>> >>> --- >>> Regards, >>> Saptarshi PURKAYASTHA >>> >>> My Tech Blog: http://sunnytalkstech.blogspot.com >>> You Live by CHOICE, Not by CHANCE**** >>> >>> >>> >>> **** >>> >>> On 19 April 2012 06:43, Darius Jazayeri <[email protected]> wrote: >>> **** >>> >>> Hi All,**** >>> >>> ** ** >>> >>> We had a very good design call discussion on this, and Burke and I got >>> some brief, dense, and helpful insight on this from Jim Webber.**** >>> >>> ** ** >>> >>> The key insight: "Dealing with inheritance is easy: REST doesn't do it." >>> Basically: think of REST resources as simply exchanging documents. There >>> can be links between documents, but they are not allowed to have any formal >>> type hierarchy.**** >>> >>> ** ** >>> >>> So, after wrapping our heads around that, we've come up with three >>> different approaches to hide inheritance in the REST API, each applicable >>> to different things in the underlying Java class model:**** >>> >>> ** ** >>> >>> *Approach #1 - Subclass links to Superclass***** >>> >>> *Used for: Person/Patient***** >>> >>> In the REST API, the subclass document contains only subclass-specific >>> attributes, as well as a link to the superclass document. Example:**** >>> >>> GET patient/1234 ->**** >>> >>> {**** >>> >>> identifiers: [ ... ],**** >>> >>> person: { ref representation of person/1234 }**** >>> >>> }**** >>> >>> ** ** >>> >>> Even though the underlying Patient object inherits from Person, the >>> PatientResource should hide this. Thus you cannot edit a birthdate via the >>> patient resource, *and* creating a patient requires two POSTs (one to >>> create a person resource, the next to create a patient resource that links >>> to the person resource).**** >>> >>> ** ** >>> >>> *Approach #2 - Hide the Superclass***** >>> >>> *Used for: ActiveListItem/Allergy/Problem***** >>> >>> We use this approach when the multiple subclasses are not meaningfully >>> related to each other, and they only share a superclass for ease of >>> implementation. The REST API has no resource for the superclass. Each >>> subclass is its own resource, which contains the union of the properties of >>> its superclass and itself.**** >>> >>> ** ** >>> >>> *Approach #3 - Single Resource for Class Hierarchy***** >>> >>> *Used for: Concept/ConceptNumeric/ConceptComplex***** >>> >>> *Used for: Order/DrugOrder/XyzModuleOrder***** >>> >>> We use this approach when multiple subclasses really are different >>> versions of the same basic thing. In other words, they inherit the meaning >>> of the superclass, not just its implementation details.**** >>> >>> The REST API has a single resource (Concept, Order) that manages the >>> documents for all subclasses. Each document represents an underlying >>> instance of some class in the hierarchy, and it contains all properties of >>> that class, including inherited ones.**** >>> >>> For example, GET order?patient=1234 ->**** >>> >>> [**** >>> >>> {**** >>> >>> type: "org.openmrs.DrugOrder",**** >>> >>> startDate: "2011-01-01",**** >>> >>> // other Order properties go here**** >>> >>> dosage: "100mg",**** >>> >>> // other DrugOrder properties go here**** >>> >>> links: [ { rel: "self", uri: "order/11111" } ]**** >>> >>> },**** >>> >>> {**** >>> >>> type: "org.openmrs.Order",**** >>> >>> startDate: "2011-02-03",**** >>> >>> // other Order properties go here**** >>> >>> links: [ { rel: "self", uri: "order/22222" } ]**** >>> >>> },**** >>> >>> {**** >>> >>> type: "org.openmrs.module.lab.LabModuleOrder",**** >>> >>> startDate: "2011-03-04",**** >>> >>> // other Order properties go here**** >>> >>> specimen: { ref representation of a lab specimen },**** >>> >>> // other LabModuleOrder properties go here**** >>> >>> links: [ { rel: "self", uri: "order/33333" } ]**** >>> >>> }**** >>> >>> ]**** >>> >>> ** ** >>> >>> I've put a discriminator field in here ("type" might not be a safe name) >>> because it seems quite useful, and I think it's necessary for object >>> creation.**** >>> >>> For example: POST order**** >>> >>> {**** >>> >>> type: "org.openmrs.module.lab.LabModuleOrder",**** >>> >>> startDate: "2011-03-04" **** >>> >>> }**** >>> >>> ...will be delegated to the registered handler for LabModuleOrder, >>> rather than being handled directly by OrderResource.**** >>> >>> ** ** >>> >>> Implementation-wise:**** >>> >>> - we are going to count on the underlying OpenMRS API (and >>> ultimately Hibernate) to work such that if we do >>> OrderService.getOrdersByPatient(1234) we get back a List<Order> whose >>> individual items are actually of their correct subclasses.**** >>> - the core RESTWS module, and other modules, need to register >>> handlers for their known subclasses**** >>> - we will have to write some code, but that's the fun part. :-)**** >>> >>> Thoughts?**** >>> >>> ** ** >>> >>> -Darius**** >>> >>> ** ** >>> >>> ** ** >>> >>> On Wed, Apr 18, 2012 at 6:35 AM, Burke Mamlin <[email protected]> >>> wrote:**** >>> >>> My biggest concern is that it requires that consumers of the API >>> know/learn our data model; however, since the person is presented as a >>> property of the patient and gender as a property of the person (not the >>> patient directly), it's about as good a solution as I can imagine.**** >>> >>> ** ** >>> >>> -Burke**** >>> >>> ** ** >>> >>> On Tue, Apr 17, 2012 at 10:42 PM, Darius Jazayeri <[email protected]> >>> wrote:**** >>> >>> Yes, I meant what you said Burke.**** >>> >>> (Hopefully that was my only typo.)**** >>> >>> -Darius (by phone)**** >>> >>> On Apr 17, 2012 6:48 PM, "Burke Mamlin" <[email protected]> wrote: >>> **** >>> >>> Darius,**** >>> >>> ** ** >>> >>> Did you mean to two posts, one to patient & the other to person? Both >>> of yours were to the same resource.**** >>> >>> ** ** >>> >>> This implies that if you want to modify a patient's gender *and* >>> identifiers, >>> you have to do *two* POSTs. >>> For example: >>> POST patient/abcd1234 { identifiers: [ ... ] } >>> POST *person*/abcd1234 { gender: 'M' }**** >>> >>> ** ** >>> >>> Cheers,**** >>> >>> ** ** >>> >>> -Burke**** >>> >>> ** ** >>> >>> On Tue, Apr 17, 2012 at 8:01 PM, Darius Jazayeri <[email protected]> >>> wrote:**** >>> >>> Hi All,**** >>> >>> ** ** >>> >>> On tomorrow's design call one topic we will discuss is how to properly >>> represent inheritance and subclasses in a RESTful way. Fun and exciting >>> background discussion can be found on the ticket: >>> https://tickets.openmrs.org/browse/RESTWS-221. Call-in details are >>> here<https://tickets.openmrs.org/browse/RESTWS-221> >>> .**** >>> >>> ** ** >>> >>> My proposal, generally supported by Saptarshi, and disliked by Roger, is >>> that we represent a subclass as basically the composition of a superclass >>> resource, and a subclass resource contains subclass-specific properties, >>> and a pointer to the superclass.**** >>> >>> ** ** >>> >>> For example: GET patient/abcd1234 ->**** >>> >>> {**** >>> >>> identifiers: [ ... ], // this is the only Patient-specific property* >>> *** >>> >>> links: [**** >>> >>> { rel: "self", uri: "patient/abcd1234" }**** >>> >>> ],**** >>> >>> person: { // this is a pointer to the superclass**** >>> >>> names: [ ... ],**** >>> >>> gender: 'M',**** >>> >>> // other properties on the Person superclass follow **** >>> >>> links: [**** >>> >>> { rel: "self", uri: "person/abcd1234" }**** >>> >>> ]**** >>> >>> }**** >>> >>> }**** >>> >>> ** ** >>> >>> This implies that if you want to modify a patient's gender * and* >>> identifiers, >>> you have to do *two* POSTs.**** >>> >>> For example:**** >>> >>> POST patient/abcd1234 { identifiers: [ ... ] }**** >>> >>> POST patient/abcd1234 { gender: 'M' }**** >>> >>> ** ** >>> >>> You should be able to *create* a patient in a single POST, but not >>> update one that way.**** >>> >>> ** ** >>> >>> At first this seems inconvenient, and unintuitive for someone who's used >>> to the OpenMRS Java API. The reason for this is that I think it's necessary >>> to support web-standard caching, which allows web service scalability. >>> Basically, imagine that someone may be running a reverse-proxy on their >>> server, which caches resources generated by the server and serves them up >>> to many web clients, relieving server load. In order for that reverse-proxy >>> cache to avoid serving up stale data, we cannot allow doing POST >>> patient/abc123 to modify the resource at person/abc123. (According to web >>> standards, if the cache sees a POST to patient/abc123, this invalidates >>> that specific cache entry, but all of this is invisible to the >>> server.) Thus my proposal.**** >>> >>> ** ** >>> >>> I'm only moderately certain I'm approaching this right, so if you know >>> or suspect the right answer to this problem (especially if it's different >>> from my proposal), please reply and/or join us on the design call tomorrow! >>> **** >>> >>> ** ** >>> >>> -Darius**** >>> >>> ** ** >>> >>> PS- The other topic we'll discuss on the call is Wyclif's proposal for a >>> module, that will allow us to reboot our implementation of orders and order >>> entry, such that we implement something better, and it runs on both old and >>> new versions of OpenMRS. All in all this will be an action-packed call.* >>> *** >>> ------------------------------ >>> >>> 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 >>> **** >>> >>> ** ** >>> >>> ** ** >>> ------------------------------ >>> >>> 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 >>> >> >> ------------------------------ >> 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]
