Liron Aravot has posted comments on this change.
Change subject: engine: consolidate userPortal vm and pool queries
......................................................................
Patch Set 1: (5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllVmsAndVmPoolsQuery.java
Line 13: public class GetAllVmsAndVmPoolsQuery<P extends
VdcQueryParametersBase> extends QueriesCommandBase<P> {
Line 14: public GetAllVmsAndVmPoolsQuery(P parameters) {
Line 15: super(parameters);
Line 16: }
Line 17:
1. basically the intention seems to save a backend call and perform two queries
at once if possible - does it really saves us that much?
2. I'd return a list of the result lists or if you prefer list of
vdcQueryReturnValue which will contain the queries results separately, it will
save instanceof checks and will let you just set the lists on the fronted
rather then iterate and check each element it's type
Line 18: @Override
Line 19: protected void executeQueryCommand() {
Line 20: boolean isSucceeded = true;
Line 21: List<Object> retValList = new ArrayList<Object>();
Line 17:
Line 18: @Override
Line 19: protected void executeQueryCommand() {
Line 20: boolean isSucceeded = true;
Line 21: List<Object> retValList = new ArrayList<Object>();
i'll be better to change to LinkedList
Line 22: VdcQueryReturnValue queryResult =
Backend.getInstance().runInternalQuery(VdcQueryType.GetAllVms, getParameters());
Line 23: if (queryResult != null && queryResult.getSucceeded()) {
Line 24: retValList.addAll((List<VM>) queryResult.getReturnValue());
Line 25: } else {
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalBasicListModel.java
Line 129: new AsyncQuery(this, new INewAsyncCallback() {
Line 130:
Line 131: @Override
Line 132: public void OnSuccess(Object model, Object
returnValue) {
Line 133: UserPortalBasicListModel
userPortalBasicListModel = (UserPortalBasicListModel) model;
general - can this method be in one place to avoid the need to maintain it in
two places?
Line 134: ArrayList<VM> vms = new ArrayList<VM>();
Line 135: ArrayList<vm_pools> pools = new
ArrayList<vm_pools>();
Line 136:
Line 137: VdcQueryReturnValue retValue =
(VdcQueryReturnValue) returnValue;
Line 136:
Line 137: VdcQueryReturnValue retValue =
(VdcQueryReturnValue) returnValue;
Line 138: if (retValue != null &&
retValue.getSucceeded()) {
Line 139: List<Object> list = (ArrayList<Object>)
retValue.getReturnValue();
Line 140: for (Object object : list) {
the list can be null..which will cause to NPE (GetAllVmsAndVmPoolsQuery line 40)
Line 141: if (object instanceof VM) {
Line 142: vms.add((VM) object);
Line 143: } else if (object instanceof
vm_pools) {
Line 144: pools.add((vm_pools) object);
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java
Line 355:
Line 356: VdcQueryReturnValue retValue =
(VdcQueryReturnValue) returnValue;
Line 357: if (retValue != null &&
retValue.getSucceeded()) {
Line 358: List<Object> list = (ArrayList<Object>)
retValue.getReturnValue();
Line 359: for (Object object : list) {
the list can be null..which will cause to NPE (GetAllVmsAndVmPoolsQuery line 40)
Line 360: if (object instanceof VM) {
Line 361: vms.add((VM) object);
Line 362: } else if (object instanceof
vm_pools) {
Line 363: pools.add((vm_pools) object);
--
To view, visit http://gerrit.ovirt.org/10475
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59346bc9f1cf6d948c4401ddbd7bb365c67b5b05
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: ofri masad <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
