From the former topic about the Order.accessionNumber I assume that the accessionNumber database column has no unique constraint because management of accessionNumber is not the concern of OpenMRS therefore no such restriction is placed on it.
However in the radiology module, I would like to generate unique accession numbers (will use the approach of the OrderNumberGenerator) and also enforce this uniqueness for
RadiologyOrders (which are a sublcass of
TestOrder and thus
I thought about a few ways and would love your input!!
- on database level
- on service level
- on DAO level
- own accessionNumber column for RadiologyOrder
1 On database level:
The hibernate mapping for
RadiologyOrder uses the table-per-subclass strategy as also done by
TestOrder see RadiologyOrder.hbm.xml. Hibernate has the possibility of overriding attributes so one can add a unique constraint on an inherited property but I think this is only possible for abstract classes (mapped superclass) or embedded classes, since in this case the value of
accessionNumber is held by the
orders table it cant be done, otherwise it would affect all orders.
2 On service level:
not exactly sure if the use of a custom validator that checks if the accessionNumber value of a new radiology order already exists in the DB is a good approach. lets say a user enters a radiology order where he sets the accessionNumber, for example for a retrospective entry. at validation time accessionNumber did not exist so it passes but when actually writing to the DB the accessionNumber is taken by another radiology order. these to radiology orders would then have the same accessionNumber which should never happen!
3 On DAO level:
I could ensure uniqueness in the
saveRadiologyOrder method which checks if accessionNumber already exists in the DB and fails if it does.
4 own accessionNumber column for RadiologyOrder:
I could create an own column radiologyAccessionNumber in
RadiologyOrder which is stored in the DB table radiology_order and has a unique constraint. Then in the
RadiologyOrderService I generate this accessionNumber and set the order.accessionNumber with it.
I am leaning to approach number 4. Seems way cleaner/less error prone to me.
What do you think?
(PS: just writing about it helped me a lot )
@darius could I please get your input on this? thank you!!
The other approach is to use a Validator for the subclass (like https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/validator/DrugOrderValidator.java ), as these are automatically called when you save a subclass of that type.
You would need to query whether there are any conflicts via a DAO.
I agree that #4 is the least error prone, but I don’t think it’s cleaner (wearing my OpenMRS core hat).
right again #4 is not cleaner (duplicate data in the db) but because its least error prone I’d favor it.
my problem with using the Validator is that it prevents duplicate accessionNumbers on entry but how can I absolutely prevent the case
at validation time accessionNumber did not exist so it passes but when actually writing to the DB the accessionNumber is taken by another radiology order. these to radiology orders would then have the same accessionNumber
which could be of different patients
what if I combine a Validator and fail in the DAO if accessionNumber already exists?
To understand, are concerned about the millisecond between validation and
-Darius (by phone)
yes not sure if thats being paranoid but better safe than sorry
I made an AccessionNumberGenerator like the OrderNumberGenerator with the sequence seed value coming from a global property. So when a user enters a radiology order he doesnt specify the accessionNumber and the accessionNumber is set in the RadiologyOrderService when saving. So with this approach I dont really need validation since the accessionNumber is auto-generated and the way the OrderNumberGenerator does it ensures uniqueness (+ its thread safe).
I just wonder what happens if I would allow users to specify the accession number themselves when entering an order. Not sure if this could be a requirement. But in that case I would have two sources (user + generator) which could cause such collisions. I dont think I will allow this now, I just wanted to think the whole accession number generation through thoroughly also with input from others on potential issues thanks for your input!!
Of course chance of duplicate nr is also there if other modules generate orders with accession nr
@darius do have any comments on my concerns expressed in my last two posts? thanks again!!
@teleivo, sorry, don’t have mindspace to actively engage with this, but it sounds to me like you’re overthinking things.
I understand that:
- you’ve written a generator that is thread-safe and will generates unique numbers (and I guess this is the primary use case)
- also, you’re worried that someday if you allow users to specify accession numbers and/or other modules will be creating RadiologyOrders with manual accession numbers
- and you’re thinking about the scenario where hypothetical scenario (2) happens and also these are being fired so fast +/- maliciously that in the (possible) millisecond between validation and saving, the same accession number is reused.
That seems like a very contrived situation, that’s never going to happen in real life, so my advice is to ignore it, and just do the straightforward thing and do (1) as you’ve already done, and have the validator also check this at the API level.