Sure, but don't you typically indicate the need for subclasses in Java by requiring an interface or specific base class as a parameter instead of putting the term "subclass" in your method name? For example, assuming new types of orders will show up as new concept classes:
registerOrderableClass(Class<? extends Order> clazz, String name, OrderableClassHandler handler); -Burke On Thu, Apr 19, 2012 at 2:40 PM, Darius Jazayeri <[email protected]>wrote: > 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 >> > > ------------------------------ > 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]
