You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by js...@apache.org on 2016/03/28 19:10:02 UTC
aurora git commit: Treat empty and null collections equivalently in
task queries.
Repository: aurora
Updated Branches:
refs/heads/master 83a078b6b -> 095009596
Treat empty and null collections equivalently in task queries.
Previously, `null` was handled differently from an empty collection in
task queries. For the Go thrift bindings, this was problematic since
zero values in Go are useful in almost all cases and in particular in the
case of maps (used to represent sets). In these cases unset `TaskQuery`
collection parameters are serialized as empty collections (empty
maps) instead of `nil` (`null`), leading to the inability to use the
query API in any natural way.
Reviewed at https://reviews.apache.org/r/45193/
Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/09500959
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/09500959
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/09500959
Branch: refs/heads/master
Commit: 0950095966ee73ef73cff0bd1a8b2aa128e7b385
Parents: 83a078b
Author: John Sirois <js...@apache.org>
Authored: Mon Mar 28 11:11:36 2016 -0600
Committer: John Sirois <jo...@gmail.com>
Committed: Mon Mar 28 11:11:36 2016 -0600
----------------------------------------------------------------------
.../apache/aurora/scheduler/base/JobKeys.java | 8 +++-----
.../org/apache/aurora/scheduler/base/Query.java | 19 +++++++++----------
.../aurora/scheduler/storage/TaskStore.java | 15 +++++++--------
.../scheduler/storage/db/DbTaskStore.java | 2 +-
.../aurora/scheduler/storage/db/TaskMapper.java | 4 ++--
.../scheduler/storage/mem/MemTaskStore.java | 12 +++++++-----
.../scheduler/thrift/ReadOnlySchedulerImpl.java | 12 ++++++++----
.../thrift/SchedulerThriftInterface.java | 2 +-
.../aurora/scheduler/storage/db/TaskMapper.xml | 10 +++++-----
.../http/api/security/HttpSecurityIT.java | 18 ++++++++++--------
.../storage/AbstractTaskStoreTest.java | 20 +++++++++++++++++---
.../thrift/ReadOnlySchedulerImplTest.java | 6 +++---
.../thrift/SchedulerThriftInterfaceTest.java | 20 ++++++++++----------
13 files changed, 83 insertions(+), 65 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/main/java/org/apache/aurora/scheduler/base/JobKeys.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/base/JobKeys.java b/src/main/java/org/apache/aurora/scheduler/base/JobKeys.java
index 8f5bf58..7bbb1f7 100644
--- a/src/main/java/org/apache/aurora/scheduler/base/JobKeys.java
+++ b/src/main/java/org/apache/aurora/scheduler/base/JobKeys.java
@@ -23,9 +23,9 @@ import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableSet;
import org.apache.aurora.gen.JobKey;
-import org.apache.aurora.gen.TaskQuery;
import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
import org.apache.aurora.scheduler.storage.entities.IJobKey;
+import org.apache.aurora.scheduler.storage.entities.ITaskQuery;
import static com.google.common.base.Preconditions.checkArgument;
@@ -123,7 +123,7 @@ public final class JobKeys {
*/
public static Optional<Set<IJobKey>> from(Query.Builder query) {
if (Query.isJobScoped(query)) {
- TaskQuery taskQuery = query.get();
+ ITaskQuery taskQuery = query.get();
ImmutableSet.Builder<IJobKey> builder = ImmutableSet.builder();
if (taskQuery.isSetJobName()) {
@@ -133,9 +133,7 @@ public final class JobKeys {
taskQuery.getJobName()));
}
- if (taskQuery.isSetJobKeys()) {
- builder.addAll(IJobKey.setFromBuilders(taskQuery.getJobKeys()));
- }
+ builder.addAll(taskQuery.getJobKeys());
return Optional.of(assertValid(builder.build()));
} else {
return Optional.absent();
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/main/java/org/apache/aurora/scheduler/base/Query.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/base/Query.java b/src/main/java/org/apache/aurora/scheduler/base/Query.java
index ee01eaa..c76b365 100644
--- a/src/main/java/org/apache/aurora/scheduler/base/Query.java
+++ b/src/main/java/org/apache/aurora/scheduler/base/Query.java
@@ -25,6 +25,7 @@ import org.apache.aurora.gen.ScheduleStatus;
import org.apache.aurora.gen.TaskQuery;
import org.apache.aurora.scheduler.storage.entities.IInstanceKey;
import org.apache.aurora.scheduler.storage.entities.IJobKey;
+import org.apache.aurora.scheduler.storage.entities.ITaskQuery;
import static java.util.Objects.requireNonNull;
@@ -50,8 +51,8 @@ public final class Query {
* @return {@code true} if the query specifies at least one job key, otherwise {@code false}.
*/
public static boolean isJobScoped(Builder taskQuery) {
- TaskQuery q = taskQuery.get();
- return q.isSetRole() && q.isSetEnvironment() && q.isSetJobName() || q.isSetJobKeys();
+ ITaskQuery q = taskQuery.get();
+ return q.isSetRole() && q.isSetEnvironment() && q.isSetJobName() || !q.getJobKeys().isEmpty();
}
public static Builder arbitrary(TaskQuery query) {
@@ -123,11 +124,11 @@ public final class Query {
*
* TODO(ksweeney): Add an environment scope.
*/
- public static final class Builder implements Supplier<TaskQuery> {
+ public static final class Builder implements Supplier<ITaskQuery> {
private final TaskQuery query;
Builder() {
- this.query = new TaskQuery();
+ this(new TaskQuery());
}
Builder(TaskQuery query) {
@@ -143,20 +144,18 @@ public final class Query {
* @return A new TaskQuery satisfying this builder's constraints.
*/
@Override
- public TaskQuery get() {
- return query.deepCopy();
+ public ITaskQuery get() {
+ return ITaskQuery.build(query);
}
@Override
public boolean equals(Object that) {
- return that != null
- && that instanceof Builder
- && Objects.equals(query, ((Builder) that).query);
+ return that instanceof Builder && get().equals(((Builder) that).get());
}
@Override
public int hashCode() {
- return Objects.hash(query);
+ return Objects.hash(get());
}
@Override
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java b/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
index ac0bb37..3be2993 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
@@ -19,12 +19,12 @@ import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
-import org.apache.aurora.gen.TaskQuery;
import org.apache.aurora.scheduler.base.Query;
import org.apache.aurora.scheduler.base.Tasks;
import org.apache.aurora.scheduler.storage.entities.IJobKey;
import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
+import org.apache.aurora.scheduler.storage.entities.ITaskQuery;
import static com.google.common.base.CharMatcher.WHITESPACE;
@@ -124,7 +124,7 @@ public interface TaskStore {
public static Predicate<IScheduledTask> queryFilter(final Query.Builder queryBuilder) {
return task -> {
- TaskQuery query = queryBuilder.get();
+ ITaskQuery query = queryBuilder.get();
ITaskConfig config = task.getAssignedTask().getTask();
// TODO(wfarner): Investigate why blank inputs are treated specially for the role field.
if (query.getRole() != null
@@ -140,22 +140,21 @@ public interface TaskStore {
return false;
}
- if (query.getJobKeysSize() > 0
- && !query.getJobKeys().contains(config.getJob().newBuilder())) {
+ if (!query.getJobKeys().isEmpty() && !query.getJobKeys().contains(config.getJob())) {
return false;
}
- if (query.getTaskIds() != null && !query.getTaskIds().contains(Tasks.id(task))) {
+ if (!query.getTaskIds().isEmpty() && !query.getTaskIds().contains(Tasks.id(task))) {
return false;
}
- if (query.getStatusesSize() > 0 && !query.getStatuses().contains(task.getStatus())) {
+ if (!query.getStatuses().isEmpty() && !query.getStatuses().contains(task.getStatus())) {
return false;
}
- if (query.getSlaveHostsSize() > 0
+ if (!query.getSlaveHosts().isEmpty()
&& !query.getSlaveHosts().contains(task.getAssignedTask().getSlaveHost())) {
return false;
}
- if (query.getInstanceIdsSize() > 0
+ if (!query.getInstanceIds().isEmpty()
&& !query.getInstanceIds().contains(task.getAssignedTask().getInstanceId())) {
return false;
}
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java b/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
index 078dd8b..b9ed417 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
@@ -209,7 +209,7 @@ class DbTaskStore implements TaskStore.Mutable {
private FluentIterable<IScheduledTask> matches(Query.Builder query) {
Iterable<DbScheduledTask> results;
Predicate<IScheduledTask> filter;
- if (query.get().getTaskIdsSize() == 1) {
+ if (query.get().getTaskIds().size() == 1) {
// Optimize queries that are scoped to a single task, as the dynamic SQL used for arbitrary
// queries comes with a performance penalty.
results = Optional.fromNullable(
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java b/src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
index 4bf4004..ea12165 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
@@ -20,10 +20,10 @@ import javax.annotation.Nullable;
import org.apache.aurora.common.collections.Pair;
import org.apache.aurora.gen.JobKey;
-import org.apache.aurora.gen.TaskQuery;
import org.apache.aurora.scheduler.storage.db.views.DbScheduledTask;
import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
import org.apache.aurora.scheduler.storage.entities.ITaskEvent;
+import org.apache.aurora.scheduler.storage.entities.ITaskQuery;
import org.apache.ibatis.annotations.Param;
/**
@@ -47,7 +47,7 @@ interface TaskMapper {
* @param query Query to use as a filter for tasks.
* @return Tasks matching the query.
*/
- List<DbScheduledTask> select(TaskQuery query);
+ List<DbScheduledTask> select(ITaskQuery query);
/**
* Gets a task by ID.
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
index 231a556..fc272dd 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
@@ -81,7 +81,9 @@ class MemTaskStore implements TaskStore.Mutable {
private static final Function<Query.Builder, Optional<Set<IJobKey>>> QUERY_TO_JOB_KEY =
JobKeys::from;
private static final Function<Query.Builder, Optional<Set<String>>> QUERY_TO_SLAVE_HOST =
- query -> Optional.fromNullable(query.get().getSlaveHosts());
+ query -> query.get().getSlaveHosts().isEmpty()
+ ? Optional.absent()
+ : Optional.of(query.get().getSlaveHosts());
// Since this class operates under the API and umbrella of {@link Storage}, it is expected to be
// thread-safe but not necessarily strongly-consistent unless the externally-controlled storage
@@ -263,10 +265,7 @@ class MemTaskStore implements TaskStore.Mutable {
private FluentIterable<IScheduledTask> matches(Query.Builder query) {
// Apply the query against the working set.
Optional<? extends Iterable<Task>> from = Optional.absent();
- if (query.get().isSetTaskIds()) {
- taskQueriesById.incrementAndGet();
- from = Optional.of(fromIdIndex(query.get().getTaskIds()));
- } else {
+ if (query.get().getTaskIds().isEmpty()) {
for (SecondaryIndex<?> index : secondaryIndices) {
Optional<Iterable<String>> indexMatch = index.getMatches(query);
if (indexMatch.isPresent()) {
@@ -283,6 +282,9 @@ class MemTaskStore implements TaskStore.Mutable {
taskQueriesAll.incrementAndGet();
from = Optional.of(tasks.values());
}
+ } else {
+ taskQueriesById.incrementAndGet();
+ from = Optional.of(fromIdIndex(query.get().getTaskIds()));
}
return FluentIterable.from(from.get())
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java b/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
index d326d24..113af6a 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
@@ -178,9 +178,13 @@ class ReadOnlySchedulerImpl implements ReadOnlyScheduler.Iface {
public Response getPendingReason(TaskQuery query) throws TException {
requireNonNull(query);
- if (query.isSetSlaveHosts() || query.isSetStatuses()) {
+ if (query.isSetSlaveHosts() && !query.getSlaveHosts().isEmpty()) {
return invalidRequest(
- "Statuses or slaveHosts are not supported in " + query.toString());
+ "SlaveHosts are not supported in " + query.toString());
+ }
+ if (query.isSetStatuses() && !query.getStatuses().isEmpty()) {
+ return invalidRequest(
+ "Statuses is not supported in " + query.toString());
}
// Only PENDING tasks should be considered.
@@ -383,10 +387,10 @@ class ReadOnlySchedulerImpl implements ReadOnlyScheduler.Iface {
requireNonNull(query);
Iterable<IScheduledTask> tasks = Storage.Util.fetchTasks(storage, Query.arbitrary(query));
- if (query.isSetOffset()) {
+ if (query.getOffset() > 0) {
tasks = Iterables.skip(tasks, query.getOffset());
}
- if (query.isSetLimit()) {
+ if (query.getLimit() > 0) {
tasks = Iterables.limit(tasks, query.getLimit());
}
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
index 5d246be..7eda474 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
@@ -425,7 +425,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
private static Query.Builder implicitKillQuery(Query.Builder query) {
// Unless statuses were specifically supplied, only attempt to kill active tasks.
- return query.get().isSetStatuses() ? query : query.byStatus(ACTIVE_STATES);
+ return query.get().getStatuses().isEmpty() ? query.byStatus(ACTIVE_STATES) : query;
}
@Override
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
----------------------------------------------------------------------
diff --git a/src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml b/src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
index 684614f..0219bf3 100644
--- a/src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
+++ b/src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
@@ -124,35 +124,35 @@
<if test="jobName != null">
AND j.name = #{jobName}
</if>
- <if test="taskIds != null">
+ <if test="!taskIds.isEmpty()">
AND t.task_id IN (
<foreach item="task_id" collection="taskIds" separator=",">
#{task_id}
</foreach>
)
</if>
- <if test="statuses != null and !statuses.isEmpty()">
+ <if test="!statuses.isEmpty()">
AND t.status IN (
<foreach item="element" collection="statuses" separator=",">
#{element, typeHandler=org.apache.aurora.scheduler.storage.db.typehandlers.ScheduleStatusTypeHandler}
</foreach>
)
</if>
- <if test="instanceIds != null and !instanceIds.isEmpty()">
+ <if test="!instanceIds.isEmpty()">
AND t.instance_id IN (
<foreach item="instance_id" collection="instanceIds" separator=",">
#{instance_id}
</foreach>
)
</if>
- <if test="slaveHosts != null">
+ <if test="!slaveHosts.isEmpty()">
AND h.host IN (
<foreach item="host" collection="slaveHosts" separator=",">
#{host}
</foreach>
)
</if>
- <if test="jobKeys != null">
+ <if test="!jobKeys.isEmpty()">
AND (
<foreach item="jobKey" collection="jobKeys" open="(" separator=") OR (" close=")">
j.role = #{jobKey.role}
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java b/src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
index dfe94d3..dbec26f 100644
--- a/src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
@@ -15,6 +15,7 @@ package org.apache.aurora.scheduler.http.api.security;
import java.io.IOException;
import java.util.Set;
+
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
@@ -37,13 +38,13 @@ import org.apache.aurora.gen.JobKey;
import org.apache.aurora.gen.Lock;
import org.apache.aurora.gen.Response;
import org.apache.aurora.gen.ResponseCode;
-import org.apache.aurora.gen.TaskQuery;
import org.apache.aurora.scheduler.base.JobKeys;
import org.apache.aurora.scheduler.base.Query;
import org.apache.aurora.scheduler.http.AbstractJettyTest;
import org.apache.aurora.scheduler.http.H2ConsoleModule;
import org.apache.aurora.scheduler.http.api.ApiModule;
import org.apache.aurora.scheduler.storage.entities.IJobKey;
+import org.apache.aurora.scheduler.storage.entities.ITaskQuery;
import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
import org.apache.aurora.scheduler.thrift.aop.MockDecoratedThrift;
import org.apache.http.HttpResponse;
@@ -240,9 +241,9 @@ public class HttpSecurityIT extends AbstractJettyTest {
expect(auroraAdmin.killTasks(null, new Lock().setMessage("2"), null, null)).andReturn(OK);
JobKey job = JobKeys.from("role", "env", "name").newBuilder();
- TaskQuery jobScopedQuery = Query.jobScoped(IJobKey.build(job)).get();
- TaskQuery adsScopedQuery = Query.jobScoped(ADS_STAGING_JOB).get();
- expect(auroraAdmin.killTasks(adsScopedQuery, null, null, null)).andReturn(OK);
+ ITaskQuery jobScopedQuery = Query.jobScoped(IJobKey.build(job)).get();
+ ITaskQuery adsScopedQuery = Query.jobScoped(ADS_STAGING_JOB).get();
+ expect(auroraAdmin.killTasks(adsScopedQuery.newBuilder(), null, null, null)).andReturn(OK);
expect(auroraAdmin.killTasks(null, null, ADS_STAGING_JOB.newBuilder(), null)).andReturn(OK);
expectShiroAfterAuthFilter().times(24);
@@ -262,7 +263,7 @@ public class HttpSecurityIT extends AbstractJettyTest {
assertEquals(
ResponseCode.AUTH_FAILED,
getAuthenticatedClient(UNPRIVILEGED)
- .killTasks(jobScopedQuery, null, null, null)
+ .killTasks(jobScopedQuery.newBuilder(), null, null, null)
.getResponseCode());
assertEquals(
ResponseCode.AUTH_FAILED,
@@ -275,7 +276,7 @@ public class HttpSecurityIT extends AbstractJettyTest {
assertEquals(
ResponseCode.AUTH_FAILED,
getAuthenticatedClient(BACKUP_SERVICE)
- .killTasks(jobScopedQuery, null, null, null)
+ .killTasks(jobScopedQuery.newBuilder(), null, null, null)
.getResponseCode());
assertEquals(
ResponseCode.AUTH_FAILED,
@@ -285,7 +286,7 @@ public class HttpSecurityIT extends AbstractJettyTest {
assertEquals(
ResponseCode.AUTH_FAILED,
getAuthenticatedClient(DEPLOY_SERVICE)
- .killTasks(jobScopedQuery, null, null, null)
+ .killTasks(jobScopedQuery.newBuilder(), null, null, null)
.getResponseCode());
assertEquals(
ResponseCode.AUTH_FAILED,
@@ -294,7 +295,8 @@ public class HttpSecurityIT extends AbstractJettyTest {
.getResponseCode());
assertEquals(
OK,
- getAuthenticatedClient(DEPLOY_SERVICE).killTasks(adsScopedQuery, null, null, null));
+ getAuthenticatedClient(DEPLOY_SERVICE)
+ .killTasks(adsScopedQuery.newBuilder(), null, null, null));
assertEquals(
OK,
getAuthenticatedClient(DEPLOY_SERVICE).killTasks(
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java b/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
index e56fed2..5895afa 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
@@ -193,16 +193,30 @@ public abstract class AbstractTaskStoreTest extends TearDownTestCase {
assertQueryResults(Query.jobScoped(JobKeys.from("role-b", "env-b", "job-b")).active(), TASK_B);
assertQueryResults(Query.jobScoped(JobKeys.from("role-b", "devel", "job-b")).active());
- // Explicitly call out the current differing behaviors for types of empty query conditions.
- // Specifically - null task IDs and empty task IDs are different than other 'IN' conditions..
+ // Explicitly call out the matching behaviors for different means of expressing empty query
+ // conditions. Specifically - neither null collections nor empty collections should trigger
+ // query restrictions and thus they should yield the same results.
assertQueryResults(new TaskQuery().setTaskIds(null), TASK_A, TASK_B, TASK_C, TASK_D);
- assertQueryResults(new TaskQuery().setTaskIds(ImmutableSet.of()));
+ assertQueryResults(new TaskQuery().setTaskIds(ImmutableSet.of()),
+ TASK_A, TASK_B, TASK_C, TASK_D);
+
+ assertQueryResults(new TaskQuery().setInstanceIds(null), TASK_A, TASK_B, TASK_C, TASK_D);
assertQueryResults(
new TaskQuery().setInstanceIds(ImmutableSet.of()),
TASK_A, TASK_B, TASK_C, TASK_D);
+
+ assertQueryResults(new TaskQuery().setStatuses(null), TASK_A, TASK_B, TASK_C, TASK_D);
assertQueryResults(
new TaskQuery().setStatuses(ImmutableSet.of()),
TASK_A, TASK_B, TASK_C, TASK_D);
+
+ assertQueryResults(new TaskQuery().setSlaveHosts(null), TASK_A, TASK_B, TASK_C, TASK_D);
+ assertQueryResults(new TaskQuery().setSlaveHosts(ImmutableSet.of()),
+ TASK_A, TASK_B, TASK_C, TASK_D);
+
+ assertQueryResults(new TaskQuery().setJobKeys(null), TASK_A, TASK_B, TASK_C, TASK_D);
+ assertQueryResults(new TaskQuery().setJobKeys(ImmutableSet.of()),
+ TASK_A, TASK_B, TASK_C, TASK_D);
}
@Test
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
index 3ba0342..fcb5c22 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
@@ -290,7 +290,7 @@ public class ReadOnlySchedulerImplTest extends EasyMockTest {
new PendingReason().setTaskId(taskId1).setReason(reason),
new PendingReason().setTaskId(taskId2).setReason(reason));
- Response response = assertOkResponse(thrift.getPendingReason(query.get()));
+ Response response = assertOkResponse(thrift.getPendingReason(query.get().newBuilder()));
assertEquals(expected, response.getResult().getGetPendingReasonResult().getReasons());
}
@@ -371,7 +371,7 @@ public class ReadOnlySchedulerImplTest extends EasyMockTest {
control.replay();
- assertResponse(INVALID_REQUEST, thrift.getPendingReason(query.get()));
+ assertResponse(INVALID_REQUEST, thrift.getPendingReason(query.get().newBuilder()));
}
@Test
@@ -586,7 +586,7 @@ public class ReadOnlySchedulerImplTest extends EasyMockTest {
control.replay();
- assertResponse(INVALID_REQUEST, thrift.getPendingReason(query.get()));
+ assertResponse(INVALID_REQUEST, thrift.getPendingReason(query.get().newBuilder()));
}
@Test
http://git-wip-us.apache.org/repos/asf/aurora/blob/09500959/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
index 0a7b518..85ca86c 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
@@ -599,22 +599,22 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
@Test
public void testKillByJobName() throws Exception {
- TaskQuery query = new TaskQuery().setJobName("job");
- storageUtil.expectTaskFetch(Query.arbitrary(query).active(), buildScheduledTask());
+ Query.Builder query = Query.arbitrary(new TaskQuery().setJobName("job")).active();
+ storageUtil.expectTaskFetch(query, buildScheduledTask());
lockManager.validateIfLocked(LOCK_KEY, java.util.Optional.empty());
expectTransitionsToKilling();
control.replay();
- Response response = thrift.killTasks(query, null, null, null);
+ Response response = thrift.killTasks(query.get().newBuilder(), null, null, null);
assertOkResponse(response);
assertMessageMatches(response, "The TaskQuery field is deprecated.");
}
@Test
public void testJobScopedKillsActive() throws Exception {
- Query.Builder query = Query.unscoped().byJob(JOB_KEY);
- storageUtil.expectTaskFetch(query.active(), buildScheduledTask());
+ Query.Builder query = Query.unscoped().byJob(JOB_KEY).active();
+ storageUtil.expectTaskFetch(query, buildScheduledTask());
lockManager.validateIfLocked(LOCK_KEY, java.util.Optional.empty());
expectTransitionsToKilling();
@@ -656,16 +656,16 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
@Test
public void testKillByTaskId() throws Exception {
// A non-admin user may kill their own tasks when specified by task IDs.
- Query.Builder query = Query.taskScoped("taskid");
+ Query.Builder query = Query.taskScoped("taskid").active();
// This query happens twice - once for authentication (without consistency) and once again
// to perform the state change (within a write transaction).
- storageUtil.expectTaskFetch(query.active(), buildScheduledTask());
+ storageUtil.expectTaskFetch(query, buildScheduledTask());
lockManager.validateIfLocked(LOCK_KEY, java.util.Optional.empty());
expectTransitionsToKilling();
control.replay();
- assertOkResponse(thrift.killTasks(query.get(), null, null, null));
+ assertOkResponse(thrift.killTasks(query.get().newBuilder(), null, null, null));
}
@Test
@@ -774,9 +774,9 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
assertEquals(
okResponse(Result.queryRecoveryResult(
new QueryRecoveryResult().setTasks(IScheduledTask.toBuildersSet(queryResult)))),
- thrift.queryRecovery(query.get()));
+ thrift.queryRecovery(query.get().newBuilder()));
- assertEquals(okEmptyResponse(), thrift.deleteRecoveryTasks(query.get()));
+ assertEquals(okEmptyResponse(), thrift.deleteRecoveryTasks(query.get().newBuilder()));
assertEquals(okEmptyResponse(), thrift.commitRecovery());