<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 5, 2017 at 11:17 PM, Roy Golan <span dir="ltr"><<a href="mailto:rgolan@redhat.com" target="_blank">rgolan@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><span class="gmail-"><div dir="ltr">On Wed, Apr 5, 2017 at 9:06 PM Moti Asayag <<a href="mailto:masayag@redhat.com" target="_blank">masayag@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604gmail_msg">Hi All,<br class="gmail-m_1772032661836158604gmail_msg"><br class="gmail-m_1772032661836158604gmail_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="gmail-m_1772032661836158604gmail_msg"><br class="gmail-m_1772032661836158604gmail_msg"></div>For instance AuditLogType.MAC_ADDRESS_IS_<wbr>EXTERNAL is being used from ImportVmCommandBase.java in the following way:<br class="gmail-m_1772032661836158604gmail_msg"><br class="gmail-m_1772032661836158604gmail_msg">private AuditLogableBase createExternalMacsAuditLog(VM vm, Set<String> externalMacs) {<br class="gmail-m_1772032661836158604gmail_msg"> AuditLogableBase logable = <b class="gmail-m_1772032661836158604gmail_msg">Injector.injectMembers</b>(new AuditLogableBase());<br class="gmail-m_1772032661836158604gmail_msg"> logable.setVmId(vm.getId());<br class="gmail-m_1772032661836158604gmail_msg"> logable.<wbr>setCustomCommaSeparatedValues(<wbr>"MACAddr", externalMacs);<br class="gmail-m_1772032661836158604gmail_msg"> return logable;<br class="gmail-m_1772032661836158604gmail_msg"> }<br class="gmail-m_1772032661836158604gmail_msg" clear="all"><div class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604gmail_msg"><br class="gmail-m_1772032661836158604gmail_msg">The entry in the properties file is:<br class="gmail-m_1772032661836158604gmail_msg">MAC_ADDRESS_IS_EXTERNAL=VM ${<b class="gmail-m_1772032661836158604gmail_msg">VmName</b>} has MAC address(es) ${MACAddr}, which is/are out of its MAC pool definitions.<br class="gmail-m_1772032661836158604gmail_msg"><br class="gmail-m_1772032661836158604gmail_msg"></div><div class="gmail-m_1772032661836158604gmail_msg">Therefore the only purpose of the injection is to allow the AuditLogDirector to resolve the ${<b class="gmail-m_1772032661836158604gmail_msg">VmName</b>} which is already known at the time of creating the AuditLogableBase entry.<br class="gmail-m_1772032661836158604gmail_msg"></div><div class="gmail-m_1772032661836158604gmail_msg"><br class="gmail-m_1772032661836158604gmail_msg"></div><div class="gmail-m_1772032661836158604gmail_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="gmail-m_1772032661836158604gmail_msg"></div><div class="gmail-m_1772032661836158604gmail_msg">This is just a wastef of injection and DB access while both can be spared.<br class="gmail-m_1772032661836158604gmail_msg"><br class="gmail-m_1772032661836158604gmail_msg"></div><div class="gmail-m_1772032661836158604gmail_msg">This could have been easily replaced by one of the following:<br class="gmail-m_1772032661836158604gmail_msg"><ul class="gmail-m_1772032661836158604gmail_msg"><li class="gmail-m_1772032661836158604gmail_msg">auditLogableBase.setVmName(vm.<wbr>getName());</li></ul></div></div></div></div></blockquote></span><div>- setVmName is protected so not usable as is <br></div></div></div></blockquote><div><br></div><div>It will become public if we agree on <br><br><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">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> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604gmail_msg"><ul class="gmail-m_1772032661836158604gmail_msg"><li class="gmail-m_1772032661836158604gmail_msg">auditLogableBase.<wbr>addCustomValue("VmName", vm.getName());</li></ul></div></div></div></div></blockquote><div>I prefer this, it is readable. and BTW it is fluent, it returns 'this' so use<br><br></div><div> AuditLogDirector(new AuditLogableBase(type)<br> .addCustomValue("VmName", vm.getName()));</div></div></div></blockquote><div><br></div><div>I'm okay with this as well.<br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span class="gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604gmail_msg"><p class="gmail-m_1772032661836158604gmail_msg">Please pick up any occurrence from your domain and send a patch to replace it where possible.<br class="gmail-m_1772032661836158604gmail_msg"></p></div><div class="gmail-m_1772032661836158604gmail_msg">Thanks in advance,<br class="gmail-m_1772032661836158604gmail_msg"><div class="gmail-m_1772032661836158604m_5996702152823124421gmail_signature gmail-m_1772032661836158604gmail_msg"><div dir="ltr" class="gmail-m_1772032661836158604gmail_msg">Moti<br class="gmail-m_1772032661836158604gmail_msg"></div></div></div></div></div></div></blockquote><div><br></div></span><div>+1<br><br></div><div>Frankly the fact that all the DAOs sets protected access in AuditLogableBase is a total abuse. Every command should declare its own deps. <br></div></div><span class="gmail-"><div class="gmail_quote"><br></div></span></div></blockquote><div><br></div><div>That will require a huge effort.<br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><span class="gmail-"><div class="gmail_quote"></div><div class="gmail_quote">______________________________<wbr>_________________<br class="gmail-m_1772032661836158604gmail_msg"></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Devel mailing list<br class="gmail-m_1772032661836158604gmail_msg">
<a href="mailto:Devel@ovirt.org" class="gmail-m_1772032661836158604gmail_msg" target="_blank">Devel@ovirt.org</a><br class="gmail-m_1772032661836158604gmail_msg">
<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" class="gmail-m_1772032661836158604gmail_msg" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a></blockquote></div></span></div>
</blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr"><div>Regards,<br></div>Moti<br></div></div>
</div></div>