The issue that I neglected to mention is that I imagine these things will be spring beans, wired up that way, so there won't actually be a register method called at all...
You're right that we should be able to use generic type signatures to require and self-document the need for a subclass. I'll deal with that as an implementation detail. (And we're way in the weeds now...) -Darius On Thu, Apr 19, 2012 at 11:52 AM, Burke Mamlin <[email protected]>wrote: > 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 >> > > ------------------------------ > 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]
