Allon Mureinik has uploaded a new change for review.
Change subject: engine: Remove instanceof from GetDomainListQuery
......................................................................
engine: Remove instanceof from GetDomainListQuery
Runtime type checking is no way of passing parameters - if the internal
domain should not be filtered, we can just pass false.
This patch changes GetDomainListQuery to exclusively use the
GetDomainListParameters class instead of allowing
VdcQueryParameterBase to infer not filtering the internal domain.
In addition, this patch introduces a test for GetDomainListQuery to
ensure that no functionality was compromised.
Advantages of this approach:
1. Explicit is better than implicit - the code is more readable, and
its more obvious what the caller is doing.
2. Better performance - runtime type checking is expensive, where as
function calls are cheap.
Change-Id: I2e59b5053792e6a68030b8dcef604d9e5acdce80
Signed-off-by: Allon Mureinik <amure...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDomainListQuery.java
A
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetDomainListQueryTest.java
M
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDomainsResource.java
M
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceBase.java
M
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java
5 files changed, 63 insertions(+), 14 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/69/18869/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDomainListQuery.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDomainListQuery.java
index c70ed23..aaf1de8 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDomainListQuery.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDomainListQuery.java
@@ -5,21 +5,15 @@
import org.ovirt.engine.core.bll.adbroker.LdapBrokerUtils;
import org.ovirt.engine.core.common.queries.GetDomainListParameters;
-import org.ovirt.engine.core.common.queries.VdcQueryParametersBase;
-public class GetDomainListQuery<P extends VdcQueryParametersBase> extends
QueriesCommandBase<P> {
+public class GetDomainListQuery<P extends GetDomainListParameters> extends
QueriesCommandBase<P> {
public GetDomainListQuery(P parameters) {
super(parameters);
}
@Override
protected void executeQueryCommand() {
- boolean filterInternalDomain = false;
- // get concrete parameters object
- if (getParameters() instanceof GetDomainListParameters) {
- filterInternalDomain =
((GetDomainListParameters)getParameters()).getFilterInternalDomain();
- }
- List<String> domains =
LdapBrokerUtils.getDomainsList(filterInternalDomain);
+ List<String> domains =
LdapBrokerUtils.getDomainsList(getParameters().getFilterInternalDomain());
Collections.sort(domains);
getQueryReturnValue().setReturnValue(domains);
}
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetDomainListQueryTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetDomainListQueryTest.java
new file mode 100644
index 0000000..05ac392
--- /dev/null
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetDomainListQueryTest.java
@@ -0,0 +1,55 @@
+package org.ovirt.engine.core.bll;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.doReturn;
+import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig;
+
+import java.util.Arrays;
+import java.util.Collection;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.ovirt.engine.core.common.config.ConfigValues;
+import org.ovirt.engine.core.common.queries.GetDomainListParameters;
+import org.ovirt.engine.core.utils.MockConfigRule;
+
+/**
+ * A test case for the {@link GetDomainListQuery} class.
+ */
+public class GetDomainListQueryTest extends
AbstractQueryTest<GetDomainListParameters,
GetDomainListQuery<GetDomainListParameters>> {
+
+ @ClassRule
+ public static final MockConfigRule mcr = new MockConfigRule(
+ mockConfig(ConfigValues.AdminDomain, "internal"),
+ mockConfig(ConfigValues.DomainName, "zzz,aaa")
+ );
+
+ @Test
+ public void testImplicitNoFilter() {
+ getQuery().executeQueryCommand();
+ assertTrue("Wrong filtered domains", CollectionUtils.isEqualCollection(
+ (Collection<String>)
getQuery().getQueryReturnValue().getReturnValue(),
+ Arrays.asList("aaa", "internal", "zzz")));
+ }
+
+ @Test
+ public void testFilter() {
+ doReturn(true).when(getQueryParameters()).getFilterInternalDomain();
+ getQuery().executeQueryCommand();
+ assertTrue("Wrong filtered domains", CollectionUtils.isEqualCollection(
+ (Collection<String>)
getQuery().getQueryReturnValue().getReturnValue(),
+ Arrays.asList("aaa", "zzz")));
+ }
+
+ @Test
+ public void testExplicitNoFilter() {
+ doReturn(false).when(getQueryParameters()).getFilterInternalDomain();
+ getQuery().executeQueryCommand();
+ assertTrue("Wrong filtered domains", CollectionUtils.isEqualCollection(
+ (Collection<String>)
getQuery().getQueryReturnValue().getReturnValue(),
+ Arrays.asList("aaa", "internal", "zzz")));
+ }
+}
+
+
diff --git
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDomainsResource.java
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDomainsResource.java
index e549310..9f74bbe 100644
---
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDomainsResource.java
+++
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDomainsResource.java
@@ -12,7 +12,7 @@
import org.ovirt.engine.api.resource.DomainResource;
import org.ovirt.engine.api.resource.DomainsResource;
import org.ovirt.engine.api.restapi.model.Directory;
-import org.ovirt.engine.core.common.queries.VdcQueryParametersBase;
+import org.ovirt.engine.core.common.queries.GetDomainListParameters;
import org.ovirt.engine.core.common.queries.VdcQueryType;
import org.ovirt.engine.core.compat.Guid;
@@ -62,7 +62,7 @@
return getBackendCollection(
String.class,
VdcQueryType.GetDomainList,
- new VdcQueryParametersBase());
+ new GetDomainListParameters());
}
public Domain lookupDirectoryById(String id, boolean addlinks) {
diff --git
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceBase.java
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceBase.java
index a202294..3f6500c 100644
---
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceBase.java
+++
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceBase.java
@@ -19,8 +19,8 @@
import org.ovirt.engine.core.common.businessentities.DbUser;
import org.ovirt.engine.core.common.businessentities.LdapUser;
import org.ovirt.engine.core.common.interfaces.SearchType;
+import org.ovirt.engine.core.common.queries.GetDomainListParameters;
import org.ovirt.engine.core.common.queries.IdQueryParameters;
-import org.ovirt.engine.core.common.queries.VdcQueryParametersBase;
import org.ovirt.engine.core.common.queries.VdcQueryType;
import org.ovirt.engine.core.common.users.VdcUser;
import org.ovirt.engine.core.compat.Guid;
@@ -85,7 +85,7 @@
List<String> domains = getBackendCollection(
String.class,
VdcQueryType.GetDomainList,
- new VdcQueryParametersBase());
+ new GetDomainListParameters());
for (String domain :domains) {
Guid domainId = new Guid(domain.getBytes(), true);
if (domainId.toString().equals(user.getDomain().getId())) {
diff --git
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java
index 0ca4317..25d5172 100644
---
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java
+++
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceTest.java
@@ -23,9 +23,9 @@
import org.ovirt.engine.core.common.businessentities.DbUser;
import org.ovirt.engine.core.common.businessentities.LdapUser;
import org.ovirt.engine.core.common.interfaces.SearchType;
+import org.ovirt.engine.core.common.queries.GetDomainListParameters;
import org.ovirt.engine.core.common.queries.IdQueryParameters;
import org.ovirt.engine.core.common.queries.SearchParameters;
-import org.ovirt.engine.core.common.queries.VdcQueryParametersBase;
import org.ovirt.engine.core.common.queries.VdcQueryReturnValue;
import org.ovirt.engine.core.common.queries.VdcQueryType;
import org.ovirt.engine.core.compat.Guid;
@@ -134,7 +134,7 @@
@Test
public void testAddUser_4() throws Exception {
setUpEntityQueryExpectations(VdcQueryType.GetDomainList,
- VdcQueryParametersBase.class,
+ GetDomainListParameters.class,
new String[] { },
new Object[] { },
setUpDomains());
--
To view, visit http://gerrit.ovirt.org/18869
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2e59b5053792e6a68030b8dcef604d9e5acdce80
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
