You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2014/05/27 20:25:09 UTC
[1/2] Enable some additional PMD rules,
and fix sources to satisfy them.
Repository: incubator-aurora
Updated Branches:
refs/heads/master 10da38a3a -> a8fa267f0
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java b/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java
index a56f037..48b36c2 100644
--- a/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java
+++ b/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java
@@ -58,19 +58,21 @@ public final class AsyncUtil {
// See java.util.concurrent.ThreadPoolExecutor#afterExecute(Runnable, Throwable)
// for more details and an implementation example.
super.afterExecute(runnable, throwable);
- if (throwable != null) {
- logger.log(Level.SEVERE, throwable.toString(), throwable);
- } else if (runnable instanceof Future) {
- try {
- Future<?> future = (Future<?>) runnable;
- if (future.isDone()) {
- future.get();
+ if (throwable == null) {
+ if (runnable instanceof Future) {
+ try {
+ Future<?> future = (Future<?>) runnable;
+ if (future.isDone()) {
+ future.get();
+ }
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ } catch (ExecutionException ee) {
+ logger.log(Level.SEVERE, ee.toString(), ee);
}
- } catch (InterruptedException ie) {
- Thread.currentThread().interrupt();
- } catch (ExecutionException ee) {
- logger.log(Level.SEVERE, ee.toString(), ee);
}
+ } else {
+ logger.log(Level.SEVERE, throwable.toString(), throwable);
}
}
};
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java b/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
index da06f12..d885b22 100644
--- a/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
+++ b/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
@@ -28,7 +28,7 @@ public final class CommandUtil {
}
private static String uriBasename(String uri) {
- int lastSlash = uri.lastIndexOf("/");
+ int lastSlash = uri.lastIndexOf('/');
if (lastSlash == -1) {
return uri;
} else {
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/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 a5350c8..cfb1d16 100644
--- a/src/main/java/org/apache/aurora/scheduler/base/Query.java
+++ b/src/main/java/org/apache/aurora/scheduler/base/Query.java
@@ -148,11 +148,11 @@ public final class Query {
public static final class Builder implements Supplier<TaskQuery> {
private final TaskQuery query;
- private Builder() {
+ Builder() {
this.query = new TaskQuery();
}
- private Builder(final TaskQuery query) {
+ Builder(final TaskQuery query) {
this.query = checkNotNull(query); // It is expected that the caller calls deepCopy.
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java b/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
index f86a5b7..47cb70b 100644
--- a/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
@@ -324,7 +324,7 @@ public final class ConfigurationManager {
IValueConstraint valueConstraint = constraint.getConstraint().getValue();
- if (!(valueConstraint.getValues().size() == 1)) {
+ if (valueConstraint.getValues().size() != 1) {
throw new TaskDescriptionException("A dedicated constraint must have exactly one value");
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java b/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
index 5a38479..9ee2a85 100644
--- a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
+++ b/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
@@ -112,7 +112,7 @@ public class Resources {
.add(Resources.makeMesosResource(CPUS, numCpus))
.add(Resources.makeMesosResource(DISK_MB, disk.as(Data.MB)))
.add(Resources.makeMesosResource(RAM_MB, ram.as(Data.MB)));
- if (selectedPorts.size() > 0) {
+ if (selectedPorts.isEmpty()) {
resourceBuilder.add(Resources.makeMesosRangeResource(Resources.PORTS, selectedPorts));
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java b/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
index 7fc4930..d511ec0 100644
--- a/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
+++ b/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
@@ -14,6 +14,7 @@
package org.apache.aurora.scheduler.configuration;
import java.util.Map;
+import java.util.logging.Logger;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Functions;
@@ -26,12 +27,15 @@ import com.google.common.collect.Range;
import org.apache.aurora.scheduler.configuration.ConfigurationManager.TaskDescriptionException;
import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
+import org.apache.commons.lang.StringUtils;
/**
* Wrapper for a configuration that has been fully-sanitized and populated with defaults.
*/
public final class SanitizedConfiguration {
+ private static final Logger LOG = Logger.getLogger(SanitizedConfiguration.class.getName());
+
private final IJobConfiguration sanitized;
private final Map<Integer, ITaskConfig> tasks;
@@ -73,6 +77,24 @@ public final class SanitizedConfiguration {
return tasks;
}
+ /**
+ * Determines whether this job is configured as a cron job.
+ *
+ * @return {@code true} if this is a cron job, otherwise {@code false}.
+ */
+ public boolean isCron() {
+ if (getJobConfig().isSetCronSchedule()) {
+ if (StringUtils.isEmpty(getJobConfig().getCronSchedule())) {
+ // TODO(ksweeney): Remove this in 0.7.0 (AURORA-423).
+ LOG.warning("Got service config with empty string cron schedule. aurora-0.7.x "
+ + "will interpret this as cron job and cause an error.");
+ return false;
+ }
+ return true;
+ }
+ return false;
+ }
+
@Override
public boolean equals(Object o) {
if (!(o instanceof SanitizedConfiguration)) {
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java
index cc08137..57d874b 100644
--- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java
+++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java
@@ -97,7 +97,7 @@ class AuroraCronJob implements Job {
private final Map<Integer, ITaskConfig> pendingTasks;
private final Set<String> activeTaskIds;
- private DeferredLaunch(Map<Integer, ITaskConfig> pendingTasks, Set<String> activeTaskIds) {
+ DeferredLaunch(Map<Integer, ITaskConfig> pendingTasks, Set<String> activeTaskIds) {
this.pendingTasks = pendingTasks;
this.activeTaskIds = activeTaskIds;
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java
index 8f72a2d..02ba94a 100644
--- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java
+++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java
@@ -32,6 +32,7 @@ import org.apache.aurora.scheduler.cron.SanitizedCronJob;
import org.apache.aurora.scheduler.events.PubsubEvent;
import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
import org.quartz.Scheduler;
+import org.quartz.SchedulerException;
import static com.google.common.base.Preconditions.checkNotNull;
@@ -79,7 +80,7 @@ class CronLifecycle extends AbstractIdleService implements PubsubEvent.EventSubs
}
@Override
- protected void startUp() throws Exception {
+ protected void startUp() throws SchedulerException {
LOG.info("Starting Quartz cron scheduler" + scheduler.getSchedulerName() + ".");
scheduler.start();
RUNNING_FLAG.set(1);
@@ -104,7 +105,7 @@ class CronLifecycle extends AbstractIdleService implements PubsubEvent.EventSubs
}
@Override
- protected void shutDown() throws Exception {
+ protected void shutDown() throws SchedulerException {
LOG.info("Shutting down Quartz cron scheduler.");
scheduler.shutdown();
RUNNING_FLAG.set(0);
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java
index 0094c88..88cb360 100644
--- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java
@@ -89,7 +89,7 @@ public class CronModule extends AbstractModule {
}
@Provides
- private TimeZone provideTimeZone() {
+ TimeZone provideTimeZone() {
TimeZone timeZone = TimeZone.getTimeZone(CRON_TIMEZONE.get());
TimeZone systemTimeZone = TimeZone.getDefault();
if (!timeZone.equals(systemTimeZone)) {
@@ -109,7 +109,7 @@ public class CronModule extends AbstractModule {
*/
@Provides
@Singleton
- private static synchronized Scheduler provideScheduler(AuroraCronJobFactory jobFactory) {
+ static synchronized Scheduler provideScheduler(AuroraCronJobFactory jobFactory) {
SimpleThreadPool threadPool = new SimpleThreadPool(NUM_THREADS.get(), Thread.NORM_PRIORITY);
threadPool.setMakeThreadsDaemons(true);
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java b/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java
index 06a5f8f..2afbef8 100644
--- a/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java
+++ b/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java
@@ -34,13 +34,13 @@ public interface PubsubEvent {
/**
* Interface with no functionality, but identifies a class as supporting task pubsub events.
*/
- public interface EventSubscriber {
+ interface EventSubscriber {
}
/**
* Event sent when tasks were deleted.
*/
- public static class TasksDeleted implements PubsubEvent {
+ class TasksDeleted implements PubsubEvent {
private final Set<IScheduledTask> tasks;
public TasksDeleted(Set<IScheduledTask> tasks) {
@@ -76,8 +76,10 @@ public interface PubsubEvent {
/**
* Event sent when a task changed state.
+ * <p>
+ * This class is final as it should only be constructed through declared factory methods.
*/
- public static final class TaskStateChange implements PubsubEvent {
+ final class TaskStateChange implements PubsubEvent {
private final IScheduledTask task;
private final Optional<ScheduleStatus> oldState;
@@ -156,7 +158,7 @@ public interface PubsubEvent {
/**
* Event sent when a host changed maintenance state.
*/
- public static class HostMaintenanceStateChange implements PubsubEvent {
+ class HostMaintenanceStateChange implements PubsubEvent {
private final HostStatus status;
public HostMaintenanceStateChange(HostStatus status) {
@@ -186,7 +188,7 @@ public interface PubsubEvent {
/**
* Event sent when a scheduling assignment was vetoed.
*/
- public static class Vetoed implements PubsubEvent {
+ class Vetoed implements PubsubEvent {
private final String taskId;
private final Set<Veto> vetoes;
@@ -220,7 +222,7 @@ public interface PubsubEvent {
}
}
- public static class DriverRegistered implements PubsubEvent {
+ class DriverRegistered implements PubsubEvent {
@Override
public boolean equals(Object o) {
return o != null && getClass().equals(o.getClass());
@@ -232,7 +234,7 @@ public interface PubsubEvent {
}
}
- public static class DriverDisconnected implements PubsubEvent {
+ class DriverDisconnected implements PubsubEvent {
@Override
public boolean equals(Object o) {
return o != null && getClass().equals(o.getClass());
@@ -244,7 +246,7 @@ public interface PubsubEvent {
}
}
- public static class SchedulerActive implements PubsubEvent {
+ class SchedulerActive implements PubsubEvent {
@Override
public boolean equals(Object o) {
return o != null && getClass().equals(o.getClass());
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java b/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
index d8daa68..b206830 100644
--- a/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
+++ b/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
@@ -32,7 +32,7 @@ public interface SchedulingFilter {
* is only intended to be used for relative ranking of vetoes for determining which veto against
* a scheduling assignment is 'weakest'.
*/
- public static class Veto {
+ class Veto {
public static final int MAX_SCORE = 1000;
private final String reason;
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java b/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java
index c955640..96aec08 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java
@@ -26,12 +26,14 @@ import org.antlr.stringtemplate.StringTemplate;
/**
* Base class for common functions needed in a jersey stringtemplate servlet.
+ *
+ * TODO(wfarner): This class should be composed rather than extended. Turn it into a helper class.
*/
-abstract class JerseyTemplateServlet {
+class JerseyTemplateServlet {
private final StringTemplateHelper templateHelper;
- JerseyTemplateServlet(String templatePath) {
+ protected JerseyTemplateServlet(String templatePath) {
templateHelper = new StringTemplateHelper(getClass(), templatePath, true);
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
index 9cb85bb..070c7da 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
@@ -128,13 +128,13 @@ public class LeaderRedirect {
.append(req.getScheme())
.append("://")
.append(target.getHostText())
- .append(":")
+ .append(':')
.append(target.getPort())
.append(req.getRequestURI());
String queryString = req.getQueryString();
if (queryString != null) {
- redirect.append("?").append(queryString);
+ redirect.append('?').append(queryString);
}
return Optional.of(redirect.toString());
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/Quotas.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/Quotas.java b/src/main/java/org/apache/aurora/scheduler/http/Quotas.java
index b516470..438d127 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/Quotas.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/Quotas.java
@@ -89,7 +89,7 @@ public class Quotas {
private final long ramMb;
private final long diskMb;
- private ResourceAggregateBean(double cpu, long ramMb, long diskMb) {
+ ResourceAggregateBean(double cpu, long ramMb, long diskMb) {
this.cpu = cpu;
this.ramMb = ramMb;
this.diskMb = diskMb;
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/Slaves.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/Slaves.java b/src/main/java/org/apache/aurora/scheduler/http/Slaves.java
index 56a8ce1..4c26db4 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/Slaves.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/Slaves.java
@@ -46,7 +46,7 @@ import static org.apache.aurora.scheduler.storage.Storage.Work;
@Path("/slaves")
public class Slaves extends JerseyTemplateServlet {
private final String clusterName;
- private Storage storage;
+ private final Storage storage;
/**
* Injected constructor.
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/StructDump.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/StructDump.java b/src/main/java/org/apache/aurora/scheduler/http/StructDump.java
index 7bf2fba..a7fbcbf 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/StructDump.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/StructDump.java
@@ -123,10 +123,10 @@ public class StructDump extends JerseyTemplateServlet {
public void execute(StringTemplate template) {
template.setAttribute("id", id);
Optional<? extends TBase<?, ?>> struct = storage.weaklyConsistentRead(work);
- if (!struct.isPresent()) {
- template.setAttribute("exception", "Entity not found");
- } else {
+ if (struct.isPresent()) {
template.setAttribute("structPretty", Util.prettyPrint(struct.get()));
+ } else {
+ template.setAttribute("exception", "Entity not found");
}
}
});
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java b/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java
index a9d5f8f..e9d9bc4 100644
--- a/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java
@@ -14,16 +14,12 @@
package org.apache.aurora.scheduler.local;
import java.util.List;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
-import java.util.logging.Level;
import java.util.logging.Logger;
-import javax.annotation.Nullable;
import javax.inject.Inject;
import javax.inject.Provider;
import javax.inject.Singleton;
@@ -34,7 +30,6 @@ import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.eventbus.Subscribe;
-import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.inject.AbstractModule;
import com.twitter.common.application.ShutdownRegistry;
import com.twitter.common.base.Command;
@@ -56,6 +51,7 @@ import org.apache.aurora.gen.TaskConfig;
import org.apache.aurora.gen.comm.DeletedTasks;
import org.apache.aurora.gen.comm.SchedulerMessage;
import org.apache.aurora.scheduler.DriverFactory;
+import org.apache.aurora.scheduler.base.AsyncUtil;
import org.apache.aurora.scheduler.base.JobKeys;
import org.apache.aurora.scheduler.configuration.ConfigurationManager;
import org.apache.aurora.scheduler.configuration.Resources;
@@ -136,24 +132,8 @@ public class IsolatedSchedulerModule extends AbstractModule {
}
private static ScheduledExecutorService createThreadPool(ShutdownRegistry shutdownRegistry) {
- final ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(
- 1,
- new ThreadFactoryBuilder().setDaemon(true).setNameFormat("TaskScheduler-%d").build()) {
-
- @Override
- protected void afterExecute(Runnable runnable, @Nullable Throwable throwable) {
- if (throwable != null) {
- LOG.log(Level.WARNING, "Error: " + throwable, throwable);
- } else if (runnable instanceof Future) {
- Future<?> future = (Future<?>) runnable;
- try {
- future.get();
- } catch (InterruptedException | ExecutionException e) {
- e.printStackTrace();
- }
- }
- }
- };
+ final ScheduledThreadPoolExecutor executor =
+ AsyncUtil.loggingScheduledExecutor(1, "TaskScheduler-%d", LOG);
Stats.exportSize("schedule_queue_size", executor.getQueue());
shutdownRegistry.addAction(new Command() {
@Override
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java b/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java
index 396cecf..1a7b421 100644
--- a/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java
+++ b/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java
@@ -49,6 +49,10 @@ public class QuotaCheckResult {
private Resource(String unit) {
this.unit = unit;
}
+
+ String getUnit() {
+ return unit;
+ }
}
private final Optional<String> details;
@@ -107,7 +111,7 @@ public class QuotaCheckResult {
.append(" quota exceeded by ")
.append(String.format("%.2f", b - a))
.append(" ")
- .append(resource.unit);
+ .append(resource.getUnit());
}
return result;
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java b/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
index 3390038..0568529 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
@@ -21,6 +21,7 @@ import com.google.common.base.Optional;
import com.twitter.common.util.Clock;
import org.apache.aurora.gen.Lock;
+import org.apache.aurora.gen.LockKey._Fields;
import org.apache.aurora.scheduler.base.JobKeys;
import org.apache.aurora.scheduler.storage.LockStore;
import org.apache.aurora.scheduler.storage.Storage;
@@ -121,11 +122,8 @@ class LockManagerImpl implements LockManager {
}
private static String formatLockKey(ILockKey lockKey) {
- switch (lockKey.getSetField()) {
- case JOB:
- return JobKeys.canonicalString(lockKey.getJob());
- default:
- return "Unknown lock key type: " + lockKey.getSetField();
- }
+ return lockKey.getSetField() == _Fields.JOB
+ ? JobKeys.canonicalString(lockKey.getJob())
+ : "Unknown lock key type: " + lockKey.getSetField();
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java b/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
index 46446a9..44ced61 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
@@ -49,7 +49,6 @@ import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
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.commons.lang.StringUtils;
import static com.google.common.base.Preconditions.checkNotNull;
@@ -112,19 +111,6 @@ class SchedulerCoreImpl implements SchedulerCore {
return hasActiveTasks || cronJobManager.hasJob(job.getKey());
}
- private static boolean isCron(SanitizedConfiguration config) {
- if (!config.getJobConfig().isSetCronSchedule()) {
- return false;
- } else if (StringUtils.isEmpty(config.getJobConfig().getCronSchedule())) {
- // TODO(ksweeney): Remove this in 0.7.0 (AURORA-423).
- LOG.warning("Got service config with empty string cron schedule. aurora-0.7.x "
- + "will interpret this as cron job and cause an error.");
- return false;
- } else {
- return true;
- }
- }
-
@Override
public synchronized void createJob(final SanitizedConfiguration sanitizedConfiguration)
throws ScheduleException {
@@ -140,7 +126,7 @@ class SchedulerCoreImpl implements SchedulerCore {
validateTaskLimits(job.getTaskConfig(), job.getInstanceCount());
// TODO(mchucarroll): deprecate cron as a part of create/kill job.(AURORA-454)
- if (isCron(sanitizedConfiguration)) {
+ if (sanitizedConfiguration.isCron()) {
try {
LOG.warning("Deprecated behavior: scheduling job " + job.getKey()
+ " with cron via createJob (AURORA_454)");
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java b/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
index 616bdc4..6aa3e1b 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
@@ -46,7 +46,7 @@ public interface AttributeStore {
* Attributes are considered mostly ephemeral and extremely low risk when inconsistency
* is present.
*/
- public interface Mutable extends AttributeStore {
+ interface Mutable extends AttributeStore {
/**
* Deletes all attributes in the store.
@@ -72,7 +72,7 @@ public interface AttributeStore {
boolean setMaintenanceMode(String host, MaintenanceMode mode);
}
- public static final class Util {
+ final class Util {
private Util() {
}
@@ -90,5 +90,4 @@ public interface AttributeStore {
? attributes.get().getAttributes() : ImmutableList.<Attribute>of();
}
}
-
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java b/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java
index dcd0ab0..ad0d67a 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java
@@ -50,7 +50,7 @@ public interface JobStore {
*/
Set<String> fetchManagerIds();
- public interface Mutable extends JobStore {
+ interface Mutable extends JobStore {
/**
* Saves the job configuration for a job that has been accepted by the scheduler. Acts as an
* update if the managerId already exists.
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java b/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java
index 4e2371c..596a378 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java
@@ -39,7 +39,7 @@ public interface LockStore {
*/
Optional<ILock> fetchLock(ILockKey lockKey);
- public interface Mutable extends LockStore {
+ interface Mutable extends LockStore {
/**
* Saves a new lock or overwrites the existing one with same LockKey.
*
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java b/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java
index 0270e58..688eb56 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java
@@ -38,7 +38,7 @@ public interface QuotaStore {
*/
Map<String, IResourceAggregate> fetchQuotas();
- public interface Mutable extends QuotaStore {
+ interface Mutable extends QuotaStore {
/**
* Deletes all quotas.
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java b/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java
index c16f70d..fa49e22 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java
@@ -41,6 +41,10 @@ public class ReadWriteLockManager {
private LockType(LockMode mode) {
this.mode = mode;
}
+
+ LockMode getMode() {
+ return mode;
+ }
}
private static class LockState {
@@ -53,14 +57,14 @@ public class ReadWriteLockManager {
initialLockMode = mode;
stateChanged = true;
}
- if (initialLockMode == mode) {
+ if (initialLockMode.equals(mode)) {
lockCount++;
}
return stateChanged;
}
private void lockReleased(LockMode mode) {
- if (initialLockMode == mode) {
+ if (initialLockMode.equals(mode)) {
lockCount--;
if (lockCount == 0) {
initialLockMode = LockMode.NONE;
@@ -95,7 +99,7 @@ public class ReadWriteLockManager {
lock.writeLock().lock();
}
- return lockState.get().lockAcquired(type.mode);
+ return lockState.get().lockAcquired(type.getMode());
}
/**
@@ -112,7 +116,7 @@ public class ReadWriteLockManager {
lock.writeLock().unlock();
}
- lockState.get().lockReleased(type.mode);
+ lockState.get().lockReleased(type.getMode());
}
/**
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java b/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java
index f6a992d..057a2e6 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java
@@ -27,7 +27,7 @@ public interface SchedulerStore {
*/
@Nullable String fetchFrameworkId();
- public interface Mutable extends SchedulerStore {
+ interface Mutable extends SchedulerStore {
/**
* Stores the given framework id overwriting any previously saved id.
*
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/Storage.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/Storage.java b/src/main/java/org/apache/aurora/scheduler/storage/Storage.java
index 768a821..bbbd7dc 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/Storage.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/Storage.java
@@ -252,7 +252,7 @@ public interface Storage {
/**
* Utility functions for interacting with a Storage instance.
*/
- public final class Util {
+ final class Util {
private Util() {
// Utility class.
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java b/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
index d507666..8c20ab6 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
@@ -84,7 +84,9 @@ public final class StorageBackfill {
// We want to keep exactly one task from this shard, so sort the IDs and keep the
// highest (newest) in the hopes that it is legitimately running.
String newestTask = Iterables.getLast(Sets.newTreeSet(activeTasksInShard));
- if (!Tasks.id(task).equals(newestTask)) {
+ if (Tasks.id(task).equals(newestTask)) {
+ LOG.info("Retaining task " + Tasks.id(task));
+ } else {
task.setStatus(ScheduleStatus.KILLED);
task.addToTaskEvents(new TaskEvent(clock.nowMillis(), ScheduleStatus.KILLED)
.setMessage("Killed duplicate shard."));
@@ -92,8 +94,6 @@ public final class StorageBackfill {
// condition between the time the scheduler is actually available without hitting
// IllegalStateException (see DriverImpl).
// driver.killTask(Tasks.id(task));
- } else {
- LOG.info("Retaining task " + Tasks.id(task));
}
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/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 40503b4..b76c937 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
@@ -36,12 +36,12 @@ public interface TaskStore {
*/
ImmutableSet<IScheduledTask> fetchTasks(Query.Builder query);
- public interface Mutable extends TaskStore {
+ interface Mutable extends TaskStore {
/**
* A convenience interface to allow callers to more concisely implement a task mutation.
*/
- public interface TaskMutation extends Function<IScheduledTask, IScheduledTask> {
+ interface TaskMutation extends Function<IScheduledTask, IScheduledTask> {
}
/**
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java b/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
index 17b79c3..4e2fb8b 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
@@ -119,13 +119,13 @@ public class BackupModule extends PrivateModule {
}
@Provides
- private File provideBackupDir() {
+ File provideBackupDir() {
if (!unvalidatedBackupDir.exists()) {
- if (!unvalidatedBackupDir.mkdirs()) {
+ if (unvalidatedBackupDir.mkdirs()) {
+ LOG.info("Created backup dir " + unvalidatedBackupDir.getPath() + ".");
+ } else {
throw new IllegalArgumentException(
"Unable to create backup dir " + unvalidatedBackupDir.getPath() + ".");
- } else {
- LOG.info("Created backup dir " + unvalidatedBackupDir.getPath() + ".");
}
}
@@ -138,7 +138,7 @@ public class BackupModule extends PrivateModule {
}
@Provides
- private BackupConfig provideBackupConfig(File backupDir) {
+ BackupConfig provideBackupConfig(File backupDir) {
return new BackupConfig(backupDir, MAX_SAVED_BACKUPS.get(), BACKUP_INTERVAL.get());
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java b/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
index e3c09e0..f1511b8 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
@@ -93,7 +93,7 @@ public interface Recovery {
* Thrown when a recovery operation could not be completed due to internal errors or improper
* invocation order.
*/
- public static class RecoveryException extends Exception {
+ class RecoveryException extends Exception {
RecoveryException(String message) {
super(message);
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java b/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java
index c31fe2d..496f13e 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java
@@ -319,7 +319,7 @@ public final class LogManager {
private final MessageDigest digest;
private final int maxEntrySizeBytes;
- private EntrySerializer(MessageDigest digest, Amount<Integer, Data> maxEntrySize) {
+ EntrySerializer(MessageDigest digest, Amount<Integer, Data> maxEntrySize) {
this.digest = checkNotNull(digest);
maxEntrySizeBytes = maxEntrySize.as(Data.BYTES);
}
@@ -374,7 +374,7 @@ public final class LogManager {
new Transaction().setSchemaVersion(storageConstants.CURRENT_SCHEMA_VERSION);
private final AtomicBoolean committed = new AtomicBoolean(false);
- private StreamTransaction() {
+ StreamTransaction() {
// supplied by factory method
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java b/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
index 899c61a..7fcc072 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
@@ -343,7 +343,7 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore
}
private static final class RecoveryFailedException extends SchedulerException {
- private RecoveryFailedException(Throwable cause) {
+ RecoveryFailedException(Throwable cause) {
super(cause);
}
}
@@ -451,10 +451,10 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore
try {
snapshot();
} catch (StorageException e) {
- if (e.getCause() != null) {
- LOG.log(Level.WARNING, e.getMessage(), e.getCause());
- } else {
+ if (e.getCause() == null) {
LOG.log(Level.WARNING, "StorageException when attempting to snapshot.", e);
+ } else {
+ LOG.log(Level.WARNING, e.getMessage(), e.getCause());
}
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java b/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java
index 87a442b..35e2e5a 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java
@@ -69,7 +69,7 @@ public class LogOpMatcher implements IArgumentMatcher {
public static final class StreamMatcher {
private final Stream stream;
- private StreamMatcher(Stream stream) {
+ StreamMatcher(Stream stream) {
this.stream = Preconditions.checkNotNull(stream);
}
@@ -106,6 +106,6 @@ public class LogOpMatcher implements IArgumentMatcher {
*/
private static byte[] sameEntry(LogEntry entry) {
EasyMock.reportMatcher(new LogOpMatcher(entry));
- return null;
+ return new byte[] {};
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
index 429691c..ff9e45c 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
@@ -51,11 +51,11 @@ class MemAttributeStore implements Mutable {
@Override
public boolean setMaintenanceMode(String host, MaintenanceMode mode) {
HostAttributes stored = hostAttributes.get(host);
- if (stored != null) {
+ if (stored == null) {
+ return false;
+ } else {
stored.setMode(mode);
return true;
- } else {
- return false;
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java
index 962861d..571636d 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java
@@ -88,10 +88,10 @@ class MemJobStore implements JobStore.Mutable {
checkNotNull(jobKey);
Optional<Manager> manager = Optional.fromNullable(managers.getIfPresent(managerId));
- if (!manager.isPresent()) {
- return Optional.absent();
- } else {
+ if (manager.isPresent()) {
return Optional.fromNullable(manager.get().jobs.get(jobKey));
+ } else {
+ return Optional.absent();
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/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 6f3ebd3..29f64f7 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
@@ -67,6 +67,7 @@ import org.apache.aurora.gen.JobSummaryResult;
import org.apache.aurora.gen.ListBackupsResult;
import org.apache.aurora.gen.Lock;
import org.apache.aurora.gen.LockKey;
+import org.apache.aurora.gen.LockKey._Fields;
import org.apache.aurora.gen.LockValidation;
import org.apache.aurora.gen.MaintenanceStatusResult;
import org.apache.aurora.gen.PopulateJobResult;
@@ -252,19 +253,6 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
return response;
}
- private static boolean isCron(SanitizedConfiguration config) {
- if (!config.getJobConfig().isSetCronSchedule()) {
- return false;
- } else if (StringUtils.isEmpty(config.getJobConfig().getCronSchedule())) {
- // TODO(ksweeney): Remove this in 0.7.0 (AURORA-424).
- LOG.warning("Got service config with empty string cron schedule. aurora-0.7.x "
- + "will interpret this as cron job and cause an error.");
- return false;
- } else {
- return true;
- }
- }
-
@Override
public Response scheduleCronJob(
JobConfiguration mutableJob,
@@ -290,7 +278,7 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
ILockKey.build(LockKey.job(jobKey.newBuilder())),
Optional.fromNullable(mutableLock).transform(ILock.FROM_BUILDER));
- if (!isCron(sanitized)) {
+ if (!sanitized.isCron()) {
LOG.info("Invalid attempt to schedule non-cron job "
+ sanitized.getJobConfig().getKey()
+ " with cron.");
@@ -870,16 +858,97 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
}
Response resp = new Response();
- if (!errors.isEmpty()) {
- resp.setResponseCode(ResponseCode.WARNING).setMessage(Joiner.on(", ").join(errors));
- } else {
+ if (errors.isEmpty()) {
resp.setResponseCode(OK).setMessage("All rewrites completed successfully.");
+ } else {
+ resp.setResponseCode(ResponseCode.WARNING).setMessage(Joiner.on(", ").join(errors));
}
return resp;
}
});
}
+ private Optional<String> rewriteJob(JobConfigRewrite jobRewrite, JobStore.Mutable jobStore) {
+ IJobConfiguration existingJob = IJobConfiguration.build(jobRewrite.getOldJob());
+ IJobConfiguration rewrittenJob;
+ Optional<String> error = Optional.absent();
+ try {
+ rewrittenJob = ConfigurationManager.validateAndPopulate(
+ IJobConfiguration.build(jobRewrite.getRewrittenJob()));
+ } catch (TaskDescriptionException e) {
+ // We could add an error here, but this is probably a hint of something wrong in
+ // the client that's causing a bad configuration to be applied.
+ throw Throwables.propagate(e);
+ }
+
+ if (existingJob.getKey().equals(rewrittenJob.getKey())) {
+ if (existingJob.getOwner().equals(rewrittenJob.getOwner())) {
+ Multimap<String, IJobConfiguration> matches = jobsByKey(jobStore, existingJob.getKey());
+ switch (matches.size()) {
+ case 0:
+ error = Optional.of(
+ "No jobs found for key " + JobKeys.canonicalString(existingJob.getKey()));
+ break;
+
+ case 1:
+ Map.Entry<String, IJobConfiguration> match =
+ Iterables.getOnlyElement(matches.entries());
+ IJobConfiguration storedJob = match.getValue();
+ if (storedJob.equals(existingJob)) {
+ jobStore.saveAcceptedJob(match.getKey(), rewrittenJob);
+ } else {
+ error = Optional.of(
+ "CAS compare failed for " + JobKeys.canonicalString(storedJob.getKey()));
+ }
+ break;
+
+ default:
+ error = Optional.of("Multiple jobs found for key "
+ + JobKeys.canonicalString(existingJob.getKey()));
+ }
+ } else {
+ error = Optional.of("Disallowing rewrite attempting to change job owner.");
+ }
+ } else {
+ error = Optional.of("Disallowing rewrite attempting to change job key.");
+ }
+
+ return error;
+ }
+
+ private Optional<String> rewriteInstance(
+ InstanceConfigRewrite instanceRewrite,
+ MutableStoreProvider storeProvider) {
+
+ InstanceKey instanceKey = instanceRewrite.getInstanceKey();
+ Optional<String> error = Optional.absent();
+ Iterable<IScheduledTask> tasks = storeProvider.getTaskStore().fetchTasks(
+ Query.instanceScoped(IJobKey.build(instanceKey.getJobKey()),
+ instanceKey.getInstanceId())
+ .active());
+ Optional<IAssignedTask> task =
+ Optional.fromNullable(Iterables.getOnlyElement(tasks, null))
+ .transform(Tasks.SCHEDULED_TO_ASSIGNED);
+
+ if (task.isPresent()) {
+ if (task.get().getTask().newBuilder().equals(instanceRewrite.getOldTask())) {
+ ITaskConfig newConfiguration = ITaskConfig.build(
+ ConfigurationManager.applyDefaultsIfUnset(instanceRewrite.getRewrittenTask()));
+ boolean changed = storeProvider.getUnsafeTaskStore().unsafeModifyInPlace(
+ task.get().getTaskId(), newConfiguration);
+ if (!changed) {
+ error = Optional.of("Did not change " + task.get().getTaskId());
+ }
+ } else {
+ error = Optional.of("CAS compare failed for " + instanceKey);
+ }
+ } else {
+ error = Optional.of("No active task found for " + instanceKey);
+ }
+
+ return error;
+ }
+
private Optional<String> rewriteConfig(
ConfigRewrite command,
MutableStoreProvider storeProvider) {
@@ -887,73 +956,11 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
Optional<String> error = Optional.absent();
switch (command.getSetField()) {
case JOB_REWRITE:
- JobConfigRewrite jobRewrite = command.getJobRewrite();
- IJobConfiguration existingJob = IJobConfiguration.build(jobRewrite.getOldJob());
- IJobConfiguration rewrittenJob;
- try {
- rewrittenJob = ConfigurationManager.validateAndPopulate(
- IJobConfiguration.build(jobRewrite.getRewrittenJob()));
- } catch (TaskDescriptionException e) {
- // We could add an error here, but this is probably a hint of something wrong in
- // the client that's causing a bad configuration to be applied.
- throw Throwables.propagate(e);
- }
- if (!existingJob.getKey().equals(rewrittenJob.getKey())) {
- error = Optional.of("Disallowing rewrite attempting to change job key.");
- } else if (!existingJob.getOwner().equals(rewrittenJob.getOwner())) {
- error = Optional.of("Disallowing rewrite attempting to change job owner.");
- } else {
- JobStore.Mutable jobStore = storeProvider.getJobStore();
- Multimap<String, IJobConfiguration> matches =
- jobsByKey(jobStore, existingJob.getKey());
- switch (matches.size()) {
- case 0:
- error = Optional.of(
- "No jobs found for key " + JobKeys.canonicalString(existingJob.getKey()));
- break;
-
- case 1:
- Map.Entry<String, IJobConfiguration> match =
- Iterables.getOnlyElement(matches.entries());
- IJobConfiguration storedJob = match.getValue();
- if (!storedJob.equals(existingJob)) {
- error = Optional.of(
- "CAS compare failed for " + JobKeys.canonicalString(storedJob.getKey()));
- } else {
- jobStore.saveAcceptedJob(match.getKey(), rewrittenJob);
- }
- break;
-
- default:
- error = Optional.of("Multiple jobs found for key "
- + JobKeys.canonicalString(existingJob.getKey()));
- }
- }
+ error = rewriteJob(command.getJobRewrite(), storeProvider.getJobStore());
break;
case INSTANCE_REWRITE:
- InstanceConfigRewrite instanceRewrite = command.getInstanceRewrite();
- InstanceKey instanceKey = instanceRewrite.getInstanceKey();
- Iterable<IScheduledTask> tasks = storeProvider.getTaskStore().fetchTasks(
- Query.instanceScoped(IJobKey.build(instanceKey.getJobKey()),
- instanceKey.getInstanceId())
- .active());
- Optional<IAssignedTask> task =
- Optional.fromNullable(Iterables.getOnlyElement(tasks, null))
- .transform(Tasks.SCHEDULED_TO_ASSIGNED);
- if (!task.isPresent()) {
- error = Optional.of("No active task found for " + instanceKey);
- } else if (!task.get().getTask().newBuilder().equals(instanceRewrite.getOldTask())) {
- error = Optional.of("CAS compare failed for " + instanceKey);
- } else {
- ITaskConfig newConfiguration = ITaskConfig.build(
- ConfigurationManager.applyDefaultsIfUnset(instanceRewrite.getRewrittenTask()));
- boolean changed = storeProvider.getUnsafeTaskStore().unsafeModifyInPlace(
- task.get().getTaskId(), newConfiguration);
- if (!changed) {
- error = Optional.of("Did not change " + task.get().getTaskId());
- }
- }
+ error = rewriteInstance(command.getInstanceRewrite(), storeProvider);
break;
default:
@@ -1006,12 +1013,11 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
}
private String getRoleFromLockKey(ILockKey lockKey) {
- switch (lockKey.getSetField()) {
- case JOB:
- JobKeys.assertValid(lockKey.getJob());
- return lockKey.getJob().getRole();
- default:
- throw new IllegalArgumentException("Unhandled LockKey: " + lockKey.getSetField());
+ if (lockKey.getSetField() == _Fields.JOB) {
+ JobKeys.assertValid(lockKey.getJob());
+ return lockKey.getJob().getRole();
+ } else {
+ throw new IllegalArgumentException("Unhandled LockKey: " + lockKey.getSetField());
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java b/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
index 8eb52dd..0c1477c 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
@@ -33,13 +33,13 @@ public class FeatureToggleInterceptor implements MethodInterceptor {
@Override
public Object invoke(MethodInvocation invocation) throws Throwable {
Method method = invocation.getMethod();
- if (!allowMethod.apply(method)) {
+ if (allowMethod.apply(method)) {
+ return invocation.proceed();
+ } else {
return Interceptors.properlyTypedResponse(
method,
ResponseCode.ERROR,
"The " + method.getName() + " feature is currently disabled on this scheduler.");
- } else {
- return invocation.proceed();
}
}
}
[2/2] git commit: Enable some additional PMD rules,
and fix sources to satisfy them.
Posted by wf...@apache.org.
Enable some additional PMD rules, and fix sources to satisfy them.
Reviewed at https://reviews.apache.org/r/21849/
Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/a8fa267f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/a8fa267f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/a8fa267f
Branch: refs/heads/master
Commit: a8fa267f03e2fa36039eb014884858074f7b7575
Parents: 10da38a
Author: Bill Farner <wf...@apache.org>
Authored: Tue May 27 11:25:02 2014 -0700
Committer: Bill Farner <wf...@apache.org>
Committed: Tue May 27 11:25:02 2014 -0700
----------------------------------------------------------------------
build.gradle | 25 +-
config/pmd/design.xml | 1907 ++++++++++++++++++
config/pmd/logging-java.xml | 183 ++
.../apache/aurora/auth/SessionValidator.java | 2 +-
.../org/apache/aurora/scheduler/Driver.java | 2 +-
.../apache/aurora/scheduler/DriverFactory.java | 8 +-
.../aurora/scheduler/MesosSchedulerImpl.java | 27 +-
.../aurora/scheduler/MesosTaskFactory.java | 4 +-
.../aurora/scheduler/SchedulerLifecycle.java | 5 +-
.../aurora/scheduler/async/OfferQueue.java | 18 +-
.../aurora/scheduler/async/TaskGroups.java | 2 -
.../apache/aurora/scheduler/base/AsyncUtil.java | 24 +-
.../aurora/scheduler/base/CommandUtil.java | 2 +-
.../org/apache/aurora/scheduler/base/Query.java | 4 +-
.../configuration/ConfigurationManager.java | 2 +-
.../scheduler/configuration/Resources.java | 2 +-
.../configuration/SanitizedConfiguration.java | 22 +
.../scheduler/cron/quartz/AuroraCronJob.java | 2 +-
.../scheduler/cron/quartz/CronLifecycle.java | 5 +-
.../scheduler/cron/quartz/CronModule.java | 4 +-
.../aurora/scheduler/events/PubsubEvent.java | 18 +-
.../scheduler/filter/SchedulingFilter.java | 2 +-
.../scheduler/http/JerseyTemplateServlet.java | 6 +-
.../aurora/scheduler/http/LeaderRedirect.java | 4 +-
.../apache/aurora/scheduler/http/Quotas.java | 2 +-
.../apache/aurora/scheduler/http/Slaves.java | 2 +-
.../aurora/scheduler/http/StructDump.java | 6 +-
.../local/IsolatedSchedulerModule.java | 26 +-
.../scheduler/quota/QuotaCheckResult.java | 6 +-
.../aurora/scheduler/state/LockManagerImpl.java | 10 +-
.../scheduler/state/SchedulerCoreImpl.java | 16 +-
.../scheduler/storage/AttributeStore.java | 5 +-
.../aurora/scheduler/storage/JobStore.java | 2 +-
.../aurora/scheduler/storage/LockStore.java | 2 +-
.../aurora/scheduler/storage/QuotaStore.java | 2 +-
.../scheduler/storage/ReadWriteLockManager.java | 12 +-
.../scheduler/storage/SchedulerStore.java | 2 +-
.../aurora/scheduler/storage/Storage.java | 2 +-
.../scheduler/storage/StorageBackfill.java | 6 +-
.../aurora/scheduler/storage/TaskStore.java | 4 +-
.../scheduler/storage/backup/BackupModule.java | 10 +-
.../scheduler/storage/backup/Recovery.java | 2 +-
.../scheduler/storage/log/LogManager.java | 4 +-
.../scheduler/storage/log/LogStorage.java | 8 +-
.../storage/log/testing/LogOpMatcher.java | 4 +-
.../storage/mem/MemAttributeStore.java | 6 +-
.../scheduler/storage/mem/MemJobStore.java | 6 +-
.../thrift/SchedulerThriftInterface.java | 180 +-
.../thrift/aop/FeatureToggleInterceptor.java | 6 +-
49 files changed, 2361 insertions(+), 250 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/build.gradle
----------------------------------------------------------------------
diff --git a/build.gradle b/build.gradle
index 0dbaa61..646cdd2 100644
--- a/build.gradle
+++ b/build.gradle
@@ -264,7 +264,30 @@ tasks.withType(FindBugs) {
}
pmd {
- sourceSets = [ sourceSets.main ]
+ sourceSets = [sourceSets.main]
+ // PMD rule set names match XML files stored in the PMD jar. For example, with 5.11 you can
+ // see all the rules included with:
+ // $ find ~/.gradle -name pmd-5.1.1.jar | xargs zipinfo -1 | egrep java/.*.xml | head -n 5
+ // rulesets/java/clone.xml
+ // rulesets/java/basic.xml
+ // rulesets/java/strings.xml
+ // rulesets/java/sunsecure.xml
+ // rulesets/java/codesize.xml
+ // The format is straightforward: 'java-basic' refers to rulesets/java/basic.xml.
+ ruleSets = [
+ 'java-basic',
+ 'java-braces',
+ // TODO(wfarner): Circle back to (at least partially) re-enable the rules below.
+ //'java-design',
+ 'java-empty',
+ //'java-imports',
+ 'java-junit',
+ //'java-logging-java',
+ //'java-naming',
+ 'java-typeresolution',
+ 'java-unnecessary',
+ 'java-unusedcode']
+ ruleSetFiles = fileTree('config/pmd/')
toolVersion = '5.1.1'
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/config/pmd/design.xml
----------------------------------------------------------------------
diff --git a/config/pmd/design.xml b/config/pmd/design.xml
new file mode 100644
index 0000000..fb86e0a
--- /dev/null
+++ b/config/pmd/design.xml
@@ -0,0 +1,1907 @@
+<?xml version="1.0"?>
+<!--
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this work except in compliance with the License.
+You may obtain a copy of the License in the LICENSE file, or at:
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+<!--
+This is a modified file from the PMD project. Copying and editing ruleset
+configurations is the approach used to modify a ruleset behavior, such as
+disabling individul rules.
+-->
+
+<ruleset name="Design"
+ xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
+
+ <description>
+The Design ruleset contains rules that flag suboptimal code implementations. Alternate approaches
+are suggested.
+ </description>
+
+ <rule name="UseUtilityClass"
+ since="0.3"
+ message="All methods are static. Consider using a utility class instead. Alternatively, you could add a private constructor or make the class abstract to silence this warning."
+ class="net.sourceforge.pmd.lang.java.rule.design.UseUtilityClassRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UseUtilityClass">
+ <description>
+ <![CDATA[
+For classes that only have static methods, consider making them utility classes.
+Note that this doesn't apply to abstract classes, since their subclasses may
+well include non-static methods. Also, if you want this class to be a utility class,
+remember to add a private constructor to prevent instantiation.
+(Note, that this use was known before PMD 5.1.0 as UseSingleton).
+ ]]>
+ </description>
+ <priority>3</priority>
+ <example>
+<![CDATA[
+public class MaybeAUtility {
+ public static void foo() {}
+ public static void bar() {}
+}
+]]>
+ </example>
+ </rule>
+
+
+ <rule name="SimplifyBooleanReturns"
+ since="0.9"
+ message="Avoid unnecessary if..then..else statements when returning booleans"
+ class="net.sourceforge.pmd.lang.java.rule.design.SimplifyBooleanReturnsRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SimplifyBooleanReturns">
+ <description>
+Avoid unnecessary if-then-else statements when returning a boolean. The result of
+the conditional test can be returned instead.
+ </description>
+ <priority>3</priority>
+ <example>
+<![CDATA[
+public boolean isBarEqualTo(int x) {
+
+ if (bar == x) { // this bit of code...
+ return true;
+ } else {
+ return false;
+ }
+}
+
+public boolean isBarEqualTo(int x) {
+
+ return bar == x; // can be replaced with this
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="SimplifyBooleanExpressions"
+ language="java"
+ since="1.05"
+ message="Avoid unnecessary comparisons in boolean expressions"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SimplifyBooleanExpressions">
+ <description>
+Avoid unnecessary comparisons in boolean expressions, they serve no purpose and impacts readability.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//EqualityExpression/PrimaryExpression
+ /PrimaryPrefix/Literal/BooleanLiteral
+]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Bar {
+ // can be simplified to
+ // bar = isFoo();
+ private boolean bar = (isFoo() == true);
+
+ public isFoo() { return false;}
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="SwitchStmtsShouldHaveDefault"
+ language="java"
+ since="1.0"
+ message="Switch statements should have a default label"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SwitchStmtsShouldHaveDefault">
+ <description>
+All switch statements should include a default option to catch any unspecified values.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//SwitchStatement[not(SwitchLabel[@Default='true'])]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public void bar() {
+ int x = 2;
+ switch (x) {
+ case 1: int j = 6;
+ case 2: int j = 8;
+ // missing default: here
+ }
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="AvoidDeeplyNestedIfStmts"
+ since="1.0"
+ message="Deeply nested if..then statements are hard to read"
+ class="net.sourceforge.pmd.lang.java.rule.design.AvoidDeeplyNestedIfStmtsRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidDeeplyNestedIfStmts">
+ <description>
+Avoid creating deeply nested if-then statements since they are harder to read and error-prone to maintain.
+ </description>
+ <priority>3</priority>
+ <example>
+<![CDATA[
+public class Foo {
+ public void bar(int x, int y, int z) {
+ if (x>y) {
+ if (y>z) {
+ if (z==x) {
+ // !! too deep
+ }
+ }
+ }
+ }
+}
+]]>
+ </example>
+ </rule>
+
+
+ <rule name="AvoidReassigningParameters"
+ since="1.0"
+ message="Avoid reassigning parameters such as ''{0}''"
+ class="net.sourceforge.pmd.lang.java.rule.design.AvoidReassigningParametersRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidReassigningParameters">
+ <description>
+Reassigning values to incoming parameters is not recommended. Use temporary local variables instead.
+ </description>
+ <priority>2</priority>
+ <example>
+<![CDATA[
+public class Foo {
+ private void foo(String bar) {
+ bar = "something else";
+ }
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="SwitchDensity"
+ since="1.02"
+ message="A high ratio of statements to labels in a switch statement. Consider refactoring."
+ class="net.sourceforge.pmd.lang.java.rule.design.SwitchDensityRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SwitchDensity">
+ <description>
+A high ratio of statements to labels in a switch statement implies that the switch statement
+is overloaded. Consider moving the statements into new methods or creating subclasses based
+on the switch variable.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ public void bar(int x) {
+ switch (x) {
+ case 1: {
+ // lots of statements
+ break;
+ } case 2: {
+ // lots of statements
+ break;
+ }
+ }
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="ConstructorCallsOverridableMethod"
+ since="1.04"
+ message="Overridable {0} called during object construction"
+ class="net.sourceforge.pmd.lang.java.rule.design.ConstructorCallsOverridableMethodRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ConstructorCallsOverridableMethod">
+ <description>
+Calling overridable methods during construction poses a risk of invoking methods on an incompletely
+constructed object and can be difficult to debug.
+It may leave the sub-class unable to construct its superclass or forced to replicate the construction
+process completely within itself, losing the ability to call super(). If the default constructor
+contains a call to an overridable method, the subclass may be completely uninstantiable. Note that
+this includes method calls throughout the control flow graph - i.e., if a constructor Foo() calls a
+private method bar() that calls a public method buz(), this denotes a problem.
+ </description>
+ <priority>1</priority>
+ <example>
+ <![CDATA[
+public class SeniorClass {
+ public SeniorClass(){
+ toString(); //may throw NullPointerException if overridden
+ }
+ public String toString(){
+ return "IAmSeniorClass";
+ }
+}
+public class JuniorClass extends SeniorClass {
+ private String name;
+ public JuniorClass(){
+ super(); //Automatic call leads to NullPointerException
+ name = "JuniorClass";
+ }
+ public String toString(){
+ return name.toUpperCase();
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="AccessorClassGeneration"
+ since="1.04"
+ message="Avoid instantiation through private constructors from outside of the constructor's class."
+ class="net.sourceforge.pmd.lang.java.rule.design.AccessorClassGenerationRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AccessorClassGeneration">
+ <description>
+Instantiation by way of private constructors from outside of the constructor's class often causes the
+generation of an accessor. A factory method, or non-privatization of the constructor can eliminate this
+situation. The generated class file is actually an interface. It gives the accessing class the ability
+to invoke a new hidden package scope constructor that takes the interface as a supplementary parameter.
+This turns a private constructor effectively into one with package scope, and is challenging to discern.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Outer {
+ void method(){
+ Inner ic = new Inner();//Causes generation of accessor class
+ }
+ public class Inner {
+ private Inner(){}
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="FinalFieldCouldBeStatic"
+ language="java"
+ since="1.1"
+ message="This final field could be made static"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#FinalFieldCouldBeStatic">
+ <description>
+If a final field is assigned to a compile-time constant, it could be made static, thus saving overhead
+in each object at runtime.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//FieldDeclaration
+ [@Final='true' and @Static='false']
+ [not (../../../../ClassOrInterfaceDeclaration[@Interface='true'])]
+ /VariableDeclarator/VariableInitializer/Expression
+ /PrimaryExpression[not(PrimarySuffix)]/PrimaryPrefix/Literal
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Foo {
+ public final int BAR = 42; // this could be static and save some space
+}
+ ]]>
+ </example>
+ </rule>
+
+
+ <rule name="CloseResource"
+ since="1.2.2"
+ message="Ensure that resources like this {0} object are closed after use"
+ class="net.sourceforge.pmd.lang.java.rule.design.CloseResourceRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#CloseResource">
+ <description>
+Ensure that resources (like Connection, Statement, and ResultSet objects) are always closed after use.
+ </description>
+ <priority>3</priority>
+ <example>
+<![CDATA[
+public class Bar {
+ public void foo() {
+ Connection c = pool.getConnection();
+ try {
+ // do stuff
+ } catch (SQLException ex) {
+ // handle exception
+ } finally {
+ // oops, should close the connection using 'close'!
+ // c.close();
+ }
+ }
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="NonStaticInitializer"
+ language="java"
+ since="1.5"
+ message="Non-static initializers are confusing"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#NonStaticInitializer">
+ <description>
+A non-static initializer block will be called any time a constructor is invoked (just prior to
+invoking the constructor). While this is a valid language construct, it is rarely used and is
+confusing.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//Initializer[@Static='false']
+]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class MyClass {
+ // this block gets run before any call to a constructor
+ {
+ System.out.println("I am about to construct myself");
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="DefaultLabelNotLastInSwitchStmt"
+ language="java"
+ since="1.5"
+ message="The default label should be the last label in a switch statement"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#DefaultLabelNotLastInSwitchStmt">
+ <description>
+By convention, the default label should be the last label in a switch statement.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//SwitchStatement
+ [not(SwitchLabel[position() = last()][@Default='true'])]
+ [SwitchLabel[@Default='true']]
+]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Foo {
+ void bar(int a) {
+ switch (a) {
+ case 1: // do something
+ break;
+ default: // the default case should be last, by convention
+ break;
+ case 2:
+ break;
+ }
+ }
+} ]]>
+ </example>
+ </rule>
+
+ <rule name="NonCaseLabelInSwitchStatement"
+ language="java"
+ since="1.5"
+ message="A non-case label was present in a switch statement"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#NonCaseLabelInSwitchStatement">
+ <description>
+A non-case label (e.g. a named break/continue label) was present in a switch statement.
+This legal, but confusing. It is easy to mix up the case labels and the non-case labels.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//SwitchStatement//BlockStatement/Statement/LabeledStatement
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Foo {
+ void bar(int a) {
+ switch (a) {
+ case 1:
+ // do something
+ break;
+ mylabel: // this is legal, but confusing!
+ break;
+ default:
+ break;
+ }
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="OptimizableToArrayCall"
+ language="java"
+ since="1.8"
+ message="This call to Collection.toArray() may be optimizable"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#OptimizableToArrayCall">
+ <description>
+Calls to a collection's toArray() method should specify target arrays sized to match the size of the
+collection. Initial arrays that are too small are discarded in favour of new ones that have to be created
+that are the proper size.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimaryExpression
+[PrimaryPrefix/Name[ends-with(@Image, 'toArray')]]
+[
+PrimarySuffix/Arguments/ArgumentList/Expression
+ /PrimaryExpression/PrimaryPrefix/AllocationExpression
+ /ArrayDimsAndInits/Expression/PrimaryExpression/PrimaryPrefix/Literal[@Image='0']
+]
+
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+List foos = getFoos();
+
+ // inefficient, the array will be discarded
+Foo[] fooArray = foos.toArray(new Foo[0]);
+
+ // much better; this one sizes the destination array,
+ // avoiding of a new one via reflection
+Foo[] fooArray = foos.toArray(new Foo[foos.size()]);
+ ]]>
+ </example>
+ </rule>
+
+
+ <rule name="BadComparison"
+ language="java"
+ since="1.8"
+ message="Avoid equality comparisons with Double.NaN"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#BadComparison">
+ <description>
+Avoid equality comparisons with Double.NaN. Due to the implicit lack of representation
+precision when comparing floating point numbers these are likely to cause logic errors.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//EqualityExpression[@Image='==']
+ /PrimaryExpression/PrimaryPrefix
+ /Name[@Image='Double.NaN' or @Image='Float.NaN']
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+boolean x = (y == Double.NaN);
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="EqualsNull"
+ language="java"
+ since="1.9"
+ message="Avoid using equals() to compare against null"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#EqualsNull">
+ <description>
+Tests for null should not use the equals() method. The '==' operator should be used instead.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimaryExpression
+ [
+ PrimaryPrefix[Name[ends-with(@Image, 'equals')]]
+ [following-sibling::node()/Arguments/ArgumentList[count(Expression)=1]
+ /Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
+
+ or
+
+ PrimarySuffix[ends-with(@Image, 'equals')]
+ [following-sibling::node()/Arguments/ArgumentList[count(Expression)=1]
+ /Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
+
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+String x = "foo";
+
+if (x.equals(null)) { // bad form
+ doSomething();
+ }
+
+if (x == null) { // preferred
+ doSomething();
+ }
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="ConfusingTernary"
+ since="1.9"
+ message="Avoid if (x != y) ..; else ..;"
+ class="net.sourceforge.pmd.lang.java.rule.design.ConfusingTernaryRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ConfusingTernary">
+ <description>
+Avoid negation within an "if" expression with an "else" clause. For example, rephrase:
+
+ if (x != y) diff(); else same();
+as:
+ if (x == y) same(); else diff();
+
+Most "if (x != y)" cases without an "else" are often return cases, so consistent use of this
+rule makes the code easier to read. Also, this resolves trivial ordering problems, such
+as "does the error case go first?" or "does the common case go first?".
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+boolean bar(int x, int y) {
+ return (x != y) ? diff : same;
+ }
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="InstantiationToGetClass"
+ language="java"
+ since="2.0"
+ message="Avoid instantiating an object just to call getClass() on it; use the .class public member instead"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#InstantiationToGetClass">
+ <description>
+Avoid instantiating an object just to call getClass() on it; use the .class public member instead.
+ </description>
+ <priority>4</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimarySuffix
+ [@Image='getClass']
+ [parent::PrimaryExpression
+ [PrimaryPrefix/AllocationExpression]
+ [count(PrimarySuffix) = 2]
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+ // replace this
+Class c = new String().getClass();
+
+ // with this:
+Class c = String.class;
+
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="IdempotentOperations"
+ since="2.0"
+ message="Avoid idempotent operations (like assigning a variable to itself)."
+ class="net.sourceforge.pmd.lang.java.rule.design.IdempotentOperationsRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#IdempotentOperations">
+ <description>
+Avoid idempotent operations - they have no effect.
+ </description>
+ <priority>3</priority>
+
+ <example>
+ <![CDATA[
+public class Foo {
+ public void bar() {
+ int x = 2;
+ x = x;
+ }
+}
+ ]]>
+ </example>
+ </rule>
+<!--
+We are not currently focusing on localization.
+ <rule
+ name="SimpleDateFormatNeedsLocale"
+ language="java"
+ since="2.0"
+ message="When instantiating a SimpleDateFormat object, specify a Locale"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SimpleDateFormatNeedsLocale">
+ <description>
+Be sure to specify a Locale when creating SimpleDateFormat instances to ensure that locale-appropriate
+formatting is used.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//AllocationExpression
+ [ClassOrInterfaceType[@Image='SimpleDateFormat']]
+ [Arguments[@ArgumentCount=1]]
+]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Foo {
+ // Should specify Locale.US (or whatever)
+ private SimpleDateFormat sdf = new SimpleDateFormat("pattern");
+}
+ ]]>
+ </example>
+ </rule>
+-->
+
+ <rule name="ImmutableField"
+ since="2.0"
+ message="Private field ''{0}'' could be made final; it is only initialized in the declaration or constructor."
+ class="net.sourceforge.pmd.lang.java.rule.design.ImmutableFieldRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ImmutableField">
+ <description>
+Identifies private fields whose values never change once they are initialized either in the declaration
+of the field or by a constructor. This helps in converting existing classes to becoming immutable ones.
+ </description>
+ <priority>3</priority>
+
+ <example>
+ <![CDATA[
+public class Foo {
+ private int x; // could be final
+ public Foo() {
+ x = 7;
+ }
+ public void foo() {
+ int a = x + 2;
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+<!--
+We're not currently focusing on localization.
+ <rule name="UseLocaleWithCaseConversions"
+ language="java"
+ since="2.0"
+ message="When doing a String.toLowerCase()/toUpperCase() call, use a Locale"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UseLocaleWithCaseConversions">
+ <description>
+When doing String.toLowerCase()/toUpperCase() conversions, use Locales to avoids problems with languages that
+have unusual conventions, i.e. Turkish.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimaryExpression
+[PrimaryPrefix/Name
+ [ends-with(@Image, 'toLowerCase') or ends-with(@Image,
+'toUpperCase')]
+ ]
+[PrimarySuffix[position() = 1]/Arguments[@ArgumentCount=0]]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+class Foo {
+ // BAD
+ if (x.toLowerCase().equals("list"))...
+ /*
+ This will not match "LIST" when in Turkish locale
+ The above could be
+ if (x.toLowerCase(Locale.US).equals("list")) ...
+ or simply
+ if (x.equalsIgnoreCase("list")) ...
+ */
+ // GOOD
+ String z = a.toLowerCase(Locale.EN);
+}
+ ]]>
+ </example>
+ </rule>
+-->
+ <rule name="AvoidProtectedFieldInFinalClass"
+ language="java"
+ since="2.1"
+ message="Avoid protected fields in a final class. Change to private or package access."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidProtectedFieldInFinalClass">
+ <description>
+Do not use protected fields in final classes since they cannot be subclassed.
+Clarify your intent by using private or package access modifiers instead.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//ClassOrInterfaceDeclaration[@Final='true']
+/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration
+/FieldDeclaration[@Protected='true']
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public final class Bar {
+ private int x;
+ protected int y; // bar cannot be subclassed, so is y really private or package visible?
+ Bar() {}
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="AssignmentToNonFinalStatic"
+ since="2.2"
+ message="Possible unsafe assignment to a non-final static field in a constructor."
+ class="net.sourceforge.pmd.lang.java.rule.design.AssignmentToNonFinalStaticRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AssignmentToNonFinalStatic">
+ <description>
+Identifies a possible unsafe usage of a static field.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class StaticField {
+ static int x;
+ public FinalFields(int y) {
+ x = y; // unsafe
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="MissingStaticMethodInNonInstantiatableClass"
+ language="java"
+ since="3.0"
+ message="Class cannot be instantiated and does not provide any static methods or fields"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#MissingStaticMethodInNonInstantiatableClass">
+ <description>
+A class that has private constructors and does not have any static methods or fields cannot be used.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//ClassOrInterfaceDeclaration[@Nested='false']
+[
+ (
+ count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration)>0
+ and
+ count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration) = count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Private='true'])
+ )
+ and
+ count(.//MethodDeclaration[@Static='true'])=0
+ and
+ count(.//FieldDeclaration[@Private='false'][@Static='true'])=0
+ and
+ count(.//ClassOrInterfaceDeclaration[@Nested='true']
+ [@Public='true']
+ [@Static='true']
+ [count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Public='true']) > 0]
+ [count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/MethodDeclaration
+ [@Public='true']
+ [./ResultType/Type/ReferenceType/ClassOrInterfaceType
+ [@Image = //ClassOrInterfaceDeclaration[@Nested='false']/@Image]
+ ]
+ ) > 0]
+ ) = 0
+]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+// This class is unusable, since it cannot be
+// instantiated (private constructor),
+// and no static method can be called.
+
+public class Foo {
+ private Foo() {}
+ void foo() {}
+}
+
+]]>
+ </example>
+ </rule>
+
+<!--
+We deliberately use method-level synchronization for simplicity in many cases.
+ <rule name="AvoidSynchronizedAtMethodLevel"
+ language="java"
+ since="3.0"
+ message="Use block level rather than method level synchronization"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidSynchronizedAtMethodLevel">
+ <description>
+Method-level synchronization can cause problems when new code is added to the method.
+Block-level synchronization helps to ensure that only the code that needs synchronization
+gets it.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//MethodDeclaration[@Synchronized='true']
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public class Foo {
+ // Try to avoid this:
+ synchronized void foo() {
+ }
+ // Prefer this:
+ void bar() {
+ synchronized(this) {
+ }
+ }
+
+ // Try to avoid this for static methods:
+ static synchronized void fooStatic() {
+ }
+
+ // Prefer this:
+ static void barStatic() {
+ synchronized(Foo.class) {
+ }
+ }
+}
+]]>
+ </example>
+ </rule>
+-->
+
+ <rule name="MissingBreakInSwitch"
+ language="java"
+ since="3.0"
+ message="A switch statement does not contain a break"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#MissingBreakInSwitch">
+ <description>
+Switch statements without break or return statements for each case option
+may indicate problematic behaviour. Empty cases are ignored as these indicate an intentional fall-through.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//SwitchStatement
+[(count(.//BreakStatement)
+ + count(BlockStatement/Statement/ReturnStatement)
+ + count(BlockStatement/Statement/ThrowStatement)
+ + count(SwitchLabel[name(following-sibling::node()) = 'SwitchLabel'])
+ + count(SwitchLabel[count(following-sibling::node()) = 0 or count(child::node()) = 0])
+ < count (SwitchLabel))]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public void bar(int status) {
+ switch(status) {
+ case CANCELLED:
+ doCancelled();
+ // break; hm, should this be commented out?
+ case NEW:
+ doNew();
+ // is this really a fall-through?
+ case REMOVED:
+ doRemoved();
+ // what happens if you add another case after this one?
+ case OTHER: // empty case - this is interpreted as an intentional fall-through
+ case ERROR:
+ doErrorHandling();
+ break;
+ }
+}
+]]>
+ </example>
+ </rule>
+
+
+ <rule name="UseNotifyAllInsteadOfNotify"
+ language="java"
+ since="3.0"
+ message="Call Thread.notifyAll() rather than Thread.notify()"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UseNotifyAllInsteadOfNotify">
+ <description>
+Thread.notify() awakens a thread monitoring the object. If more than one thread is monitoring, then only
+one is chosen. The thread chosen is arbitrary; thus its usually safer to call notifyAll() instead.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//StatementExpression/PrimaryExpression
+[count(PrimarySuffix/Arguments/ArgumentList) = 0]
+[
+PrimaryPrefix[./Name[@Image='notify' or ends-with(@Image,'.notify')]
+or ../PrimarySuffix/@Image='notify'
+or (./AllocationExpression and ../PrimarySuffix[@Image='notify'])
+]
+]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+ void bar() {
+ x.notify();
+ // If many threads are monitoring x, only one (and you won't know which) will be notified.
+ // use instead:
+ x.notifyAll();
+ }
+]]>
+ </example>
+ </rule>
+
+ <rule name="AvoidInstanceofChecksInCatchClause"
+ language="java"
+ since="3.0"
+ message="An instanceof check is being performed on the caught exception. Create a separate catch clause for this exception type."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidInstanceofChecksInCatchClause">
+ <description>
+Each caught exception type should be handled in its own catch clause.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//CatchStatement/FormalParameter
+ /following-sibling::Block//InstanceOfExpression/PrimaryExpression/PrimaryPrefix
+ /Name[
+ @Image = ./ancestor::Block/preceding-sibling::FormalParameter
+ /VariableDeclaratorId/@Image
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+try { // Avoid this
+ // do something
+} catch (Exception ee) {
+ if (ee instanceof IOException) {
+ cleanup();
+ }
+}
+try { // Prefer this:
+ // do something
+} catch (IOException ee) {
+ cleanup();
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="AbstractClassWithoutAbstractMethod"
+ language="java"
+ since="3.0"
+ message="This abstract class does not have any abstract methods"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AbstractClassWithoutAbstractMethod">
+ <description>
+The abstract class does not contain any abstract methods. An abstract class suggests
+an incomplete implementation, which is to be completed by subclasses implementing the
+abstract methods. If the class is intended to be used as a base class only (not to be instantiated
+directly) a protected constructor can be provided prevent direct instantiation.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value><![CDATA[
+//ClassOrInterfaceDeclaration
+ [@Abstract='true'
+ and count( .//MethodDeclaration[@Abstract='true'] )=0 ]
+ [count(ImplementsList)=0]
+ [count(.//ExtendsList)=0]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public abstract class Foo {
+ void int method1() { ... }
+ void int method2() { ... }
+ // consider using abstract methods or removing
+ // the abstract modifier and adding protected constructors
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="SimplifyConditional"
+ language="java"
+ since="3.1"
+ message="No need to check for null before an instanceof"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SimplifyConditional">
+ <description>
+No need to check for null before an instanceof; the instanceof keyword returns false when given a null argument.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//Expression
+ [ConditionalOrExpression
+ [EqualityExpression[@Image='==']
+ //NullLiteral
+ and
+ UnaryExpressionNotPlusMinus
+ [@Image='!']//InstanceOfExpression[PrimaryExpression
+ //Name/@Image = ancestor::ConditionalOrExpression/EqualityExpression
+ /PrimaryExpression/PrimaryPrefix/Name/@Image]
+ and
+ (count(UnaryExpressionNotPlusMinus) + 1 = count(*))
+ ]
+or
+ConditionalAndExpression
+ [EqualityExpression[@Image='!=']//NullLiteral
+ and
+InstanceOfExpression
+ [PrimaryExpression[count(PrimarySuffix[@ArrayDereference='true'])=0]
+ //Name[not(contains(@Image,'.'))]/@Image = ancestor::ConditionalAndExpression
+ /EqualityExpression/PrimaryExpression/PrimaryPrefix/Name/@Image]
+ and
+(count(InstanceOfExpression) + 1 = count(*))
+ ]
+]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+class Foo {
+ void bar(Object x) {
+ if (x != null && x instanceof Bar) {
+ // just drop the "x != null" check
+ }
+ }
+} ]]>
+ </example>
+ </rule>
+
+<rule name="CompareObjectsWithEquals"
+ since="3.2"
+ message="Use equals() to compare object references."
+ class="net.sourceforge.pmd.lang.java.rule.design.CompareObjectsWithEqualsRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#CompareObjectsWithEquals">
+ <description>
+Use equals() to compare object references; avoid comparing them with ==.
+ </description>
+ <priority>3</priority>
+ <example>
+<![CDATA[
+class Foo {
+ boolean bar(String a, String b) {
+ return a == b;
+ }
+}
+
+]]>
+ </example>
+</rule>
+
+
+<rule name="PositionLiteralsFirstInComparisons"
+ language="java"
+ since="3.3"
+ message="Position literals first in String comparisons"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#PositionLiteralsFirstInComparisons">
+ <description>
+Position literals first in comparisons, if the second argument is null then NullPointerExceptions
+can be avoided, they will just return false.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimaryExpression[
+ PrimaryPrefix[Name
+ [
+ (ends-with(@Image, '.equals'))
+ ]
+ ]
+ [
+ (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal)
+ and
+ ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 )
+ ]
+]
+[not(ancestor::Expression/ConditionalAndExpression//EqualityExpression[@Image='!=']//NullLiteral)]
+[not(ancestor::Expression/ConditionalOrExpression//EqualityExpression[@Image='==']//NullLiteral)]
+
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+class Foo {
+ boolean bar(String x) {
+ return x.equals("2"); // should be "2".equals(x)
+ }
+}
+
+]]>
+ </example>
+</rule>
+
+
+<rule name="PositionLiteralsFirstInCaseInsensitiveComparisons"
+ language="java"
+ since="5.1"
+ message="Position literals first in String comparisons for EqualsIgnoreCase"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#PositionLiteralsFirstInCaseInsensitiveComparisons">
+ <description>
+Position literals first in comparisons, if the second argument is null then NullPointerExceptions
+can be avoided, they will just return false.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimaryExpression[
+ PrimaryPrefix[Name
+ [
+ (ends-with(@Image, '.equalsIgnoreCase'))
+ ]
+ ]
+ [
+ (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal)
+ and
+ ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 )
+ ]
+]
+[not(ancestor::Expression/ConditionalAndExpression//EqualityExpression[@Image='!=']//NullLiteral)]
+[not(ancestor::Expression/ConditionalOrExpression//EqualityExpression[@Image='==']//NullLiteral)]
+
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+class Foo {
+ boolean bar(String x) {
+ return x.equalsIgnoreCase("2"); // should be "2".equalsIgnoreCase(x)
+ }
+}
+
+]]>
+ </example>
+</rule>
+
+
+ <rule name="UnnecessaryLocalBeforeReturn"
+ since="3.3"
+ message="Consider simply returning the value vs storing it in local variable ''{0}''"
+ class="net.sourceforge.pmd.lang.java.rule.design.UnnecessaryLocalBeforeReturnRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UnnecessaryLocalBeforeReturn">
+ <description>
+Avoid the creation of unnecessary local variables
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ public int foo() {
+ int x = doSomething();
+ return x; // instead, just 'return doSomething();'
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="NonThreadSafeSingleton"
+ since="3.4"
+ message="Singleton is not thread safe"
+ class="net.sourceforge.pmd.lang.java.rule.design.NonThreadSafeSingletonRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#NonThreadSafeSingleton">
+ <description>
+Non-thread safe singletons can result in bad state changes. Eliminate
+static singletons if possible by instantiating the object directly. Static
+singletons are usually not needed as only a single instance exists anyway.
+Other possible fixes are to synchronize the entire method or to use an
+initialize-on-demand holder class (do not use the double-check idiom).
+
+See Effective Java, item 48.
+ </description>
+ <priority>3</priority>
+ <example><![CDATA[
+private static Foo foo = null;
+
+//multiple simultaneous callers may see partially initialized objects
+public static Foo getFoo() {
+ if (foo==null)
+ foo = new Foo();
+ return foo;
+}
+ ]]></example>
+ </rule>
+
+
+
+ <rule name="UncommentedEmptyMethod"
+ language="java"
+ since="3.4"
+ message="Document empty method"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UncommentedEmptyMethod">
+ <description>
+Uncommented Empty Method finds instances where a method does not contain
+statements, but there is no comment. By explicitly commenting empty methods
+it is easier to distinguish between intentional (commented) and unintentional
+empty methods.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//MethodDeclaration/Block[count(BlockStatement) = 0 and @containsComment = 'false']
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public void doSomething() {
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="UncommentedEmptyConstructor"
+ language="java"
+ since="3.4"
+ message="Document empty constructor"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UncommentedEmptyConstructor">
+ <description>
+Uncommented Empty Constructor finds instances where a constructor does not
+contain statements, but there is no comment. By explicitly commenting empty
+constructors it is easier to distinguish between intentional (commented)
+and unintentional empty constructors.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//ConstructorDeclaration[@Private='false'][count(BlockStatement) = 0 and ($ignoreExplicitConstructorInvocation = 'true' or not(ExplicitConstructorInvocation)) and @containsComment = 'false']
+ ]]>
+ </value>
+ </property>
+ <property name="ignoreExplicitConstructorInvocation" type="Boolean" description="Ignore explicit constructor invocation when deciding whether constructor is empty or not" value="false"/>
+ </properties>
+ <example>
+ <![CDATA[
+public Foo() {
+ // This constructor is intentionally empty. Nothing special is needed here.
+}
+ ]]>
+ </example>
+ </rule>
+
+<rule name="AvoidConstantsInterface"
+ language="java"
+ since="3.5"
+ message="An Interface should be used only to model a behaviour; consider converting this to a class."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidConstantsInterface">
+ <description>
+An interface should be used only to characterize the external behaviour of an
+implementing class: using an interface as a container of constants is a poor
+usage pattern and not recommended.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//ClassOrInterfaceDeclaration[@Interface="true"]
+ [
+ count(.//MethodDeclaration)=0
+ and
+ count(.//FieldDeclaration)>0
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public interface ConstantsInterface {
+ public static final int CONSTANT1=0;
+ public static final String CONSTANT2="1";
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="UnsynchronizedStaticDateFormatter"
+ since="3.6"
+ message="Static DateFormatter objects should be accessed in a synchronized manner"
+ class="net.sourceforge.pmd.lang.java.rule.design.UnsynchronizedStaticDateFormatterRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UnsynchronizedStaticDateFormatter">
+ <description>
+SimpleDateFormat instances are not synchronized. Sun recommends using separate format instances
+for each thread. If multiple threads must access a static formatter, the formatter must be
+synchronized either on method or block level.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ private static final SimpleDateFormat sdf = new SimpleDateFormat();
+ void bar() {
+ sdf.format(); // poor, no thread-safety
+ }
+ synchronized void foo() {
+ sdf.format(); // preferred
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="PreserveStackTrace"
+ since="3.7"
+ message="New exception is thrown in catch block, original stack trace may be lost"
+ class="net.sourceforge.pmd.lang.java.rule.design.PreserveStackTraceRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#PreserveStackTrace">
+ <description>
+Throwing a new exception from a catch block without passing the original exception into the
+new exception will cause the original stack trace to be lost making it difficult to debug
+effectively.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ void good() {
+ try{
+ Integer.parseInt("a");
+ } catch (Exception e) {
+ throw new Exception(e); // first possibility to create exception chain
+ }
+ try {
+ Integer.parseInt("a");
+ } catch (Exception e) {
+ throw (IllegalStateException)new IllegalStateException().initCause(e); // second possibility to create exception chain.
+ }
+ }
+ void bad() {
+ try{
+ Integer.parseInt("a");
+ } catch (Exception e) {
+ throw new Exception(e.getMessage());
+ }
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="UseCollectionIsEmpty"
+ since="3.9"
+ message="Substitute calls to size() == 0 (or size() != 0) with calls to isEmpty()"
+ class="net.sourceforge.pmd.lang.java.rule.design.UseCollectionIsEmptyRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UseCollectionIsEmpty">
+ <description>
+The isEmpty() method on java.util.Collection is provided to determine if a collection has any elements.
+Comparing the value of size() to 0 does not convey intent as well as the isEmpty() method.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ void good() {
+ List foo = getList();
+ if (foo.isEmpty()) {
+ // blah
+ }
+ }
+
+ void bad() {
+ List foo = getList();
+ if (foo.size() == 0) {
+ // blah
+ }
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="ClassWithOnlyPrivateConstructorsShouldBeFinal"
+ language="java"
+ since="4.1"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="A class which only has private constructors should be final"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ClassWithOnlyPrivateConstructorsShouldBeFinal">
+ <description>
+A class with only private constructors should be final, unless the private constructor
+is invoked by a inner class.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="xpath">
+ <value><![CDATA[
+TypeDeclaration[count(../TypeDeclaration) = 1]/ClassOrInterfaceDeclaration
+[@Final = 'false']
+[count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Private = 'true']) >= 1 ]
+[count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[(@Public = 'true') or (@Protected = 'true') or (@PackagePrivate = 'true')]) = 0 ]
+[not(.//ClassOrInterfaceDeclaration)]
+ ]]></value>
+ </property>
+ </properties>
+ <example><![CDATA[
+public class Foo { //Should be final
+ private Foo() { }
+}
+ ]]></example>
+ </rule>
+
+
+ <rule name="EmptyMethodInAbstractClassShouldBeAbstract"
+ language="java"
+ since="4.1"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="An empty method in an abstract class should be abstract instead"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#EmptyMethodInAbstractClassShouldBeAbstract">
+ <description>
+Empty methods in an abstract class should be tagged as abstract. This helps to remove their inapproprate
+usage by developers who should be implementing their own versions in the concrete subclasses.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+ //ClassOrInterfaceDeclaration[@Abstract = 'true']
+ /ClassOrInterfaceBody
+ /ClassOrInterfaceBodyDeclaration
+ /MethodDeclaration[@Abstract = 'false' and @Native = 'false']
+ [
+ ( boolean(./Block[count(./BlockStatement) = 1]/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral) = 'true' )
+ or
+ ( boolean(./Block[count(./BlockStatement) = 1]/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal[@Image = '0']) = 'true' )
+ or
+ ( boolean(./Block[count(./BlockStatement) = 1]/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal[string-length(@Image) = 2]) = 'true' )
+ or
+ (./Block[count(./BlockStatement) = 1]/BlockStatement/Statement/EmptyStatement)
+ or
+ ( count (./Block/*) = 0 )
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public abstract class ShouldBeAbstract {
+ public Object couldBeAbstract() {
+ // Should be abstract method ?
+ return null;
+ }
+
+ public void couldBeAbstract() {
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="SingularField"
+ since="3.1"
+ message="Perhaps ''{0}'' could be replaced by a local variable."
+ class="net.sourceforge.pmd.lang.java.rule.design.SingularFieldRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SingularField">
+ <description>
+ <![CDATA[
+Fields whose scopes are limited to just single methods do not rely on the containing
+object to provide them to other methods. They may be better implemented as local variables
+within those methods.
+ ]]>
+ </description>
+ <priority>3</priority>
+ <example><![CDATA[
+public class Foo {
+ private int x; // no reason to exist at the Foo instance level
+ public void foo(int y) {
+ x = y + 5;
+ return x;
+ }
+}
+ ]]></example>
+ </rule>
+
+ <rule name="ReturnEmptyArrayRatherThanNull"
+ language="java"
+ since="4.2"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="Return an empty array rather than 'null'."
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ReturnEmptyArrayRatherThanNull">
+ <description>
+For any method that returns an array, it is a better to return an empty array rather than a
+null reference. This removes the need for null checking all results and avoids inadvertent
+NullPointerExceptions.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+ //MethodDeclaration
+ [
+ (./ResultType/Type[@Array='true'])
+ and
+ (./Block/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral)
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example><![CDATA[
+public class Example {
+ // Not a good idea...
+ public int[] badBehavior() {
+ // ...
+ return null;
+ }
+
+ // Good behavior
+ public String[] bonnePratique() {
+ //...
+ return new String[0];
+ }
+}
+ ]]></example>
+ </rule>
+
+ <rule name="AbstractClassWithoutAnyMethod"
+ language="java"
+ since="4.2"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="No abstract method which means that the keyword is most likely used to prevent instantiation. Use a private or protected constructor instead."
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AbstractClassWithoutAnyMethod">
+ <description>
+If an abstract class does not provides any methods, it may be acting as a simple data container
+that is not meant to be instantiated. In this case, it is probably better to use a private or
+protected constructor in order to prevent instantiation than make the class misleadingly abstract.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//ClassOrInterfaceDeclaration[
+ (@Abstract = 'true')
+ and
+ (count(//MethodDeclaration) + count(//ConstructorDeclaration) = 0)
+]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class abstract Example {
+ String field;
+ int otherField;
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="TooFewBranchesForASwitchStatement"
+ language="java"
+ since="4.2"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="A switch with less than three branches is inefficient, use a 'if statement' instead."
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#TooFewBranchesForASwitchStatement">
+ <description>
+Switch statements are indended to be used to support complex branching behaviour. Using a switch for only a few
+cases is ill-advised, since switches are not as easy to understand as if-then statements. In these cases use the
+if-then statement to increase code readability.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="minimumNumberCaseForASwitch" type="Integer" description="Minimum number of branches for a switch" min="1" max="100" value="3"/>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//SwitchStatement[
+ (count(.//SwitchLabel) < $minimumNumberCaseForASwitch)
+]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+// With a minimumNumberCaseForASwitch of 3
+public class Foo {
+ public void bar() {
+ switch (condition) {
+ case ONE:
+ instruction;
+ break;
+ default:
+ break; // not enough for a 'switch' stmt, a simple 'if' stmt would have been more appropriate
+ }
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+
+ <rule name="LogicInversion"
+ language="java"
+ since="5.0"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="Use opposite operator instead of the logic complement operator."
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#LogicInversion">
+ <description>
+Use opposite operator instead of negating the whole expression with a logic complement operator.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//UnaryExpressionNotPlusMinus[@Image='!']/PrimaryExpression/PrimaryPrefix/Expression[EqualityExpression or RelationalExpression]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public boolean bar(int a, int b) {
+
+ if (!(a == b)) // use !=
+ return false;
+
+ if (!(a < b)) // use >=
+ return false;
+
+ return true;
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="UseVarargs"
+ language="java"
+ minimumLanguageVersion="1.5"
+ since="5.0"
+ message="Consider using varargs for methods or constructors which take an array the last parameter."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UseVarargs">
+ <description>
+Java 5 introduced the varargs parameter declaration for methods and constructors. This syntactic
+sugar provides flexibility for users of these methods and constructors, allowing them to avoid
+having to deal with the creation of an array.
+</description>
+ <priority>4</priority>
+ <properties>
+ <property name="xpath">
+ <value><![CDATA[
+//FormalParameters/FormalParameter[position()=last() and @Array='true' and @Varargs='false']
+ ]]></value>
+ </property>
+ </properties>
+ <example><![CDATA[
+public class Foo {
+ public void foo(String s, Object[] args) {
+ // Do something here...
+ }
+
+ public void bar(String s, Object... args) {
+ // Ahh, varargs tastes much better...
+ }
+}
+ ]]></example>
+ </rule>
+<!--
+We don't follow this practice, as we often prefer to keep like constants closest to where they are
+used.
+ <rule name="FieldDeclarationsShouldBeAtStartOfClass"
+ language="java"
+ since="5.0"
+ message="Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#FieldDeclarationsShouldBeAtStartOfClass">
+ <description>
+Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/FieldDeclaration
+ [count(../preceding-sibling::ClassOrInterfaceBodyDeclaration/child::*[1]
+ [name() != 'FieldDeclaration' and name() != 'Annotation' and
+ (name() != 'EnumDeclaration' or $ignoreEnumDeclarations = 'false')]) > 0]
+ ]]>
+ </value>
+ </property>
+ <property name="ignoreEnumDeclarations" description="Ignore Enum Declarations that precede fields." type="Boolean" value="true"/>
+ </properties>
+ <example>
+ <![CDATA[
+public class HelloWorldBean {
+
+ // Field declared before methods / inner classes - OK
+ private String _thing;
+
+ public String getMessage() {
+ return "Hello World!";
+ }
+
+ // Field declared after methods / inner classes - avoid this
+ private String _fieldInWrongLocation;
+}
+ ]]>
+ </example>
+ </rule>
+-->
+<!--
+Unfortunately we have several large classes that trip this rule.
+TODO(wfarner): Break apart god classes.
+ <rule name="GodClass"
+ language="java"
+ since="5.0"
+ message="Possible God class"
+ class="net.sourceforge.pmd.lang.java.rule.design.GodClassRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#GodClass">
+ <description>
+The God Class rule detects the God Class design flaw using metrics. God classes do too many things,
+are very big and overly complex. They should be split apart to be more object-oriented.
+The rule uses the detection strategy described in "Object-Oriented Metrics in Practice".
+The violations are reported against the entire class. See also the references:
+Michele Lanza and Radu Marinescu. Object-Oriented Metrics in Practice:
+Using Software Metrics to Characterize, Evaluate, and Improve the Design
+of Object-Oriented Systems. Springer, Berlin, 1 edition, October 2006. Page 80.
+ </description>
+ <priority>3</priority>
+ </rule>
+-->
+ <rule name="AvoidProtectedMethodInFinalClassNotExtending"
+ language="java"
+ since="5.1"
+ message="Avoid protected methods in a final class that doesn't extend anything other than Object. Change to private or package access."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidProtectedMethodInFinalClassNotExtending">
+ <description>
+Do not use protected methods in most final classes since they cannot be subclassed. This should
+only be allowed in final classes that extend other classes with protected methods (whose
+visibility cannot be reduced). Clarify your intent by using private or package access modifiers instead.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//ClassOrInterfaceDeclaration[@Final='true' and not(ExtendsList)]
+/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration
+/MethodDeclaration[@Protected='true']
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public final class Foo {
+ private int bar() {}
+ protected int baz() {} // Foo cannot be subclassed, and doesn't extend anything, so is baz() really private or package visible?
+}
+ ]]>
+ </example>
+ </rule>
+
+</ruleset>
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/config/pmd/logging-java.xml
----------------------------------------------------------------------
diff --git a/config/pmd/logging-java.xml b/config/pmd/logging-java.xml
new file mode 100644
index 0000000..e4ec6ad
--- /dev/null
+++ b/config/pmd/logging-java.xml
@@ -0,0 +1,183 @@
+<?xml version="1.0"?>
+<!--
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this work except in compliance with the License.
+You may obtain a copy of the License in the LICENSE file, or at:
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+<!--
+This is a modified file from the PMD project. Copying and editing ruleset
+configurations is the approach used to modify a ruleset behavior, such as
+disabling individul rules.
+-->
+
+<ruleset name="Java Logging"
+ xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
+
+ <description>
+The Java Logging ruleset contains a collection of rules that find questionable usages of the logger.
+ </description>
+
+ <rule name="MoreThanOneLogger"
+ since="2.0"
+ message="Class contains more than one logger."
+ class="net.sourceforge.pmd.lang.java.rule.logging.MoreThanOneLoggerRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-java.html#MoreThanOneLogger">
+ <description>
+Normally only one logger is used in each class.
+ </description>
+ <priority>2</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ Logger log = Logger.getLogger(Foo.class.getName());
+ // It is very rare to see two loggers on a class, normally
+ // log information is multiplexed by levels
+ Logger log2= Logger.getLogger(Foo.class.getName());
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="LoggerIsNotStaticFinal"
+ language="java"
+ since="2.0"
+ message="The Logger variable declaration does not contain the static and final modifiers"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-java.html#LoggerIsNotStaticFinal">
+ <description>
+In most cases, the Logger reference can be declared as static and final.
+ </description>
+ <priority>2</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//VariableDeclarator
+ [parent::FieldDeclaration]
+ [../Type/ReferenceType
+ /ClassOrInterfaceType[@Image='Logger']
+ and
+ (..[@Final='false'] or ..[@Static = 'false'] ) ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Foo{
+ Logger log = Logger.getLogger(Foo.class.getName()); // not recommended
+
+ static final Logger log = Logger.getLogger(Foo.class.getName()); // preferred approach
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="SystemPrintln"
+ language="java"
+ since="2.1"
+ message="System.out.print is used"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-java.html#SystemPrintln">
+ <description>
+References to System.(out|err).print are usually intended for debugging purposes and can remain in
+the codebase even in production code. By using a logger one can enable/disable this behaviour at
+will (and by priority) and avoid clogging the Standard out log.
+ </description>
+ <priority>2</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//Name[
+ starts-with(@Image, 'System.out.print')
+ or
+ starts-with(@Image, 'System.err.print')
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+class Foo{
+ Logger log = Logger.getLogger(Foo.class.getName());
+ public void testA () {
+ System.out.println("Entering test");
+ // Better use this
+ log.fine("Entering test");
+ }
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="AvoidPrintStackTrace"
+ language="java"
+ since="3.2"
+ message="Avoid printStackTrace(); use a logger call instead."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-java.html#AvoidPrintStackTrace">
+ <description>
+Avoid printStackTrace(); use a logger call instead.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//PrimaryExpression
+ [PrimaryPrefix/Name[contains(@Image,'printStackTrace')]]
+ [PrimarySuffix[not(boolean(Arguments/ArgumentList/Expression))]]
+]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+class Foo {
+ void bar() {
+ try {
+ // do something
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+ }
+}
+]]>
+ </example>
+ </rule>
+<!--
+In practice, we don't have high enough log volume for this rule to be useful.
+
+ <rule name="GuardLogStatementJavaUtil"
+ language="java"
+ since="5.1.0"
+ message="There is log block not surrounded by if"
+ class="net.sourceforge.pmd.lang.java.rule.logging.GuardLogStatementJavaUtilRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-java.html#GuardLogStatementJavaUtil">
+ <description>
+Whenever using a log level, one should check if the loglevel is actually enabled, or
+otherwise skip the associate String creation and manipulation.
+ </description>
+ <priority>2</priority>
+ <example>
+ <![CDATA[
+ // Add this for performance
+ if (log.isLoggable(Level.FINE)) { ...
+ log.fine("This happens");
+]]>
+ </example>
+ </rule>
+-->
+</ruleset>
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/auth/SessionValidator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/auth/SessionValidator.java b/src/main/java/org/apache/aurora/auth/SessionValidator.java
index 6b0f423..eeebb78 100644
--- a/src/main/java/org/apache/aurora/auth/SessionValidator.java
+++ b/src/main/java/org/apache/aurora/auth/SessionValidator.java
@@ -60,7 +60,7 @@ public interface SessionValidator {
/**
* Thrown when authentication is not successful.
*/
- public static class AuthFailedException extends Exception {
+ class AuthFailedException extends Exception {
public AuthFailedException(String msg) {
super(msg);
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/Driver.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/Driver.java b/src/main/java/org/apache/aurora/scheduler/Driver.java
index 15e90d4..ffedfc3 100644
--- a/src/main/java/org/apache/aurora/scheduler/Driver.java
+++ b/src/main/java/org/apache/aurora/scheduler/Driver.java
@@ -76,7 +76,7 @@ public interface Driver {
/**
* Mesos driver implementation.
*/
- static class DriverImpl implements SettableDriver {
+ class DriverImpl implements SettableDriver {
private static final Logger LOG = Logger.getLogger(Driver.class.getName());
/**
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/DriverFactory.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/DriverFactory.java b/src/main/java/org/apache/aurora/scheduler/DriverFactory.java
index 549ef11..0f40112 100644
--- a/src/main/java/org/apache/aurora/scheduler/DriverFactory.java
+++ b/src/main/java/org/apache/aurora/scheduler/DriverFactory.java
@@ -48,7 +48,7 @@ import org.apache.mesos.SchedulerDriver;
*/
public interface DriverFactory extends Function<String, SchedulerDriver> {
- static class DriverFactoryImpl implements DriverFactory {
+ class DriverFactoryImpl implements DriverFactory {
private static final Logger LOG = Logger.getLogger(DriverFactory.class.getName());
@NotNull
@@ -136,11 +136,11 @@ public interface DriverFactory extends Function<String, SchedulerDriver> {
.setCheckpoint(REQUIRE_SLAVE_CHECKPOINT.get())
.setFailoverTimeout(FRAMEWORK_FAILOVER_TIMEOUT.get().as(Time.SECONDS));
- if (frameworkId != null) {
+ if (frameworkId == null) {
+ LOG.warning("Did not find a persisted framework ID, connecting as a new framework.");
+ } else {
LOG.info("Found persisted framework ID: " + frameworkId);
frameworkInfo.setId(FrameworkID.newBuilder().setValue(frameworkId));
- } else {
- LOG.warning("Did not find a persisted framework ID, connecting as a new framework.");
}
if (FRAMEWORK_AUTHENTICATION_FILE.hasAppliedValue()) {
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java b/src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java
index f05b05a..2b97198 100644
--- a/src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java
@@ -30,6 +30,7 @@ import org.apache.aurora.GuiceUtils.AllowUnchecked;
import org.apache.aurora.codec.ThriftBinaryCodec;
import org.apache.aurora.gen.ScheduleStatus;
import org.apache.aurora.gen.comm.SchedulerMessage;
+import org.apache.aurora.gen.comm.SchedulerMessage._Fields;
import org.apache.aurora.scheduler.base.Conversions;
import org.apache.aurora.scheduler.base.SchedulerException;
import org.apache.aurora.scheduler.events.EventSink;
@@ -180,7 +181,7 @@ class MesosSchedulerImpl implements Scheduler {
@Override
public void statusUpdate(SchedulerDriver driver, TaskStatus status) {
String info = status.hasData() ? status.getData().toStringUtf8() : null;
- String infoMsg = info != null ? " with info " + info : "";
+ String infoMsg = info == null ? "" : " with info " + info;
String coreMsg = status.hasMessage() ? " with core message " + status.getMessage() : "";
LOG.info("Received status update for task " + status.getTaskId().getValue()
+ " in state " + status.getState() + infoMsg + coreMsg);
@@ -236,20 +237,16 @@ class MesosSchedulerImpl implements Scheduler {
return;
}
- switch (schedulerMsg.getSetField()) {
- case DELETED_TASKS:
- for (String taskId : schedulerMsg.getDeletedTasks().getTaskIds()) {
- stateManager.changeState(
- taskId,
- Optional.<ScheduleStatus>absent(),
- ScheduleStatus.SANDBOX_DELETED,
- Optional.of("Sandbox disk space reclaimed."));
- }
- break;
-
- default:
- LOG.warning("Received unhandled scheduler message type: " + schedulerMsg.getSetField());
- break;
+ if (schedulerMsg.getSetField() == _Fields.DELETED_TASKS) {
+ for (String taskId : schedulerMsg.getDeletedTasks().getTaskIds()) {
+ stateManager.changeState(
+ taskId,
+ Optional.<ScheduleStatus>absent(),
+ ScheduleStatus.SANDBOX_DELETED,
+ Optional.of("Sandbox disk space reclaimed."));
+ }
+ } else {
+ LOG.warning("Received unhandled scheduler message type: " + schedulerMsg.getSetField());
}
} catch (ThriftBinaryCodec.CodingException e) {
LOG.log(Level.SEVERE, "Failed to decode framework message.", e);
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java b/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
index 06a12a1..f48ca51 100644
--- a/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
+++ b/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
@@ -61,7 +61,7 @@ public interface MesosTaskFactory {
*/
TaskInfo createFrom(IAssignedTask task, SlaveID slaveId) throws SchedulerException;
- static class ExecutorConfig {
+ class ExecutorConfig {
private final String executorPath;
public ExecutorConfig(String executorPath) {
@@ -73,7 +73,7 @@ public interface MesosTaskFactory {
}
}
- static class MesosTaskFactoryImpl implements MesosTaskFactory {
+ class MesosTaskFactoryImpl implements MesosTaskFactory {
private static final Logger LOG = Logger.getLogger(MesosTaskFactoryImpl.class.getName());
private static final String EXECUTOR_PREFIX = "thermos-";
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java b/src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java
index 5c0f09f..48b4404 100644
--- a/src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java
+++ b/src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java
@@ -147,10 +147,7 @@ public class SchedulerLifecycle implements EventSubscriber {
private final LeadingOptions leadingOptions;
private final ScheduledExecutorService executorService;
- private DefaultDelayedActions(
- LeadingOptions leadingOptions,
- ScheduledExecutorService executorService) {
-
+ DefaultDelayedActions(LeadingOptions leadingOptions, ScheduledExecutorService executorService) {
this.leadingOptions = checkNotNull(leadingOptions);
this.executorService = checkNotNull(executorService);
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java b/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java
index e2492de..cf22e9c 100644
--- a/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java
+++ b/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java
@@ -104,13 +104,13 @@ public interface OfferQueue extends EventSubscriber {
* The delay is calculated for each offer that is received, so the return delay may be
* fixed or variable.
*/
- public interface OfferReturnDelay extends Supplier<Amount<Integer, Time>> {
+ interface OfferReturnDelay extends Supplier<Amount<Integer, Time>> {
}
/**
* Thrown when there was an unexpected failure trying to launch a task.
*/
- static class LaunchException extends Exception {
+ class LaunchException extends Exception {
LaunchException(String msg) {
super(msg);
}
@@ -151,7 +151,13 @@ public interface OfferQueue extends EventSubscriber {
// There's also a chance that we return an offer for compaction ~simultaneously with the
// same-host offer being canceled/returned. This is also fine.
Optional<HostOffer> sameSlave = hostOffers.get(offer.getSlaveId());
- if (!sameSlave.isPresent()) {
+ if (sameSlave.isPresent()) {
+ // If there are existing offers for the slave, decline all of them so the master can
+ // compact all of those offers into a single offer and send them back.
+ LOG.info("Returning offers for " + offer.getSlaveId().getValue() + " for compaction.");
+ decline(offer.getId());
+ removeAndDecline(sameSlave.get().offer.getId());
+ } else {
hostOffers.add(new HostOffer(offer, maintenance.getMode(offer.getHostname())));
executor.schedule(
new Runnable() {
@@ -162,12 +168,6 @@ public interface OfferQueue extends EventSubscriber {
},
returnDelay.get().as(Time.MILLISECONDS),
TimeUnit.MILLISECONDS);
- } else {
- // If there are existing offers for the slave, decline all of them so the master can
- // compact all of those offers into a single offer and send them back.
- LOG.info("Returning offers for " + offer.getSlaveId().getValue() + " for compaction.");
- decline(offer.getId());
- removeAndDecline(sameSlave.get().offer.getId());
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java b/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
index 17ddfc9..8a8e6e2 100644
--- a/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
+++ b/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
@@ -34,7 +34,6 @@ import com.twitter.common.quantity.Amount;
import com.twitter.common.quantity.Time;
import com.twitter.common.stats.Stats;
import com.twitter.common.util.BackoffStrategy;
-import com.twitter.common.util.Clock;
import com.twitter.common.util.concurrent.ExecutorServiceShutdown;
import org.apache.aurora.scheduler.base.AsyncUtil;
@@ -87,7 +86,6 @@ public class TaskGroups implements EventSubscriber {
ShutdownRegistry shutdownRegistry,
TaskGroupsSettings settings,
TaskScheduler taskScheduler,
- Clock clock,
RescheduleCalculator rescheduleCalculator) {
this(