Allon Mureinik has uploaded a new change for review.

Change subject: core: remove SearchReturnValue
......................................................................

core: remove SearchReturnValue

SearchReturnValue extends VdcQueryReturnValue and adds the boolean
isSearchValid. However, this boolean is only set in various flows, and
never read, meaning this entire class is useless.

This patch does the following:
1. Removes SearchReturnValue
2. Removes QueriesCommandBase.createReturnValue() and replaces it with
   an inline call to new VdsQueryReturnValue(), since now there is no
   reason to override it.
3. Removes QueriesCommandBase.proceedOnFail(), since now no class
   implements it.

Change-Id: I39fd1f1fb4bf66e594ea3157bd36959736faa6dc
Signed-off-by: Allon Mureinik <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/QueriesCommandBase.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/LdapSearchQueryTestBase.java
D 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/SearchReturnValue.java
4 files changed, 7 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/10/15010/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/QueriesCommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/QueriesCommandBase.java
index 34905ac..630f3b1 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/QueriesCommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/QueriesCommandBase.java
@@ -27,13 +27,6 @@
     private final static Validator validator = 
Validation.buildDefaultValidatorFactory().getValidator();
     private static final String QuerySuffix = "Query";
 
-    /**
-     * Factory to determine the type of the ReturnValue field
-     */
-    protected VdcQueryReturnValue createReturnValue() {
-        return new VdcQueryReturnValue();
-    }
-
     // get correct return value type
     private final VdcQueryReturnValue returnValue;
     private final VdcQueryType queryType;
@@ -43,7 +36,7 @@
 
     public QueriesCommandBase(P parameters) {
         this.parameters = parameters;
-        returnValue = createReturnValue();
+        returnValue = new VdcQueryReturnValue();
         queryType = initQueryType();
         user = initUser();
     }
@@ -141,10 +134,6 @@
             return false;
         }
         return true;
-    }
-
-    protected void proceedOnFail() {
-        // Empty default implementation, method is here to allow inheriting 
classes to override.
     }
 
     public VdcQueryReturnValue getQueryReturnValue() {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java
index 3840ed6..db957d8 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java
@@ -15,20 +15,20 @@
 import org.ovirt.engine.core.bll.adbroker.LdapReturnValueBase;
 import org.ovirt.engine.core.bll.adbroker.LdapSearchByQueryParameters;
 import org.ovirt.engine.core.bll.quota.QuotaManager;
-import org.ovirt.engine.core.common.businessentities.LdapUser;
 import org.ovirt.engine.core.common.businessentities.AuditLog;
 import org.ovirt.engine.core.common.businessentities.DbUser;
 import org.ovirt.engine.core.common.businessentities.Disk;
 import org.ovirt.engine.core.common.businessentities.IVdcQueryable;
+import org.ovirt.engine.core.common.businessentities.LdapGroup;
+import org.ovirt.engine.core.common.businessentities.LdapUser;
 import org.ovirt.engine.core.common.businessentities.Quota;
+import org.ovirt.engine.core.common.businessentities.StorageDomain;
+import org.ovirt.engine.core.common.businessentities.StoragePool;
 import org.ovirt.engine.core.common.businessentities.VDS;
 import org.ovirt.engine.core.common.businessentities.VDSGroup;
 import org.ovirt.engine.core.common.businessentities.VM;
-import org.ovirt.engine.core.common.businessentities.VmTemplate;
-import org.ovirt.engine.core.common.businessentities.LdapGroup;
-import org.ovirt.engine.core.common.businessentities.StorageDomain;
-import org.ovirt.engine.core.common.businessentities.StoragePool;
 import org.ovirt.engine.core.common.businessentities.VmPool;
+import org.ovirt.engine.core.common.businessentities.VmTemplate;
 import 
org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeEntity;
 import org.ovirt.engine.core.common.businessentities.network.NetworkView;
 import org.ovirt.engine.core.common.config.Config;
@@ -36,7 +36,6 @@
 import 
org.ovirt.engine.core.common.errors.SearchEngineIllegalCharacterException;
 import org.ovirt.engine.core.common.errors.SqlInjectionException;
 import org.ovirt.engine.core.common.queries.SearchParameters;
-import org.ovirt.engine.core.common.queries.SearchReturnValue;
 import org.ovirt.engine.core.common.utils.ListUtils;
 import org.ovirt.engine.core.common.utils.ListUtils.Filter;
 import org.ovirt.engine.core.compat.DateTime;
@@ -53,21 +52,6 @@
 
     public SearchQuery(P parameters) {
         super(parameters);
-    }
-
-    @Override
-    protected SearchReturnValue createReturnValue() {
-        return new SearchReturnValue();
-    }
-
-    @Override
-    public SearchReturnValue getQueryReturnValue() {
-        return (SearchReturnValue) super.getQueryReturnValue();
-    }
-
-    @Override
-    protected void proceedOnFail() {
-        getQueryReturnValue().setIsSearchValid(false);
     }
 
     @Override
@@ -284,7 +268,6 @@
             String searchText = getParameters().getSearchPattern();
             // find if this is a trivial search expression (like 'Vms:' etc).
             isSafe = SearchObjects.isSafeExpression(searchText);
-            getQueryReturnValue().setIsSearchValid(true);
             if (useCache) {
                 // first lets check the cache of queries.
                 searchKey = String.format("%1$s,%2$s,%3$s", searchText, 
getParameters().getMaxCount(),getParameters().getCaseSensitive());
@@ -332,7 +315,6 @@
                 searchObj.setSearchFrom(getParameters().getSearchFrom());
 
                 if (searchObj.getError() != SyntaxError.NO_ERROR) {
-                    getQueryReturnValue().setIsSearchValid(false);
                     log.warnFormat("ResourceManager::searchBusinessObjects - 
erroneous search text - ''{0}''",
                             searchText);
                     int startPos = searchObj.getErrorStartPos();
@@ -353,7 +335,6 @@
                     return null;
                 }
                 if (searchObj.getvalid() != true) {
-                    getQueryReturnValue().setIsSearchValid(false);
                     log.warnFormat("ResourceManager::searchBusinessObjects - 
Invalid search text - ''{0}''", searchText);
                     return null;
                 }
@@ -373,15 +354,12 @@
         } catch (SearchEngineIllegalCharacterException e) {
             log.error("Search expression can not end with ESCAPE character:" + 
getParameters().getSearchPattern());
             data = null;
-            getQueryReturnValue().setIsSearchValid(false);
         } catch (SqlInjectionException e) {
             log.error("Sql Injection in search: " + 
getParameters().getSearchPattern());
             data = null;
-            getQueryReturnValue().setIsSearchValid(false);
         } catch (RuntimeException ex) {
             log.warn("Illegal search: " + getParameters().getSearchPattern(), 
ex);
             data = null;
-            getQueryReturnValue().setIsSearchValid(false);
         }
         return data;
     }
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/LdapSearchQueryTestBase.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/LdapSearchQueryTestBase.java
index d4b5105..e679b72 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/LdapSearchQueryTestBase.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/LdapSearchQueryTestBase.java
@@ -97,7 +97,7 @@
         getQuery().setInternalExecution(true);
         getQuery().execute();
         assertTrue("Query should succeed, but failed with: " + 
getQuery().getQueryReturnValue().getExceptionString(),
-                getQuery().getQueryReturnValue().getIsSearchValid());
+                getQuery().getQueryReturnValue().getSucceeded());
         assertEquals("Wrong ldap result returned",
                 result,
                 ((List<IVdcQueryable>) 
getQuery().getQueryReturnValue().getReturnValue()).get(0));
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/SearchReturnValue.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/SearchReturnValue.java
deleted file mode 100644
index 858efdf..0000000
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/SearchReturnValue.java
+++ /dev/null
@@ -1,18 +0,0 @@
-package org.ovirt.engine.core.common.queries;
-
-public class SearchReturnValue extends VdcQueryReturnValue {
-    private static final long serialVersionUID = -3741619784123689926L;
-
-    private boolean _validSearch;
-
-    public boolean getIsSearchValid() {
-        return _validSearch;
-    }
-
-    public void setIsSearchValid(boolean value) {
-        _validSearch = value;
-    }
-
-    public SearchReturnValue() {
-    }
-}


--
To view, visit http://gerrit.ovirt.org/15010
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I39fd1f1fb4bf66e594ea3157bd36959736faa6dc
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to