On 08/25/2011 02:30 PM, Johannes Renner wrote: > So (1.) there are some helper classes that I put into a new separate > package, see 01-new-classes.patch:
AuditLog.java has 4 (!) functions log() and all of them has different semantics - if I ommit that they are utilized to logging. Please consider different names. > > Therefore there is (2.) calls to the logger from within the existing > code, see 03-rhnaction-example.patch: Hmm, what I dislike is the need to change the code. Which will be prone to coder error and you may end up in situtation where somebody commit code, which are worth auditing, but the audit call will not be there. If I would be designing such functionality I would prefer something like mod_security. So you will have one entry point and you will not care if the action is actually handled by cobbler, backend, frontend or some aliens. This would even allow you to have configuration file where you will define what and how will be audited. In your design it will be hard to say which actions are audited. Especially in 2 years from now, when other will add/remove/change your initial audit calls. > - While most of the struts actions are RhnAction or RhnLookupDispatchAction, > there is still some that are not derived from these classes. Therefore it's > not enough to put a shortcut method to the logger in the(se) superclass(es). Exactly, if you go from bottom (base classes) you can be never sure if log all childs and getting the big picture will be very hard. And you could not be really sure till you test it. -- Miroslav Suchy Red Hat Satellite Engineering _______________________________________________ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
