This ticket was discussed on the dev list June 7-12, 2012.
I'm pretty sure that performance would be improved if we didn't have to maintain relational integrity for the audit info. The idea would be to replace it with a log table that would contain date/time, user, object type, object version, type of change, after-image. The after-image would be a serialized version (or json or xml) of the object after the change. There might be the ability to use the DB's transaction log feature rather than/supplemented by our features. There should be the ability to view the log and to view the history of an object and navigate the object model (e.g. drill down on history from patient to encounters to obs). Maybe a rollback capability.
Actually i have been developing an audit log module in my free time, am like 60% done with the code.
It keeps a log of created, deleted and updated, reverted items, for the updated ones it shows you the old values and you can choose to view all the changes for an object. I plan to add a feature that allows you to revert simple properties for the first pass.
@Wyclif, that sounds like it's moving in the right direction. My thought was also that we would put that into core and use it instead of our current mechanism. My sense is that coding the capability would be less than half the effort, the big load would be to assure that everything got changed appropriately to not break anything.
The other thing worth investigating is the use of the DB's transaction log, it might save storing all the before-image data
Whoops, looks like they never committed to that branch. Here is the review for it:
There was some work done by students and in svn in this branch:
I don't think its 100%, but it was at least partially done.
Ben thanks for the link, actually i never knew some work had been done for this module.
When i glanced at the code i think there is alot to be changed or to be done and it seems to be error prone in its state.
My concerns about the code:
1- I dont know why we store the current value in the blob since the 'current' value is what actually is in the database.
2- I also don't like assuming a domain object has to be an instance of Auditable to be watched, i would rather let implementations define what domain objects they want to watch otherwise, if none then all are watched.
3. The code that creates log entries is in the wrong interceptor method because right now the log entry will always be created even when the operation is rolledback in case of an exception.
What i have in the module i have been working on the side:
1- The domain object is pretty much the same
2- The operation is a property of AuditLog and they its possibile values are defined as enum constants (Action.CREATED,Action.DELETED,Action.UPDATED), which means we don't have to parse the xml just to get the operation.
3- The module lets users define monitored objects, in the long run users should be able to define what properties of a monitored object they wish to ignore for changes.
4- Log entries are not created on rollbacks
5- Lets the user to define if a change from blank to null and vice versa should be logged
6- The module doesn't store the current value since it appears to be redundant after all we have it in the database
7- The module also exposes a service too fetch view log entries with additional filtering on the domain object, operation and date
8 - The only web component the module has so far is a page to view log entries but i was working on even letting users revert simple changes.
9- Mappings between changed property names and their old values are stored in a database table but i think storing them as xml might be a better idea.
I hadn't yet code anything to do with handling changes in child collections on a monitored object, but i think this is also something we need to take care of.
I see a benefit in using the code in my module rather than beating up the code for that project, i can make the changes in the module that you guys don't like in my design. We also have the option to keep this in a module and make it a bundled one or i can move the code to core but first in a branch.
What do you think?
Seems like more of a core change than a module. If we want core objects to take advantage of it, it would have to be core.
If the objects aren't Auditable, how do you get the creator/changedBy/etc properties off of it?
The current value is stored so that you know what it changed to as well as what it changed from. If you have 5 changes listed, to know what the change history was in the 3rd block you'd have to look at the 3rd and 4th elements. Seems easier to just have to look at one. But if you aren't storing as xml its less of an issue.
I'd love to see what you have done. Can you write up a quick something describing the high points? Perhaps a schema? And the api calls?
This is in response to Ben's request for an overview of what i have covered in the module. You can find the attached java file which is the current state of the interceptor.
- Uses a hibernate interceptor to track changes, deletes and inserts
- The operation is a property of AuditLog and they its possibile values are defined as enum constants (Action.CREATED,Action.DELETED,Action.UPDATED, Action.REVERTED)
- Log entries are not created on rollbacks
- The module also exposes a service to fetch view log entries with additional filtering on the domain object, operation and date
- The service doesn't expose saveAuditLog method
- Mappings between changed property names and their old values are stored in a database table(determining appropriate column size for old value might make me change this to xml)
- Only handles OpenmrsObjects
If this is to be moved to core, the key change to make is to bake in setting of audit info for Auditables.
At a later version, i had planned to allow other code register AuditListeners so that the interceptor notifies them when objects are created/edited/deleted.
Key end user features
- The module lets users define monitored objects, in the long run users should be able to define what properties of a monitored object they wish to ignore for changes.
- Lets the user to define if a change from blank to null and vice versa should be logged
- Should let the user revert changes for simple properties/child collections for the first pass
- Ability to auto or manually archive old logs to file system otherwise the table is likely to gor so big and take long to query
API and Domian objects
On Fri, Jun 8, 2012 at 1:20 PM, Wyclif Luyima <firstname.lastname@example.org> wrote:
Since i was doing this in a module bearing in mind that there was core auditable interceptor to handle Auditables, i was not checking for instances of Auditable, my interceptor keeps logs for all instances of OpenmrsObject that are marked as monitored by the user otherwise all if none is marked. If we are to move my stuff to core i would just that have to handle Auditables accordingly by drawing a line between the 2 concerns that come up i.e. setting audit info and creating log entries since not everything, this lets admins to also monitor objects that are not Auditables.
Why i don't store the current value? If i had 5 update logs for the same object, for any log i get the value it was changed to by doing this. I have a service method that can give me a list of auditLog entries for a given object filtered by action, startDate, sorting order with paging and it returns them sorted by dateCreated, so if i set the length arg to 1 and sortOrder to ASC, the previousValue of the returned log is the value it was changed to, if none, then it is the latest change so i use the vale in the DB. If using xml probably storing current value might be worth it and i plan to switch to xml because i had started to worry about the column datatype of the previousValue field since some objects might have properties used to store long pieces of text.
I will write up something about the current state of what i have and attach the Interceptor java class.