I was looking at updating sync2 and atomfeed modules to allow syncing obs and encounters using the REST client and ran into a couple of issues.
Syncing obs is already supported but doesn’t work because of the bugs caused by the issues below.
Custom feed configurations in the atomfeed module aren’t getting picked, the default configuration has writing of feeds for obs disabled meaning another module is unable to turn it on, see ATF-25 for details.
The category name used by the sync2 module for obs is observation which breaks some code that assumes category names match resource names in the rest module, see SYNCT-268 for more details.
In the web services module as part of RESTWS-571, a new method was added that the framework calls to instantiate resource objects but existing usages of the old one were not updated to call the new method. This means an obs with a numeric question concept can’t get synced because an exception is thrown here since the framework still calls it, see RESTWS-735.
ObsResource1_8.setValue needs to be updated to better handle obs with coded answers, see RESTWS-736 for more details.
I don’t think syncing works for updates and voided items for the rest client or maybe I just don’t understand how it works, I have tested several resources and it fails, I want to think it is because when a new item is synced onto another server, it gets a different uuid but the calls that sync updates always use the local uuid in the rest url, the module needs to be updated to remove calls like this now that RESTWS-729 is complete that way uuids stay the same across all servers which would make updates sync-able.
Some of the @PropertySetter annotations we have on some resources are redundant and actually are only causing bugs, see RESTWS-737 for details.
I also think the usage of this switch statement to do the conversion is anti-pattern, we need to reimplement it, one way that comes to mind is to introduce an interface which can be implemented by converters, the converters could be registered as spring beans. This will allow other modules to make their persistent entities sync-able which currently isn’t possible, but there is no pressure to have this done soon yet.
I will be creating PRs for the tickets mentioned above.
Syncing Obs and encounters fails sometimes and when I took a closer look at the involved modules I noticed the following:
The sync module processes feed entries by category rather than the order in which they were recorded, this is problematic because some entities need to be synced before others to satisfy relationship references. E.g. visits need to be synced before encounters, and encounters before observations.
The other issue is that the atomfeed module doesn’t pay attention to relationships when creating feed entries, e.g when you create an encounter that contains new observations in the same transaction, it should create the encounter feed entry before those of the observations, right now they are in random order, this is problematic because you end up with a similar issue as above where relationship references aren’t satisfied depending on which one gets processed first. I believe this is the issue that was discussed a while back where some folks were suggesting that the interceptor in the atomfeed module needs to fire events serially.
I also think that the sync module doesn’t handle created and deleted items exactly well. For created items, it needs to check the existence of an item before attempting to sync it. Take the example of a visit, when a visit is created along with new encounters, the atomfeed module creates feed entries for the visit and each contained encounter, the sync module will process the visit feed and will create encounters it contains in the same request implying that, when the create feed entries for the individual encounters are processed, they will fail because the module will be creating entities with duplicate uuids since they already exist, this is also true if you change the order and sync encounters before visits. The issue is prevalent pretty much across all many-to-one relationships like encounter and Obs.
The quick fix could be to change the sync module to always in this order; Persons, Patients, Visits, Encounters, Obs and then everything else.
The long-term solution is to fix the atomfeed module to fire events serially and then change the sync module process the feeds in the same order they were created rather than by category. We would also need to update the module no first check existence of an item before processing a CREATE/DELETE feed entry.
I also think the web services module needs to be updated to handle certain scenarios better, take the example of the posted new encounter payload below;
Basically, the code above is creating an encounter that belongs to a visit, the visit field contains a reference back to the encounter it contains to complete the bi-directional relationship since the uuids match and this is a very typical scenario when posting back data. Unfortunately, the rest module ends up creating 2 separate encounters but the operation fails anyways, and as you might guess, this will be true for encounters and obs they contain. This might be tricky to address but would be nice to support otherwise the sync module would need to always remove one side of these bidirectional collection relationships.