Technical Discussion
This page contains the history of items debated between developers. It could stand to be cleaned up a little bit. Perhaps we need to break some of the larger discussions into separate pages?
Action Items
There are no actions items at this time.
Discussion Items
Auditing / Event Broadcasting
Justin and I were talking about the intersection between auditing / event broadcasting and transactions. Here are some thoughts: Considering these functions
PatientService.createPatient(Patient p) { dao.saveOrUpdate(p); context.postMessage(new PatientCreatedEvent(p)); } ObsService.createObs(Obs o) { dao.saveOrUpdate(o); context.postMessage(new ObsCreatedEvent(o)); }
Assume we want to do a batch creation of patient + several obs like this:
// START TRANSACTION context.getPatientService().createPatient(p); for (Obs o : obsBatch) { context.getObsService().createObs(o); } // END TRANSACTION
If we truly want to guarantee that the application messages we're delivering are correct, then we need to meet two criteria:
- Don't post messages until all database writes in the transaction are committed. (I.e. if a call to createObs() fails, then don't post the PatientCreatedEvent message.)
- Failure to deliver a message should cause the whole transaction to rollback. (I.e. if we can't deliver the PatientCreatedEvent message, then don't create the patient.)
I think we can meet the first criterion under our current framework, although it informs how we'd handle transactions. (I'm not 100% sure this works, but I think that when calling context.postMessage() we can check whether there's an active transaction, and if so hold, queue up that message. Committing the transaction would have to cause a callback to the context, so it knows to actually post the queued messages.)
The second criterion is much more complicated. Justin points out that to meet it we'd need to be running an application server that does Container Managed Transactions like JBoss. (This strikes fear into my heart.)
So I suppose there are 3 options here:
- Event notification is unreliable and you should expect occasional phantom messages.
- Event notification is mostly reliable, but if the messaging system itself fails, then actions can happen without notifying listeners.
- Event notification is 100% reliable: every action guarantees its listeners are notified.
I like #2 myself. Any thoughts?
Djazayeri 09:23, 16 December 2005 (US Eastern Standard Time)
Message Service API
The Message Service API will comprise the main message transport service. It should be used as the underlying service for all messaging in the system. For example, the Note Service API will use the Message Service API when message are send via email, phone, pager.
Methods
void sendMessage(Message message) throws SendMessageException;
Implementing Class (example)
public class EmailMessageServiceImpl implements MessageService { public void sendMessage(Message notification) { try { Context context = new InitialContext(); Session mailSession = (Session) context.lookup("java:/Mail"); Message mailMessage = new MimeMessage(mailSession); for ( User user : notification.getRecipients() ) { mailMessage.setRecipients(javax.mail.Message.RecipientType.TO, InternetAddress.parse(user.getEmail(), false) ); mailMessage.setFrom(InternetAddress.parse("info@openmrs.org", false)); mailMessage.setSubject(notification.getSubject()); mailMessage.setText(notification.getMessage()); Transport.send(mailMessage); // send message (probably want a try-catch around this ... left out for brevity) } } catch (Exception e) { e.printStackTrace(); throw new SendMailMessageException(); // subclass of SendMessageException } } }
Alert API
Over the past few days we have had several conversations about messaging and alerting. We have been cognizant of the fact that there are some subtle differences, but have tried to keep the same model for both types of messages. However, after a little more discussion today, we've decided (at least for the time being) to follow your advice and discern the two.
Our conversations today lead us to the position that Alerts are very closely related to Notes and User-Reported Errors. We checked the data model and confirmed that Alerts can be implemented as Notes. The only difference between Notes/Errors and Alerts are:
- A Note is entered manually (by a data entry clerk) to report an error or concern with a patient, observation, and/or encounter, while an Alert will tend to be automatically generated (in the case of the decision support system).
- An alert has a list of people who need to be actively informed of it, while a note should just be seen by anyone viewing a patient/obs page. (Anyone viewing a patient/obs should see related alerts too.) An error is sort of like an alert-to-the-data-manager.
- An alert is actionable, while a note isn't. (An error is probably actionable as "fixed")
So we're planning to model the alerting system around the Note.
Methods
List<Alert> getAlerts(User user); // Get alerts by user Alert getAlert(Integer alertId); // Get alert details AlertCount getAlertCount(User user); // Get the alert count (contains information like # read vs # actioned vs # all alerts) List<Alert> getAlertsByPatient(Integer patientId); // Get alerts by patient List<Alert> getAlertsByObservation(Integer obsId); // Get alerts by observation List<Alert> getAlertsByEncounter(Integer encounterId); // Get alerts by encounter void raiseAlert(Alert alert); // Raise an alert
Model
NoteRecipient
user_id
group_id
relationship_type
relationship_target
view_date
action_status ('none', 'actioned', ...) maybe defined in template
action_date
An alert is (a Note) + (Possible Actions, Current Action Status) + (People who need to know about it)
Alert Note - associated with a patient, observation, and/or encounter AlertType ActionTaken (move to Note?)
AlertType Name Description Template
Template - used to generate the body for the alert Name TemplateText
Action-related
Action - actions that can be taken for an alert Name Description
PossibleAction - associated possible actions that can be taken for an alert Alert Action
Notification-related: we want to be able to send an alert to
- a specific user
- a specific group
- a specific role
- everybody with a certain relationship to a certain patient (e.g. MD-in-charge of patient_id 1234)
NoteRecipient // each row should have either UserId OR GroupId OR RoleId OR (RelationshipType AND RelationshipTarget) NoteRecipientId (PK) NoteId (FK to Notes table) UserId GoupId RoleId RelationshipType RelationshipTarget
AddressBookEntry // named distribution lists of NoteRecipients, maybe with a default value in the alert-type table for convenience, but not directly referenced by specific alerts
--Jmiranda 17:01, 23 January 2006 (US Eastern Standard Time)
Transactions
Considering the issue of doing transactions programmatically, consider this contrived example:
PatientService.createPatient(Patient p) { dao.startTransaction(); dao.saveOrUpdate(p); dao.endTransaction(); } ApplicationCode.someMethod(List<Patient> patientBatch) { dao.startTransaction(); for (Patient p : patientBatch) { context.getPatientService().createPatient(p); } dao.endTransaction(); }
These need to be nested transactions in the sense that the endTransaction() call in PatientService.createPatient() should not end the "higher-level" transaction initiated by ApplicationCode.someMethod().
Modifying PatientService.createPatient() as follows doesn't work:
PatientService.createPatient(Patient p) { dao.startOrContinueTransaction(); dao.saveOrUpdate(p); dao.maybeEndTransaction(); // what does this mean? }
My book gives this as Spring's way of doing this programmatically:
TransactionDefinition td = new DefaultTransactionDefinition(); // this means PROPAGATION_REQUIRED TransactionStatus tranStatus = transactionManager.getTransaction(td); try { //something } catch (ApplicationException e) { transactionManager.rollback(tranStatus); throw e; } transactionManager.commit(tranStatus);
The proper solution seems to be using Java 1.5 annotations to do transactions declaratively, and letting Spring work its magic. So something like:
@TxAttribute(propagationType=PropagationType.REQUIRED) PatientService.createPatient(Patient p) { dao.saveOrUpdate(p); } @TxAttribute(propagationType=PropagationType.REQUIRED) ApplicationCode.someMethod(List<Patient> patientBatch) { for (Patient p : patientBatch) { context.getPatientService().createPatient(p); } }
Also on the topic I'm going to transcribe a couple of insightful paragraphs from my Spring book: "We recommend applying transactions at the business layer. This allows the business layer to catch any exceptions that caused a rollback and issue the appropriate business-level application exceptions. ... Data access code should not delimit transaction boundaries because it doesn't implement business logic, and atomicity is a business-level concept. Let's say we apply a transaction on a data access method that updates the balance of an account. This would prevent us from reusing this method in a task that transferred data between two accounts. If the subtraction from the first account was already committed, then we could not roll back all changes in the case of the addition to the second account failing."
Djazayeri 16:07, 15 December 2005 (US Eastern Standard Time)
Selecting Obs
Hi Vibha,
To follow up from the call today (and mostly repeat what I emailed before) we will need something like this method:
List<Obs> findObs(Patient who, Concept what, Clause c, Date fromDate, Date toDate, GroupMethod m);
"Clause c" needs to be able to represent the following:
- For numeric, text, and datetime values
- { <, <=, >, >=, =, != } a constant number/string/date
- { between, not-between } two constant numbers/strings/dates
- { in, not-in } a collection of numbers/strings/dates
- For numeric values
- above_hi_critical, above_hi_normal, below_low_normal, below_low_critical, outside_normal_range, outside_critical_range, inside_normal_range, inside_critical_range
- For coded (i.e. concept) values
- { =, != } a concept
- { in, not-in } a collection of concepts
- For boolean values
- { =, != } a constant boolean
- True (i.e. just get all obs of that type, regardless of value)
"GroupMethod m" needs to be able to represent: ALL_MATCHES, EARLIEST_MATCH, LATEST_MATCH
We’ll also want the same method that takes a ConceptSet as its second argument. And we’ll really want to have a version that takes a Collection<Patient>. I know you don’t need that.
So…any thoughts?
-Darius
Darius,
Here's a thought for grouping clauses...you could use "and()" and "or()" methods in the Clause class:
class Clause { public static final Clause TRUE = new Clause("=", new Object[] {Boolean.TRUE}); public static final Clause NOT_EQUAL = "!="; // etc. // for convenience for operators with one argument public Clause(String operator, Object arg) { ... } // for operators that have multiple arguments (e.g., BETWEEN) public Clause(String operator, Object[] args) { ... } // allow for grouping of clauses public Clause or(Clause c) { ... } public Clause and(Close c) { ... } }
to allow for:
Clause A = new Clause(Clause.LESS_THAN, Double.valueOf(3.5)); Clause B = new Clause(Clause.BEFORE, DateFormat.parse("1/1/2005")); Clause C = new Clause(Clause.NOT_EQUAL, Double.valueOf(1.5)); (A and B) or C becomes (A.and(B)).or(C) A and (B or C) becomes A.and(B.or(C))
How would you handle a query like this?
get the highest creatinine before first CD4 < 200
in two steps?
Okay, I'm going to step way outside of the box for a second.
What if the findObs() method used a generic "ObsQuery" object instead, e.g.:
Obs<List> findObs(Patient who, ObsQuery query);
ObsQuery could be called recursively to build sophisticated queries. The resulting ObsQuery could be passed to the DAO level to generate the appropriate SQL.
class ObsQuery { public static ObsQuery query(ObsQuery q) { ... } public static ObsQuery query(Concept c) { ... } public static ObsQuery query(List<Concept> c) { ... } ... public ObsQuery before(Date d) { ... } public ObsQuery before(ObsQuery q) { ... } ... public ObsQuery between(Date start, Date finish); ... public ObsQuery lt(double n); // op="<", operands=self & n public ObsQuery lte(double n); // op="<=", operands=self & n ... public ObsQuery first(); // op=first, operands=self public ObsQuery last(); // op=last, operands=self ... public ObsQuery and(ObsQuery q); // op=and, operands=self & q public ObsQuery or(ObsQuery q); // op=or, operands=self & q ... // you could event support a generic operator public ObsQuery op(String operator, Object[] operands); // and support Clause public ObsQuery clause(Clause c); // op=c.op, operands=c.args ... // for DAO level to traverse the query public String getOperator(); // action for this part of query public Object[] getOperands(); // could be ObsQuery, Concept, Date, etc. }
Some examples:
"get most recent concept"
findObs(who, new ObsQuery(concept).last());
"get concepts between fromDate and toDate"
findObs(who, new ObsQuery(concept).after(fromDate).before(toDate));
"get max creatinine before CD4 < 200"
findObs(who, new ObsQuery(creatinine).before(new ObsQuery(CD4).lt(200).first()).max());
So, your proposed method could be implemented as:
List<Obs> findObs(Patient who, Concept what, Clause c, Date fromDate, Date toDate, GroupMethod m) { return findObs(who, new ObsQuery(what).clause(c).after(fromDate).before(toDate).group(m)); }
-Burke 18:33, 16 December 2005 (US Eastern Standard Time)
Voided Bit
I've noticed that tables in the data model have the "voided" bit, but I'm curious as to why it's nullable. Shouldn't a row need to be voided or not voided? And doesn't that make for extra work when selecting because the where clause has to say "where voided is not null and voided=0", when really it should just have to say "where voided=0"? -Christian
Agreed. -Ben 09:33, 12 December 2005 (US Eastern Standard Time)
Concepts within the PATIENT table
This is in OpenmrsConstants.java. I'm vehemently opposed:
public static final Map<String, String> CIVIL_STATUS() { HashMap<String, String> civilStatus = new HashMap<String, String>(); civilStatus.put("1", "Single"); civilStatus.put("2", "Married"); civilStatus.put("3", "Divorced"); civilStatus.put("4", "Widowed"); return civilStatus; }
Generally speaking the problem is that we have something that should really be a concept, being stored in the Patient table. A couple possible solutions:
- Get civil_status out of the Patient table, and make it an Obs.
- Implement this as a Java 1.5 Enum.
- Put the meaning in the message.properties file
Djazayeri 17:14, 13 February 2006 (US Eastern Standard Time)
Noting errors
I just added the "if (role == null) { ... }" to the following code in Context.hasPrivilege(String). I figure this will be a one-time error for each developer who's using an older test database. Does anyone care how we mark these errors?
- Should I email the fix to the developers list?
- Should (someone) have emailed the list saying "update to the latest demo database"?
- Should I put a bad-style comment like I just did?
- Should I put in a log.warn instead?
Role role = getUserService().getRole(OpenmrsConstants.ANONYMOUS_ROLE); if (role == null) { throw new RuntimeException("Database out of sync with code: " + OpenmrsConstants.ANONYMOUS_ROLE + " role does not exist"); } if (role.hasPrivilege(privilege)) return true;
Djazayeri 17:30, 13 February 2006 (US Eastern Standard Time)
I appreciate the note. I've followed similar syntax with another change I made. In the future, we'll try and either have sql updates available and/or a note sent to the developer's list. Ben 16:04, 27 February 2006 (US Eastern Standard Time)
Export Patient XML Format
<patient_data> <patient patient_id="123" given_name="Darius" middle_name="Graham" last_name="Jazayeri" last_name2="Xyz" gender="M" race="Xyz" birthdate="1978-04-11 00:00:00" birthdate_estimated="false" birthplace="Atlanta" citizenship="US" tribe="tribeName" mothers_name="Mary" civil_status="Divorced" death_date="..." cause_of_death="..." health_district="Massachusetts" health_center="Cambridge Hospital" health_center_id="321"> <names> --One name is picked randomly to go in the attributes above. All names including that one will go here also <name given_name="Darius" middle_name="Graham" last_name="Jazayeri" last_name2="Xyz"/> </names> <encounters> <metadata> <location location_id="1">BIDMC Primary Care</location> <encounter_type encounter_type_id="1">Physical Exam</encounter_type> <form form_id="1">Physical exam form</form> <provider provider_id="1">Bob, MD</provider> </metadata> <observations> <obs obs_id="1" concept_id="1" concept_name="Concept name in session locale" datetime="2006-02-13 15:45:00" accession_number="abc123" comment="a comment" date_started="..." date_stopped="..." obs_group_id="1" value_group_id="1" value_coded_id="1" value_coded="name of concept" datatype="coded" --only one of these will actually exist value_boolean="true" datatype="boolean" --only one of these will actually exist value_datetime="2006-02-14 14:23:05" datatype="datetime" --only one of these will actually exist value_numeric="5.3" datatype="numeric" --only one of these will actually exist value_text="some text" datatype="text" --only one of these will actually exist value_modifier="..."> The non-null one of the above values goes here, preceded by value_modifier, if any </obs> </observations> <orders> <order concept_id="1" instructions="as needed" start_date="2006-02-14 02:55:00" auto_expire_date="..." orderer="1^Bob, MD" discontinued="true" discontinued_date="..." discontinued_reason="..."> Concept name of this order </order> </orders> </encounters> <observations> --these are observations that don't belong to any encounter same format as observations in encounters </observations> </patient> </patient_data>
Djazayeri 03:07, 14 February 2006 (US Eastern Standard Time)
Drug data model
I was looking at the drug data table, and the following thoughts spring to mind:
- Are these one-to-one with concepts, or would "AZT 300mg tablet" and "AZT 100mg tablet" both share the concept "AZT"?
- If so:
- How are we dealing with translations of this?
- Otherwise:
- Why do we need drug_id instead of just concept_id?
- Why do we have a name column
- If so:
- The answer is "yes" — that means we want to support both ways. In smaller settings where docs are ordering straight from the inventory, you'd have a concept for every drug form (and drug_id and name are less useful). In larger scale settings, however, we'd like to have AZT as the concept and each dosage form in the drug table. Basically, the drug table represents the "pharmacy inventory" and whether or not these match one-to-one with concepts is an implementation decision. We can discuss how we imagine handling this. -Burke
- Should daily_mg_per_kg really be in this table? If so, some prescribing guidlines say something like "20-25 mg/kg", and we probably want to represent that too.
- Agreed. We're not excited about how this is currently modeled. I think the daily_mg_per_kg was a carry-over from PIH's MDR TB stuff. We need to have dosage, frequency, and concentration information...we can discuss options during the conference call. -Burke
- Why is combination a smallint(5)?
- Should be tinyint (boolean equiv). This table has not been groomed as much as others, since we haven't started using it other than for a few brand drugs for AMRS. -Burke
- What's the distinction between inn and name?
- inn was carried over from discussion with Hamish about the PIH model. inn is supposed to represent the international standard name for the active ingredient(s). For AMRS, we plan on using concepts for this (i.e., linking drug forms through the same concept_id); however, I suppose if you decided to create a concept-per-drug-form one-to-one mapping between drugs and concepts, then you may still want a mechanism to relate all of your forms of AMPICILLIN. Personally, I'd rather folks do that with concept_set and ditch the inn attribute. We can discuss options. -Burke
- We have boolean columns for "important?" and "voided?"
- "important?" is probably webapp functionality than doesn't belong in the core data model, but one should be able to void drug formulations.
- I don't know what "important" means, but we'd agree that changed_by, date_chagned, voided, voided_by, date_voided, and voided_reason should be added to the drug table. -Burke
Djazayeri 15:59, 27 February 2006 (US Eastern Standard Time)
Application properties
I want to define some application-level properties that are going to differ between the PIH and Regenstrief systems. For example the PIH patient-set-selector tool will allow the user to select patients based on a concept called "ARV TREATMENT GROUP" which Regenstrief won't have. Any strong feelings about where this type of configuration should go?
- I assume we should have separate config properties files for each module.
- Should we structure each module so it has a base properties file, and then a second properties file that we/you use to override those shared values?
Djazayeri 17:59, 13 March 2006 (Eastern Standard Time)
Paul and I were talking about this recently. I agree that we need to both support implementation-specific settings and module-specific settings. Additionally, I agree that each should have a "default" set of properties and a defined method for extending/adjusting those properties. Site specific changes (other than simply overriding properties iin OpenMRS-build.properties as we do now) could be treated almost like just another module — i.e., the same process we define to customize & separate modules from the base could be extended to implementation-specific changes by simply choosing a unique "module" name for each site (e.g., amrs or hiv-emr). The compile process needs a way to detect, at compile time, which modules are desired in the WAR and dynamically include them. I've got more thoughts on this, but don't have time. I'll add this topic for this Thursday's conference call. -Burke 18:20, 13 March 2006 (Eastern Standard Time)
Task Scheduling
Here's a very crude attempt at coding what I've been thinking. I'm having a little bit of difficulty picturing how we plug the different frameworks (jdk timer, quartz) into our application and will need to spend some more time tomorrow trying to finish the puzzle. The more concrete elements are at the bottom of the page where i try to show how we might create a ProcessFormEntryQueueTask to fulfill our current needs.
I have been struggling with the "Why don't we just adopt one of these frameworks" idea, but I really want this to be an elegant solution that allows us some flexibility, so I'm not giving up yet.
As you can see, the scheduler service is an interface. Implementations might include one for the JDK timer (SimpleSchedulerService), one for Quartz (QuartzSchedulerService), and maybe one for a Quartz persisted service (PersistentQuartzSchedulerService). I haven't thought this through completely, so bear with me.
Scheduler Service
package org.openmrs.scheduler; /** * Defines methods required to schedule a task. */ public interface SchedulerService { /** * Schedule a recurring task that occurs according to the given schedule (start time and interval). */ public void scheduleTask( Task task, Schedule schedule ); }
Task
package org.openmrs.scheduler; /** * Stateless task interface */ public interface Task { /** * */ public void execute( ); }
Stateful Task
package org.openmrs.scheduler; /** * Stateful task interface */ public interface StatefulTask extends Task { /** * Execute a task with the given state (in case the process will need some * data from the database (via the services layer). */ public void setContext(Context context); // Not sure if we need it, but might be a good idea. //public void setData(Map data); }
Schedule
package org.openmrs.scheduler; /** * Describes when to start a task and how often it is executed. */ public class Schedule { private long start; private long interval; /** * Public constructor * * @param start timestamp for when to start the task (does not need to be in the future if the interval is specified). * @param interval time to wait between executing task (<= 0 indicates that it should only be run once) */ public Schedule(long start, long interval) { this.start = start; this.interval = interval; } /** * Get the start time (in seconds) when the task should be executed. * For instance, use "new Date().getTime()", if you want it to start now. * * @return long start time */ public long getStart() { return start; } /** * Get number of seconds until task is executed again * * @return long number of seconds. */ public long getInterval() { return interval; } }
Simple Scheduler Service
package org.openmrs.scheduler; import java.util.TimerTask; import java.util.Timer; /** * Simple scheduler service that uses JDK timer to trigger and execute scheduled tasks */ public class SimpleSchedulerService { /** * Schedule the given task according to the given schedule. */ public void scheduleTask( SimpleTimerTask task, Schedule schedule ) { Timer timer = new Timer(); if ( schedule.getInterval() == 0 ) { timer.schedule(task, schedule.getStart() ); } else { timer.schedule(task, schedule.getStart(), schedule.getInterval() ); } } }
Simple Timer Task
package org.openmrs.scheduler; import java.util.TimerTask; public class SimpleTimerTask implements TimerTask { // private Task task; public SimpleTimerTask(Task task) { this.task = task; } public void run() { task.execute(); } }
Say Hello Task
/** * Implementation of the stateless task. */ public class SayHelloTask implements Task { /** * Illustrates stateless functionality as simply as possible. Not very * useful in our system, except maybe as a polling thread that checks internet * connectivity by opening a connection to an external URL. * * But even that isn't very useful unless it tells someone or something * about the connectivity (i.e. calls another service method) */ public void execute() { System.out.println("Hello World!"); } }
Process Form Entry Queue Task #1
/** * Implementation of the stateful task that simply gets the next form entry out of the queue * */ public class ProcessFormEntryQueueTask implements StatefulTask { // Instance of context used during task execution private Context context; /** * Setter method for context */ public void setContext( Context context ) { this.context = context; } /** * Process the next form entry in the database and then remove the form entry from the database. */ public void execute() { FormEntryQueue queue = context.getFormEntryService().getNextFormEntryQueue(); FormEntryQueueProcessor processor = new FormEntryQueueProcessor( context ); processor.transformFormEntryQueue( queue ); // This actually happens in transformFormEntryQueue ... wondering if client should worry about this instead. //context.getFormEntryService().deleteFormEntryQueue( queue ); } }
Process Form Entry Queue Task #2
/** * Implementation of a stateless task that gets the next form entry and processes it. */ public class ProcessFormEntryQueueTask implements StatefulTask { // Instance of context used during task execution private Context context; /** * Setter method for context */ public void setContext( Context context ) { this.context = context; } public void execute() { // Or instead of doing all that, you could look at the FormEntryQueueProcessor // class and realize that the transform next entry has already been implemented. :) new FormEntryQueueProcessor( context ).transformNextFormEntryQueue(); } }
--Jmiranda 02:36, 16 March 2006 (Eastern Standard Time)
Justin, this is a great start. Reponding to your comment above about deleting queue entries in the processor vs. the calling method...in its current state, calling processFormEntryQueue(FormEntryQeueu feq), it might make sense to only do the processing and leave it to the caller to delete the entry. In fact, I'd rather put that code into a FormEntryQueueProcessor.processNextFormEntryQueue() method, so the schedule would just call that method and the business of deleting queues after they're processed is left to the processor.
This brings up another point: ideally, we create the FormEntryQueueProcessor object only once per context. I guess thats could be up to FromEntryQueueProcessor if I added a .getInstance(Context myContext) method.
I googled an older arcticle on the topic of task scheduling and another slightly more recent one. You may have seen them & I don't know if they contain anything particularly insightful.
I think we could survive being bound to quartz, but if you can create an abstraction layer, that'd be great!
-Burke 12:15, 16 March 2006 (Eastern Standard Time)
Sorry, I added this section yesterday, but it was up with the rest of my rambling above, so you may have missed it. I just cut-and-paste it down here to give the conversation a better flow.
The more I think about it, the more I realize that synchronization should be handled by the code that is being executed, not by the scheduler or task components. The reason is because synchronization is a requirement of the use case that we were discussing (i.e. we don't want to process the same form twice). This is a business rule. We might have a task that we want to schedule twice at different times/intervals and we may not care whether they are synchronized (unfortunately, I cannot think of an example). If we synchronize at a level higher than the code that needs to be synchronized (i.e. getting next entry queue within FormEntryQueueProcessor.processFormEntryQueue() method) we might significantly degrade performance. In addition, I think we risk running into deadlock situations (although I can't envision a scenario at the moment) the higher up we synchronize. I think we need to block on the call to getNextFormEntryQueue(), right? Or maybe have a synchronized processNextFormEntryQueue() method that looks like this ...
public synchronized void processNextFormEntryQueue() { FormEntryQueue formEntry = context.getFormService().getNextFormEntryQueue(); transformFormEntryQueue( formEntry ); context.getFormService().deleteFormEntryQueue( formEntry ) }
--Jmiranda 08:54, 17 March 2006 (Eastern Standard Time)
Here's another issue I had written about yesterday ... and then tried to clarify (read: obfuscate) this morning.
One of the issues with scheduling that we may need to be careful about is that fact that we only want one instance of a task to be run for a system (i.e. email report daily should only run once per day for the entire system). If we utilize clustering or design some complex server hierarchy, we need to make sure that only one of the servers instantiate any given task. We may even need to think about designating a single server as the scheduling server, and make sure all scheduling goes through that server. That seems a bit overly complex to me right now, but I was thinking about the bigger picture and I got a little freaked out about issues we may run into having multiple systems sharing information.
The complex server hierarchy doesn't scare me as much as the clustering issue. I envision the following scenario. We have 4 clinics in a project. Those 4 clinics needs to send daily email reports with statistics compiled using their local database. We have a fifth node that handles all the data and needs to send a weekly report on the entire project. In this case, we simply schedule the daily emails on the 4 child nodes, and a weekly report on the parent node. I don't see any issues with that, related specifically to task scheduling. The data synchronization issue is problematic, but that's an issue for the Data Synchronization and Complex Server Hierarchy teams. Oh wait, that's us. But at least that's far enough into the future for us not to worry about at the moment. Just thought we should have some of these issues in the back of our minds.
Clustering will be an issue though. If we decide we want to use clustering at some point, we need to make task scheduling "cluster-aware". I have no idea how Tomcat handles clustering and don't really think it's worthwhile to pursue that research at the moment. But if you guys have any plans to go down that route, let's have another discussion.
--Jmiranda 09:18, 17 March 2006 (Eastern Standard Time)
User and Patient extend Person
I know you guys have given a lot of thought to this already and will probably have much more to say, but I wanted to throw down some use cases we have that would support this model and also help us think about implementation challenges. Most of the use cases stem from the fact that we have a provider called an "accompagnateur" (which must be said in a snobby french accent) that we often want to track almost as rigorously as a patient:
- We need to be able to locate accompagnateurs by address
- We need to be able to issue/search on Identifiers for accompagnateurs
- At some point in the not-so-distant future, we will need to schedule both patients and accompagnateurs to come on certain days and track whether or not they showed up
- We very well may want to eventually be able to make observations about accompagnateurs or other health care providers.
- This may not affect the model at all, but we will want to be able to put together a drug order based on an accompagnateur, who may have 5 patients that they are going to visit and bring medication to.
Again, these are just examples to think about.
Also, I decided to do a quick check of how MySQL felt about FKs referencing FKs, and it seems to be ok with them. Here are my tests:
CREATE TABLE `test_person` ( `p_id` int(11) NOT NULL DEFAULT '0', `name` varchar(255) NOT NULL DEFAULT '', PRIMARY KEY (`p_id`) ) engine=innodb DEFAULT charset=utf8;
CREATE TABLE `test_patient` ( `p_id` int(11) NOT NULL DEFAULT '0', `location` varchar(255) NOT NULL DEFAULT '', PRIMARY KEY (`p_id`), constraint `patient_person_id` FOREIGN KEY (`p_id`) REFERENCES `test_person` (`p_id`) ) engine=innodb DEFAULT charset=utf8;
CREATE TABLE `test_user` ( `p_id` int(11) NOT NULL DEFAULT '0', `username` varchar(255) NOT NULL DEFAULT '', PRIMARY KEY (`p_id`), constraint `user_person_id` FOREIGN KEY (`p_id`) REFERENCES `test_person` (`p_id`) ) engine=innodb DEFAULT charset=utf8;
CREATE TABLE `test_role` ( `role_id` int(11) NOT NULL DEFAULT '0', `p_id` int(11) NOT NULL DEFAULT '0', `rolename` varchar(255) NOT NULL DEFAULT '', PRIMARY KEY (`role_id`), constraint `user_role_id` FOREIGN KEY (`p_id`) REFERENCES `test_user` (`p_id`) ) engine=innodb DEFAULT charset=utf8;
CREATE TABLE `test_obs` ( `obs_id` int(11) NOT NULL DEFAULT '0', `p_id` int(11) NOT NULL DEFAULT '0', `observation` varchar(255) NOT NULL DEFAULT '', PRIMARY KEY (`obs_id`), constraint `patient_obs_id` FOREIGN KEY (`p_id`) REFERENCES `test_patient` (`p_id`) ) engine=innodb DEFAULT charset=utf8;
INSERT INTO test_person (p_id, name) VALUES (1, 'Christian'); INSERT INTO test_person (p_id, name) VALUES (2, 'Darius'); INSERT INTO test_person (p_id, name) VALUES (3, 'Justin'); INSERT INTO test_person (p_id, name) VALUES (4, 'Aunt Jemima'); SELECT * FROM test_person; INSERT INTO test_patient(p_id, location) VALUES(1, 'rwinkwavu hospital'); INSERT INTO test_patient(p_id, location) VALUES(4, 'supermarket shelves everywhere'); INSERT INTO test_patient(p_id, location) VALUES(5, 'this wont work'); SELECT * FROM test_patient; INSERT INTO test_user(p_id, username) VALUES(1, 'callen'); INSERT INTO test_user(p_id, username) VALUES(3, 'jmiranda'); INSERT INTO test_user(p_id, username) VALUES(5, 'thiswontwork'); SELECT * FROM test_user; INSERT INTO test_role(role_id, p_id, rolename) VALUES(1, 1, 'village idiot'); INSERT INTO test_role(role_id, p_id, rolename) VALUES(2, 2, 'darius is just a person so this should not work'); INSERT INTO test_role(role_id, p_id, rolename) VALUES(3, 3, 'van triliquist'); INSERT INTO test_role(role_id, p_id, rolename) VALUES(4, 4, 'aunt jemima is a patient but not a user so this should not work'); INSERT INTO test_role(role_id, p_id, rolename) VALUES(5, 5, 'thiswontwork'); SELECT * FROM test_role; INSERT INTO test_obs(obs_id, p_id, observation) VALUES(1, 1, 'needs a better haircut'); INSERT INTO test_obs(obs_id, p_id, observation) VALUES(2, 3, 'justin is only a user so this should not work'); INSERT INTO test_obs(obs_id, p_id, observation) VALUES(3, 4, 'loves monorails'); INSERT INTO test_obs(obs_id, p_id, observation) VALUES(4, 5, 'this wont work'); SELECT * FROM test_obs; ALTER TABLE test_obs DROP FOREIGN KEY patient_obs_id; ALTER TABLE test_obs DROP INDEX patient_obs_id; ALTER TABLE test_obs ADD INDEX(p_id); ALTER TABLE test_obs ADD constraint person_obs_id FOREIGN KEY (p_id) REFERENCES test_person (p_id); INSERT INTO test_obs(obs_id, p_id, observation) VALUES(5, 5, 'this still should not work'); INSERT INTO test_obs(obs_id, p_id, observation) VALUES(6, 2, 'now this should work'); INSERT INTO test_obs(obs_id, p_id, observation) VALUES(7, 3, 'and this too'); SELECT * FROM test_obs;
You can see from this that, not only can you do what we want to do (FKs that reference FKs), but you can still hold up referential integrity at the database level. By referencing the person_id that exists in a patient or user table (which is a foreign key), you can still require that the person be a patient or user. AND, in addition to that, it is also possible if we decided to start out allowing only patients to have observations, it's easy to modify the table later to allow all persons to have observations. It's like casting, but in a database... sort of.
Of course this only addresses data model level implementation issues, but I'd say, from what I saw here in these tests, and knowing that Hibernate is strong in the ways of inheritance in tables, I'm thinking this might not be so bad after all.
Callen 10:04, 28 April 2006 (Eastern Standard Time)
I just ran the same/similar tests in Oracle with identical results. It's a little more of a pain to alter a foreign key constraint in Oracle, but still very much possible. So that makes both MySQL and Oracle supporting the functionality at the data model that we're looking for. Again, still not sure if this is standard SQL, but I'm pretty sure it's supported, minus perhaps the altering of the foreign key reference - but I don't think that really matters.
Callen 05:58, 29 April 2006 (Eastern Standard Time)
I’ve talked it over a little with Justin and Darius, but wanted to know what you guys thought about using tab libraries to generate the fields that appear in forms. I created a small prototype of what I’m talking about, called “fieldGen�?, that resides in the openmrs taglib. Basically, you pass the tag the Class name of the object for which you are trying to show the field (whether it be String, Boolean, Long, Integer, Concept, User, Encounter), and it generates the HTML that corresponds. Still somewhat useful for simple objects/fields, but - like PropertyEditors - comes in very handy when used for more complex types.
The FieldGen tag itself has some built in handling for many of the basic types (String, Boolean, Integer, etc) already, and then also has a mechanism that makes it extensible. You can add as many handlers as you want for as many types as you like using this notation in the openmrs-servlet.xml file:
<bean id="fieldGenHandlerFactory" class="org.openmrs.web.taglib.fieldgen.FieldGenHandlerFactory"> <property name="handlers"> <props> <prop key="org.openmrs.Patient">org.openmrs.web.taglib.fieldgen.PatientHandler</prop> <prop key="org.openmrs.Concept">org.openmrs.web.taglib.fieldgen.ConceptHandler</prop> <prop key="org.openmrs.OrderType">org.openmrs.web.taglib.fieldgen.OrderTypeHandler</prop> </props> </property> </bean>
The Handler classes are very simple, and you could look the files the correspond to any of the examples above to see. They only need to implement one output method (from the FieldGenHandler interface).
The advantages I see to this, combined with using PropertyEditors are: 1) Much cleaner .jsp files (included a sample further down) 2) Allows simpler and faster writing of JSPs – don’t have to find similar HTML/code to copy paste – only need to know the Class name of the field 3) Interface is more consistent across the application 4) Reusable code means that one of us writes the Handler for a new class, and the rest of us never have to worry about it (similar to PropertyEditors) 5) Reusable code means you only have to change one Handler class when you want to change the way one selects, say, a concept in a form 6) Reusable code means less duplication of work 7) One of the more interesting advantages is that you don’t actually need to even know the class of an Object to display it and Spring-bind it. This means that you could actually generate a form for an object without actually knowing anything about it – using reflection you could just iterate over the fields in obj.getClass().getFields(), bind to it, and use the tag library to display the field for it. In fact, this is how the Admin interface for ReportObjects works, since there’s no way of knowing all the different classes of ReportObjects that we’ll make ahead of time.
I say all of this knowing that the Indiana team may very well have already thought of this and done similar, but I wasn’t able to find anything in the code. I can see some places where tag libraries are used to render complex DHTML interfaces, but I was thinking that we could maybe use a solution like this that handles all of the types that we need it to handle with some consistency.
Here’s a sample of some code in a JSP file using this notation:
<table> <tr> <td><spring:message code="Order.orderType"/></td> <td> <spring:bind path="order.orderType"> <openmrs:fieldGen type="org.openmrs.OrderType" formFieldName="${status.expression}" startVal="${status.value}" emptySelectMessage="${emptyOrderTypeList}" /> <c:if test="${status.errorMessage != ''}"><span class="error">${status.errorMessage}</span></c:if> </spring:bind> </td> </tr> <tr> <td valign="top"><spring:message code="Order.concept"/></td> <td valign="top"> <spring:bind path="order.concept"> <openmrs:fieldGen type="org.openmrs.Concept" formFieldName="${status.expression}" startVal="${status.value}" /> <c:if test="${status.errorMessage != ''}"><span class="error">${status.errorMessage}</span></c:if> </spring:bind> </td> </tr> <tr> <td valign="top"><spring:message code="general.instructions"/></td> <td valign="top"> <spring:bind path="order.instructions"> <openmrs:fieldGen type="java.lang.String" formFieldName="${status.expression}" startVal="${status.value}" fieldLength="40" /> <c:if test="${status.errorMessage != ''}"><span class="error">${status.errorMessage}</span></c:if> </spring:bind> </td> </tr> <tr> <td valign="top"><spring:message code="general.dateStart"/></td> <td valign="top"> <spring:bind path="order.startDate"> <openmrs:fieldGen type="java.util.Date" formFieldName="${status.expression}" startVal="${status.value}" /> <c:if test="${status.errorMessage != ''}"><span class="error">${status.errorMessage}</span></c:if> </spring:bind> </td> </tr>
Callen 09:19, 18 May 2006 (Eastern Standard Time)
Should obs.patient_id become obs.person_id?
As we re-arrange the data model such that `patient` and `users` tables extend `person` (see #119), the consideration for storing observations about people (rather than just patients) arose. We discussed the possibility
Pros
- We could store observations about any person, including users — e.g., longitudinal data on patient providers or relatives that are not patients in the system.
Cons
- Blurs focus from an electronic medical record to a general data collection system.
Concerns
- Changes the assumption about observational data — i.e., selecting people based on observational data could return data for people who are not patients. How would this affect our API?
- To find patients with CD4 < 250, we would need to join the obs table with patient to limit the results to patient data. How would this affect query times when we have a hundred thousand patients and of tens of millions of observations? Would the cost of the extra join be minimal or significant?
- Do services like PatientSetService become PersonSetService? What are the ramifications to this and other aspects of the API?
- We'd like to keep the focus of OpenMRS on being a patient repository and do that well. That is, we would insist that obs.person_id could be supported without changing the focus of OpenMRS (i.e., forcing everyone using data to account for the possibility that the HIV status observation they received from the API might not belong to a patient) — e.g., accomplished through modules or non-core extensions to OpenMRS.
Coding Concepts
Figure 1. Symptoms section of Donnees de Base form
THE OLD WAY
There are several ways to code a concept for a form. Take, for example, “Toux (> 3 semaines)�?, which translates to “Cough (> 3 weeks)�?. Using our current (or old) model, we would create a set of flat concepts.
Cough (Symptom/Finding) https://dev.openmrs.org/openmrs/dictionary/concept.htm?conceptId=107
Cough Duration (Coded) https://dev.openmrs.org/openmrs/dictionary/concept.htm?conceptId=5959
In the “Symptoms�? section above, Cough needs to have a Boolean value (as well) to show whether patient does or does not have a Cough, or whether the doctor did not fill out this section. So we could presumably add another Boolean concept to the mix “Cough Present�? to show whether the patient is showing symptoms of a cough.
Cough Present (Boolean or Coded) N/A
Another approach would be to code Symptoms as the question and allow Cough (Symptom/Finding) to be an answer, along with Fever, Night Sweats, etc (other symptoms taken from the form). Using the old concept construction (sans Concept Set) it is not clear to me how you would then link Cough Duration, Fever Duration, etc to their symptoms.
THE NEW WAY (I think)
With our new model, we can build concept hierarchies that encapsulate all the information you need about a symptom. One way to do this would be to build a Cough concept (symptom/finding) that has Cough Present and Cough Duration as related concepts (part of the concept set of Cough).
Furthermore (and this is the main point of this entry), we don’t need to use modifiers like “Cough�? in “Cough Duration�? anymore. We can simply create a more base-level concept (i.e. Duration) and add that as a child within all the symptoms. My conception of the hierarchy looks like this.
Cough (Concept) Duration (Concept) Present (Boolean)
Fever (Concept) Duration (Concept) Present (Boolean)
Duration (Concept) Start Date (Date) – or Start Date and End Date as Concepts? End Date (Date)
The main issue with this approach (that I can see) is that, at some point in the future, the definition of Duration might change for one concept (say, Cough) and not the others. That is when we would need to be able to create subclasses of Concepts like Cough Duration.
Cough Duration (Concept) Start Date (Concept) End Date (Concept) Decibel Level (Numeric)
Does this sound ok?
Well, here’s where things might get a bit tricky. Left to our own devices, Administrators (like myself) could even create base-level concepts for our data types: Boolean, Numeric, Date, etc. We could then create complex concepts like Symptom as a concept set of String and Boolean concepts.
Symptom (Concept) String (Concept) – might represent a name, but who knows? Boolean (Concept) – might represent "presence", but who knows?
Boolean (Concept) boolean (Boolean)
String (Concept) string (String)
The main problem with doing things this way is that the children Concepts are very interpretive. String could be anything: a name, comment, description. Same with the Boolean value. Only the author of the concept would know. This would be a very bad way to use the system, but it is possible since we are leaving things open. I don’t suggest we do anything about it, just acknowledge that it is possible.
Back to the discussion at hand. Here is the Past Medical History section taken from our Donnees de Base (initial intake).
Figure 2. Past Medical History section of Donnees de Base form
I've had several discussions with Darius and Hamish about the "right" way to code this concept. My first thought was to create a relationship between the Tuberculosis Diagnosis concept and the Tuberculosis Diagnosis Date and Tuberculosis Diagnosis Remark concepts
Tuberculosis (Concept:Diagnosis) Tuberculosis Date (Concept:Date) Tuberculosis Remark (Concept:String)
(Forgive the poor notation above. I'm simply stating that each of these is of Concept:Type.)
But we need to be able to say whether the doctor answered yes/no to whether the patient has had Tuberculosis, so I added a Boolean concept to the mix.
Tuberculosis (Concept:Diagnosis) Present (Concept:Boolean) Diagnosis Date (Concept:Date) Remark (Concept:String)
This basically makes Tuberculosis a container for its attributes (Present, TB Date, TB Remark). Therefore Tuberculosis itself would not be associated with a value. Correct? It is a container for the questions "Did the patient have tuberculosis?", "When?", and "What other information do you want to provide?"
Darius pointed out that Tuberculosis should be the answer to the Past Medical History question and not a container.
Past Medical History (Concept) Past Medical Diagnosis (Concept) where Tuberculosis, Pneumonia are answers
Tuberculosis could then be mapped as we have it above, right? But doesn't that extend our hierarchy to two levels (i.e. Past history -> Tuberculosis -> Date).
Tuberculosis (Concept:Diagnosis) Present (Concept:Boolean) Diagnosis Date (Concept:Date) Remark (Concept:String)
Anyway, I'm a little bit confused about the correct (or preferred) approach here, so any suggestions would be helpful.
UI portlets
We want three main pages in our GUI to be customizable in a data-driven way.
- Home Page
- Patient Dashboard
- widgets may assume a patientId parameter
- Cohort Dashboard
- widgets may assume a patientSet parameter
- probably we should have an optional patientAnalysis parameter too
We might say something like: homepage.layout = WelcomeMessageWidget,FindOnePatientWidget(size=compact),PickSavedCohortWidget(size=expandable)
We should define a standard set of parameters that most widgets should understand. For example:
- size=compact ... take up as little space as possible
- size=full ... take up as much space as necessary
- size=expandable ... start out small, but take more space if the user touches the widget
I think that these widgets will generally need to pull data from the database, so I think our only option is to include them with a jsp:include plus some arguments.
So the patient dashboard page could look something like this:
Find another patient: <div id="widget1"> <jsp:include page="FindOnePatient.widget" flush="true"> <jsp:param name="size" value="compact"/> </jsp:include> </div> Currently looking at patient: <div id="widget2"> <jsp:include> <jsp:param name="patientId" value="${model.patient.patientId}"/> </jsp:include> </div>
Djazayeri 15:28, 27 June 2006 (EDT)
Internationalization of non-Concept database objects
At present we are dealing with internationalization in different ways for different things:
- Concepts are special-cased, have a concept_name table, synonyms table, etc.
- Webapp messages are done via message.properties.
- Other database objects are not internationalized at all
We need to be able to internationalize other domain objects. So far I've only considered internationalizing the names, and not really anything else. For example Forms (English: "Intake form", French "Donnees de Base"), locations ("Cange Hospital" versus "Hopital de Cange"), relationship_types, etc.
One option is to follow the model we used with concepts for other db objects as well, but nobody really likes this, since it's going to add a lot of tables.
Another option is to create one table which holds translations for names of many domain objects. Like one of these options:
- create table name_lookup (table_name, code, locale, display_name)
- ("relationship_type", "1", "en_US") -> "Community Health Worker"
- ("relationship_type", "1", "fr") -> "Accompagnateur"
- create table name_lookup (code, locale, display_name)
- ("relationship_type.1", "en_US") -> "Community Health Worker"
I don't know if we can get Hibernate to make this magically work, because there won't actually be a foreign key relationship
A third option is to deal with this between the DAO layer and the Service layer. For example:
class LocationService { public Location getLocationByName(String localizedName) { String code = translatorService.lookupCode(localizedName); return dao.getLocationByName(code); } }
At first thought I like option #3. But we need to consider whether option #2 is actually doable in Hibernate, and what the performance considerations are. Djazayeri 16:08, 10 July 2006 (EDT)
Are there any examples of how other folks are handling this? -Burke 16:39, 10 July 2006 (EDT)
I know that the developers do not want to tie to the UI to the database, but I would strongly favor putting messages in the database. Perhaps these can be null at which the current messages.properties file would be used. However, the importance of being able to easily manage the messages for all users as well as for translations should not be diminished. As for names of forms, I don't think they need to be translated as the entire form would have to be translated, and so I think locale is a meta data element for the form which then would be used for filtering which forms are seen. When forms are completely database driven with on the fly translation of captions and data elements, then I think we can change the name of the form as well... --Akanter 06:54, 28 April 2007 (EDT)
Orders and Order entry
I'd like to open the discussion about the Order table containing an encounter_id, but not a patient_id. This implies that we cannot have an order without an encounter. Shouldn't this be more like Obs (with a direct link to patient possible)?
-Christian 14:04, 18 July 2006 (EDT)
Their reasoning for a nullable obs.encounter is that something like a blood lab can send some data (via an hl7 soap service? :-)) without having to attach to a certain encounter.
I can see this reasoning being used on an order possibly. However, what is the use case for a doctor firing off an order for a patient without some sort of encounter with that patient?
Do we need to add order.patient_id and leave order.encounter_id as NOT NULL ? Would that speed up processing/searching for orders?
-Ben 14:50, 18 July 2006 (EDT)
I think that's reasonable - that an order could occur without an encounter, but I'm pretty sure there will almost always (at least more often) be a patient associated with that order. And also, in the Obs table, we allow a nullable link to encounter and patient. Shouldn't it look more like that - both for symmetry and to map more accurately to reality?
-Christian 14:04, 19 July 2006 (EDT)
In many cases, we want to create a Drug Order for a patient using a fairly simple interface. Ideally, the patient would not have to select an OrderType, but that would mean hard-coding it somehow, to make sure it is entered in the database as a "Drug" type order. What is our approach for doing this kind of type hard-coding? Inevitably different people using OpenMRS will have different OrderTypes, so hard-coding a value will be a problem.
-Christian 10:19, 30 July 2006 (EDT)
Again, hope to get some feedback about whether or not we should have patient_id as part of the order table.
-Christian 10:19, 30 July 2006 (EDT)
Modeling Parameters
class EvaluationContext { public void setParameter(String name, Object value); public Object getParameter(String name); /** * Allows access to parameters, and arithmetic and date operations. E.g. "$startDate - 7days" * (Maybe use Velocity.) */ public Object evaluateExpression(String expression); public void cacheCalculation(String name, Object value); public Object getCalculation(String name); public void clearCache(); }
class Parameter { String name; Object defaultValue; Object value; // null if this is a parameter definition Class dataType; }
/** * Should be implemented by Reports, Logic rules, and cohort queries. * We need a core web UI element that can query an instance for its required parameters, and queries the user for values */ interface Parametrized { public List<Parameter> getRequiredParameters }
