<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Apr 6, 2017 at 11:26 AM Roy Golan <<a href="mailto:rgolan@redhat.com">rgolan@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Apr 5, 2017 at 11:34 PM Moti Asayag <<a href="mailto:masayag@redhat.com" target="_blank">masayag@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_extra m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg">On Wed, Apr 5, 2017 at 11:17 PM, Roy Golan <span dir="ltr" class="m_621998939108385272gmail_msg"><<a href="mailto:rgolan@redhat.com" class="m_621998939108385272gmail_msg" target="_blank">rgolan@redhat.com</a>></span> wrote:<br class="m_621998939108385272gmail_msg"><blockquote class="gmail_quote m_621998939108385272gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><span class="m_621998939108385272m_-6163671470618302110gmail- m_621998939108385272gmail_msg"><div dir="ltr" class="m_621998939108385272gmail_msg">On Wed, Apr 5, 2017 at 9:06 PM Moti Asayag <<a href="mailto:masayag@redhat.com" class="m_621998939108385272gmail_msg" target="_blank">masayag@redhat.com</a>> wrote:<br class="m_621998939108385272gmail_msg"></div><blockquote class="gmail_quote m_621998939108385272gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">Hi All,<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"></div>ATM, there are 78 occurrences of "Injector.injectMembers(new AuditLogableBase())" in ovirt-engine project, which their main purpose is to ease the resolve of the placeholders of the audit log message while logging an event.<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"></div>For instance AuditLogType.MAC_ADDRESS_IS_EXTERNAL is being used from ImportVmCommandBase.java in the following way:<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">private AuditLogableBase createExternalMacsAuditLog(VM vm, Set<String> externalMacs) {<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"> AuditLogableBase logable = <b class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">Injector.injectMembers</b>(new AuditLogableBase());<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"> logable.setVmId(vm.getId());<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"> logable.setCustomCommaSeparatedValues("MACAddr", externalMacs);<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"> return logable;<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"> }<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg" clear="all"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">The entry in the properties file is:<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">MAC_ADDRESS_IS_EXTERNAL=VM ${<b class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">VmName</b>} has MAC address(es) ${MACAddr}, which is/are out of its MAC pool definitions.<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"></div><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">Therefore the only purpose of the injection is to allow the AuditLogDirector to resolve the ${<b class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">VmName</b>} which is already known at the time of creating the AuditLogableBase entry.<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"></div><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"></div><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">The result is injecting the DAOs for the AuditLogableBase instance and using the VM dao to retrieve the VM entry from the DB.<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"></div><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">This is just a wastef of injection and DB access while both can be spared.<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"></div><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">This could have been easily replaced by one of the following:<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><ul class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><li class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">auditLogableBase.setVmName(vm.getName());</li></ul></div></div></div></div></blockquote></span><div class="m_621998939108385272gmail_msg">- setVmName is protected so not usable as is <br class="m_621998939108385272gmail_msg"></div></div></div></blockquote><div class="m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"></div></div></div></div><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_extra m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><div class="m_621998939108385272gmail_msg">It will become public if we agree on <br class="m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"><a href="https://gerrit.ovirt.org/#/c/75244/2/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogableBase.java" class="m_621998939108385272gmail_msg" target="_blank">https://gerrit.ovirt.org/#/c/75244/2/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogableBase.java</a><br class="m_621998939108385272gmail_msg"> <br class="m_621998939108385272gmail_msg"></div></div></div></div><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_extra m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><blockquote class="gmail_quote m_621998939108385272gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><div class="m_621998939108385272gmail_msg"></div><blockquote class="gmail_quote m_621998939108385272gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><ul class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><li class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">auditLogableBase.addCustomValue("VmName", vm.getName());</li></ul></div></div></div></div></blockquote><div class="m_621998939108385272gmail_msg">I prefer this, it is readable. and BTW it is fluent, it returns 'this' so use<br class="m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"></div><div class="m_621998939108385272gmail_msg"> AuditLogDirector(new AuditLogableBase(type)<br class="m_621998939108385272gmail_msg"> .addCustomValue("VmName", vm.getName()));</div></div></div></blockquote><div class="m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"></div></div></div></div><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_extra m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><div class="m_621998939108385272gmail_msg">I'm okay with this as well.<br class="m_621998939108385272gmail_msg"> <br class="m_621998939108385272gmail_msg"></div></div></div></div><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_extra m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><blockquote class="gmail_quote m_621998939108385272gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><span class="m_621998939108385272m_-6163671470618302110gmail- m_621998939108385272gmail_msg"><blockquote class="gmail_quote m_621998939108385272gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><p class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">Please pick up any occurrence from your domain and send a patch to replace it where possible.<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"></p></div><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">Thanks in advance,<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604m_5996702152823124421gmail_signature m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"><div dir="ltr" class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">Moti<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"></div></div></div></div></div></div></blockquote><div class="m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"></div></span><div class="m_621998939108385272gmail_msg">+1<br class="m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"></div><div class="m_621998939108385272gmail_msg">Frankly the fact that all the DAOs sets protected access in AuditLogableBase is a total abuse. Every command should declare its own deps. <br class="m_621998939108385272gmail_msg"></div></div><span class="m_621998939108385272m_-6163671470618302110gmail- m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"></div></span></div></blockquote><div class="m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"></div></div></div></div><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_extra m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><div class="m_621998939108385272gmail_msg">That will require a huge effort.<br class="m_621998939108385272gmail_msg"></div></div></div></div></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>Removed them all, <a href="https://gerrit.ovirt.org/75262" target="_blank">https://gerrit.ovirt.org/75262</a> compile +1<br>Now need to fix the tests - I'd appreciate help here<br><br></div></div></div></blockquote><div>Verified +1 the removal of all protected DAO's in AuditLogableBase. <br></div><div>Instead of constant injection of *66* dao fields for *every* command inject only *9* which are most atm, They will be removed as well in the future.<br></div><div><br></div><div>Please help review, its a long, but stupid patch which just adds injections.<br><a target="_blank" href="https://gerrit.ovirt.org/75262">https://gerrit.ovirt.org/75262</a><br><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_extra m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><div class="m_621998939108385272gmail_msg"> <br class="m_621998939108385272gmail_msg"></div></div></div></div><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_extra m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"><blockquote class="gmail_quote m_621998939108385272gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="m_621998939108385272gmail_msg"><span class="m_621998939108385272m_-6163671470618302110gmail- m_621998939108385272gmail_msg"><div class="gmail_quote m_621998939108385272gmail_msg"></div><div class="gmail_quote m_621998939108385272gmail_msg">_______________________________________________<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg"></div><div class="gmail_quote m_621998939108385272gmail_msg"><blockquote class="gmail_quote m_621998939108385272gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Devel mailing list<br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">
<a href="mailto:Devel@ovirt.org" class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg" target="_blank">Devel@ovirt.org</a><br class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg">
<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" class="m_621998939108385272m_-6163671470618302110gmail-m_1772032661836158604gmail_msg m_621998939108385272gmail_msg" target="_blank">http://lists.ovirt.org/mailman/listinfo/devel</a></blockquote></div></span></div>
</blockquote></div></div></div><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="gmail_extra m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg"><br class="m_621998939108385272gmail_msg" clear="all"><br class="m_621998939108385272gmail_msg">-- <br class="m_621998939108385272gmail_msg"><div class="m_621998939108385272m_-6163671470618302110gmail_signature m_621998939108385272gmail_msg"><div dir="ltr" class="m_621998939108385272gmail_msg"><div class="m_621998939108385272gmail_msg">Regards,<br class="m_621998939108385272gmail_msg"></div>Moti<br class="m_621998939108385272gmail_msg"></div></div>
</div></div></blockquote></div></div></blockquote></div></div>