Hi Martin,

I'd like to address this with the same approach as suggested by Idan for Commands [1] .
  1. It preserves the support for postConstruct() which is implemented explicitly in your patch by setup() method.
  2. The suggested patch allows to define the classes as mutable - where the affect of it is unknown and probably not welcome.
  3. It blocks queries from any future treatment of the parameters are creation time.

Having a single c'tor which expects both parameters will solve that issue, same as done for the commands.


[1] https://gerrit.ovirt.org/#/c/52657/

On Tue, Jan 26, 2016 at 11:01 AM, Martin Mucha <mmucha@redhat.com> wrote:
Hi,

I got another bug about missing constructor:

public …(P parameters, EngineContext engineContext) {
        super(parameters, engineContext);
}

so I looked into:
org.ovirt.engine.core.bll.QueriesCommandBase

and it seems, that sole 'problem' is with initialization of user in @PostConstruct method. It seems, that we can easily set all those fields via setters, and user initialization can be done later, for example before calling executeQueryCommand in:
org.ovirt.engine.core.bll.QueriesCommandBase#executeCommand

doing this will move us closer to CDI and avoid errors because of forgotten constructor. Is there a obstacle blocking us from doing this?

thanks,
Mar.

Edit: I had some time left so I tried to implement it, and it seems to be working. It's big patch, but mostly there's just constructors removal. Actual changes are in:

QueriesCommandBase
CommandsFactory
RegisterVdsQuery
GetAllTemplateBasedEntityQuery
GetAllInstanceTypesQuery
GetAllVmTemplatesQuery
GetAllImageTypesQuery

and test files.
_______________________________________________
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel



--
Regards,
Moti