<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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><span class=""><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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090gmail_msg">Hi All,<br class="m_-262973864516788090gmail_msg"><br class="m_-262973864516788090gmail_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_-262973864516788090gmail_msg"><br class="m_-262973864516788090gmail_msg"></div>For instance AuditLogType.MAC_ADDRESS_IS_<wbr>EXTERNAL is being used from ImportVmCommandBase.java in the following way:<br class="m_-262973864516788090gmail_msg"><br class="m_-262973864516788090gmail_msg">private AuditLogableBase createExternalMacsAuditLog(VM vm, Set<String> externalMacs) {<br class="m_-262973864516788090gmail_msg"> AuditLogableBase logable = <b class="m_-262973864516788090gmail_msg">Injector.injectMembers</b>(new AuditLogableBase());<br class="m_-262973864516788090gmail_msg"> logable.setVmId(vm.getId());<br class="m_-262973864516788090gmail_msg"> logable.<wbr>setCustomCommaSeparatedValues(<wbr>"MACAddr", externalMacs);<br class="m_-262973864516788090gmail_msg"> return logable;<br class="m_-262973864516788090gmail_msg"> }<br class="m_-262973864516788090gmail_msg" clear="all"><div class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090gmail_msg"><br class="m_-262973864516788090gmail_msg">The entry in the properties file is:<br class="m_-262973864516788090gmail_msg">MAC_ADDRESS_IS_EXTERNAL=VM ${<b class="m_-262973864516788090gmail_msg">VmName</b>} has MAC address(es) ${MACAddr}, which is/are out of its MAC pool definitions.<br class="m_-262973864516788090gmail_msg"><br class="m_-262973864516788090gmail_msg"></div><div class="m_-262973864516788090gmail_msg">Therefore the only purpose of the injection is to allow the AuditLogDirector to resolve the ${<b class="m_-262973864516788090gmail_msg">VmName</b>} which is already known at the time of creating the AuditLogableBase entry.<br class="m_-262973864516788090gmail_msg"></div><div class="m_-262973864516788090gmail_msg"><br class="m_-262973864516788090gmail_msg"></div><div class="m_-262973864516788090gmail_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_-262973864516788090gmail_msg"></div><div class="m_-262973864516788090gmail_msg">This is just a wastef of injection and DB access while both can be spared.<br class="m_-262973864516788090gmail_msg"><br class="m_-262973864516788090gmail_msg"></div><div class="m_-262973864516788090gmail_msg">This could have been easily replaced by one of the following:<br class="m_-262973864516788090gmail_msg"><ul class="m_-262973864516788090gmail_msg"><li class="m_-262973864516788090gmail_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><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090gmail_msg"><ul class="m_-262973864516788090gmail_msg"><li class="m_-262973864516788090gmail_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><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090gmail_msg"><p class="m_-262973864516788090gmail_msg">Please pick up any occurrence from your domain and send a patch to replace it where possible.<br class="m_-262973864516788090gmail_msg"></p></div><div class="m_-262973864516788090gmail_msg">Thanks in advance,<br class="m_-262973864516788090gmail_msg"><div class="m_-262973864516788090m_5996702152823124421gmail_signature m_-262973864516788090gmail_msg"><div dir="ltr" class="m_-262973864516788090gmail_msg">Moti<br class="m_-262973864516788090gmail_msg"></div></div></div></div></div></div></blockquote><div><br></div></span><div>+1<br></div></div></div></blockquote><div>+1 <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><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></div></blockquote><div><br></div><div>+100 Can't agree more with declaring fine grained dependencies on each bean/command. <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></div></div><div class="gmail_quote"><br></div><div class="gmail_quote">______________________________<wbr>_________________<br class="m_-262973864516788090gmail_msg"></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Devel mailing list<br class="m_-262973864516788090gmail_msg">
<a href="mailto:Devel@ovirt.org" class="m_-262973864516788090gmail_msg" target="_blank">Devel@ovirt.org</a><br class="m_-262973864516788090gmail_msg">
<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" class="m_-262973864516788090gmail_msg" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a></blockquote></div></div>
<br>______________________________<wbr>_________________<br>
Devel mailing list<br>
<a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a><br>
<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a><br></blockquote></div><br></div></div>