Add findbugs maven plugin (#973)

* Add basic findbugs configuration

* Refactor code to pass findbugs

* Replace `Cloneable` interface with `copy` method

* Fix bug with using static date format

* Use `StringBuilder` instead of concatenating `String`

* Replace `keySet` iteration with `entrySet`

* Remove redundant guava dependencies

* Fix findbugs issue with unused return value

* Fix mistake with `AtomicQuery#equals`

* Add implementation of abstract method

* Use `Instant` instead of `Date` in tasks

* Use `%n` instead of `\n` in format string

* Make sure `FileInputStream` instances are always closed

* Make sure a lock is released

* Don't do unnecessary unboxing

* Add default switch case

* Use string builder for string concatenation in a loop

* stop `TaskState` being `Serializable`

* Add `removeInstance` static methods

* Handle null cases

* Add default case to switch statement

* Convert `Json` to `String` correctly

* Fixes after rebase

* Fix minor issue in graql components

* Add hashcode method to ReasonerAtomicQuery. Get Kasper to double check this definition

* Resolve bad locking in fctory

* Specify default encoding in SystemKeyspace

* Specify default encoding in EngineCommunicator

* Add specific encoding to engine

* Remove synchronisatin from GraknStateSTorage

* Sort our encoding in graql shell

* Make `COMMAND` immutable

* Add default to switch case

* Remove Ctrl-C handling code

This used internal sun classes to operate and didn't work very well.

* Specify default encoding for stdout

* Refactor history file creation

* Change message handling in graql shell to occur synchronously

* Add explicit encodings to migration

* Fix bug with mix-up of seconds and milliseconds

* Fix bug with incorrect arguments to `Duration.between`

findbugs-exclude.xml

@@ -0,0 +1,6 @@
+<FindBugsFilter>
+    <!-- Exclude generated ANTLR classes -->
+    <Match>
+        <Class name="~ai\.grakn\.graql\.internal\.antlr\..*"/>
+    </Match>
+</FindBugsFilter>

grakn-core/src/main/java/ai/grakn/concept/ResourceType.java

@@ -20,10 +20,9 @@
 
 
 import ai.grakn.util.Schema;
+import com.google.common.collect.ImmutableMap;
 
 import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
 
 /**
  * A ResourceType is a schema element which represents the category of resources in the graph.
@@ -154,15 +153,14 @@ class DataType<D> {
         public static final DataType<Boolean> BOOLEAN = new DataType<>(Boolean.class.getName(), Schema.ConceptProperty.VALUE_BOOLEAN);
         public static final DataType<Long> LONG = new DataType<>(Long.class.getName(), Schema.ConceptProperty.VALUE_LONG);
         public static final DataType<Double> DOUBLE = new DataType<>(Double.class.getName(), Schema.ConceptProperty.VALUE_DOUBLE);
-        public static final Map<String, DataType<?>> SUPPORTED_TYPES = new HashMap<>();
-
-        static {
-            SUPPORTED_TYPES.put(STRING.getName(), STRING);
-            SUPPORTED_TYPES.put(BOOLEAN.getName(), BOOLEAN);
-            SUPPORTED_TYPES.put(LONG.getName(), LONG);
-            SUPPORTED_TYPES.put(DOUBLE.getName(), DOUBLE);
-            SUPPORTED_TYPES.put(Integer.class.getName(), LONG);
-        }
+
+        public static final ImmutableMap<String, DataType<?>> SUPPORTED_TYPES = ImmutableMap.of(
+                STRING.getName(), STRING,
+                BOOLEAN.getName(), BOOLEAN,
+                LONG.getName(), LONG,
+                DOUBLE.getName(), DOUBLE,
+                Integer.class.getName(), LONG
+        );
 
         private final String dataType;
         private final Schema.ConceptProperty conceptProperty;

grakn-core/src/main/java/ai/grakn/graql/admin/Atomic.java

@@ -32,9 +32,9 @@
  * @author Kasper Piskorski
  *
  */
-public interface Atomic extends Cloneable{
+public interface Atomic {
 
-    Atomic clone();
+    Atomic copy();
 
     default boolean isAtom(){ return false;}
     default boolean isPredicate(){ return false;}

grakn-engine/src/main/java/ai/grakn/engine/GraknEngineServer.java

@@ -45,8 +45,8 @@
 import java.io.InputStream;
 import java.net.HttpURLConnection;
 import java.net.URL;
+import java.time.Instant;
 import java.util.Arrays;
-import java.util.Date;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -129,7 +129,7 @@ public static void startCluster() {
         manager.open();
         manager.scheduleTask(new PostProcessingTask(),
                              GraknEngineServer.class.getName(),
-                             new Date(),
+                             Instant.now(),
                              prop.getPropertyAsInt(ConfigProperties.TIME_LAPSE),
                              new JSONObject());
 

grakn-engine/src/main/java/ai/grakn/engine/backgroundtasks/StateStorage.java

@@ -21,15 +21,15 @@
 import javafx.util.Pair;
 import org.json.JSONObject;
 
-import java.util.Date;
+import java.time.Instant;
 import java.util.Set;
 
 public interface StateStorage {
     /**
      * Create a new task state and store it, returning an ID to later access this task state.
      * @param taskName String class name of object implementing the BackgroundTask interface. This must not be null.
      * @param createdBy String of who is creating this new state. This must not be null.
-     * @param runAt Date when should this task be executed. This must not be null.
+     * @param runAt Instant when should this task be executed. This must not be null.
      * @param recurring Boolean marking if this task should be run again after it has finished executing successfully.
      *                  This must not be null.
      * @param interval If a task is marked as recurring, this represents the time delay between the next executing of this task.
@@ -42,7 +42,7 @@ public interface StateStorage {
      */
     String newState(String taskName,
                     String createdBy,
-                    Date runAt,
+                    Instant runAt,
                     Boolean recurring,
                     long interval,
                     JSONObject configuration);

grakn-engine/src/main/java/ai/grakn/engine/backgroundtasks/TaskManager.java

@@ -20,23 +20,23 @@
 
 import org.json.JSONObject;
 
-import java.util.Date;
+import java.time.Instant;
 import java.util.concurrent.CompletableFuture;
 
 public interface TaskManager extends AutoCloseable {
     /**
      * Schedule a single shot/one off BackgroundTask to run after a @delay in milliseconds. All parameters must not be
      * null unless stated otherwise.
      * @param task Any object implementing the BackgroundTask interface that is to be scheduled for later execution.
-     * @param runAt Date when task should run.
+     * @param runAt Instant when task should run.
      * @param period A non-zero value indicates that this should be a recurring task and period indicates the delay between
      *               subsequent runs of the task after successful execution.
      * @param configuration A JSONObject instance containing configuration and optionally data for the task. This is an
      *                      optional parameter and may be set to null to not pass any configuration (task.start() will
      *                      get an initialised but empty JSONObject).
      * @return Assigned ID of task scheduled for later execution.
      */
-    String scheduleTask(BackgroundTask task, String createdBy, Date runAt, long period, JSONObject configuration);
+    String scheduleTask(BackgroundTask task, String createdBy, Instant runAt, long period, JSONObject configuration);
 
     /**
      * Return a future that allows registering asynchronous callbacks triggered when a task is completed.

grakn-engine/src/main/java/ai/grakn/engine/backgroundtasks/TaskState.java

@@ -20,21 +20,20 @@
 
 import org.json.JSONObject;
 
-import java.io.Serializable;
-import java.util.Date;
+import java.time.Instant;
 
 /**
  * Internal task state model used to keep track of scheduled tasks.
  */
-public class TaskState implements Cloneable, Serializable {
+public class TaskState implements Cloneable {
     /**
      * Task status, @see TaskStatus.
      */
     private TaskStatus status;
     /**
      * Time when task status was last updated.
      */
-    private Date statusChangeTime;
+    private Instant statusChangeTime;
     /**
      * String identifying who last updated task status.
      */
@@ -54,7 +53,7 @@ public class TaskState implements Cloneable, Serializable {
     /**
      * When this task should be executed.
      */
-    private Date runAt;
+    private Instant runAt;
     /**
      * Should this task be run again after it has finished executing successfully.
      */
@@ -91,12 +90,12 @@ public TaskStatus status() {
         return status;
     }
 
-    public TaskState statusChangeTime(Date statusChangeTime) {
+    public TaskState statusChangeTime(Instant statusChangeTime) {
         this.statusChangeTime = statusChangeTime;
         return this;
     }
 
-    public Date statusChangeTime() {
+    public Instant statusChangeTime() {
         return statusChangeTime;
     }
 
@@ -131,12 +130,12 @@ public String engineID() {
         return engineID;
     }
 
-    public TaskState runAt(Date runAt) {
+    public TaskState runAt(Instant runAt) {
         this.runAt = runAt;
         return this;
     }
 
-    public Date runAt() {
+    public Instant runAt() {
         return runAt;
     }
 

grakn-engine/src/main/java/ai/grakn/engine/backgroundtasks/distributed/DistributedTaskManager.java

@@ -32,7 +32,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Date;
+import java.time.Instant;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -96,7 +96,7 @@ public void close() {
     }
 
     @Override
-    public String scheduleTask(BackgroundTask task, String createdBy, Date runAt, long period, JSONObject configuration) {
+    public String scheduleTask(BackgroundTask task, String createdBy, Instant runAt, long period, JSONObject configuration) {
         Boolean recurring = period > 0;
 
         String id = stateStorage.newState(task.getClass().getName(), createdBy, runAt, recurring, period, configuration);

grakn-engine/src/main/java/ai/grakn/engine/backgroundtasks/distributed/Scheduler.java

@@ -33,8 +33,9 @@
 import org.apache.kafka.common.TopicPartition;
 import org.apache.kafka.common.errors.WakeupException;
 
+import java.time.Duration;
+import java.time.Instant;
 import java.util.Collections;
-import java.util.Date;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executors;
@@ -87,7 +88,7 @@ public void run() {
                 ConsumerRecords<String, String> records = consumer.poll(properties.getPropertyAsInt(SCHEDULER_POLLING_FREQ));
 
                 for(ConsumerRecord<String, String> record:records) {
-                    LOG.debug(String.format("Scheduler received topic = %s, partition = %s, offset = %s, taskid = %s, value = %s\n",
+                    LOG.debug(String.format("Scheduler received topic = %s, partition = %s, offset = %s, taskid = %s, value = %s%n",
                             record.topic(), record.partition(), record.offset(), record.key(), record.value()));
 
                     scheduleTask(record.key(), record.value());
@@ -183,7 +184,7 @@ private void scheduleTask(String id, String configuration) {
      * @param state state of the task
      */
     private void scheduleTask(String id,  String configuration, TaskState state) {
-        long delay = state.runAt().getTime() - new Date().getTime();
+        long delay = Duration.between(Instant.now(), state.runAt()).toMillis();
 
         markAsScheduled(id);
         if(state.isRecurring()) {

grakn-engine/src/main/java/ai/grakn/engine/backgroundtasks/distributed/TaskFailover.java

@@ -31,6 +31,7 @@
 import org.apache.kafka.clients.producer.ProducerRecord;
 import org.json.JSONArray;
 
+import java.nio.charset.StandardCharsets;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
@@ -146,7 +147,7 @@ private void reQueue(CuratorFramework client, String engineID) throws Exception
         byte[] b = client.getData().forPath(RUNNERS_STATE+"/"+engineID);
 
         // Re-queue all of the IDs.
-        JSONArray ids = new JSONArray(new String(b));
+        JSONArray ids = new JSONArray(new String(b, StandardCharsets.UTF_8));
         for(Object o: ids) {
             String id = (String)o;
 

grakn-engine/src/main/java/ai/grakn/engine/backgroundtasks/distributed/TaskRunner.java

@@ -37,6 +37,7 @@
 import org.json.JSONArray;
 import org.json.JSONObject;
 
+import java.nio.charset.StandardCharsets;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
@@ -343,7 +344,7 @@ private void updateOwnState() {
         out.put(runningTasks);
 
         try {
-            zkStorage.connection().setData().forPath(RUNNERS_STATE+"/"+ engineID, out.toString().getBytes());
+            zkStorage.connection().setData().forPath(RUNNERS_STATE+"/"+ engineID, out.toString().getBytes(StandardCharsets.UTF_8));
         }
         catch (Exception e) {
             LOG.error("Could not update TaskRunner taskstorage in ZooKeeper! " + e);

grakn-engine/src/main/java/ai/grakn/engine/backgroundtasks/standalone/StandaloneTaskManager.java

@@ -29,7 +29,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Date;
+import java.time.Duration;
+import java.time.Instant;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
@@ -90,16 +91,20 @@ public TaskManager open() {
     public void close(){
         executorService.shutdown();
         schedulingService.shutdown();
+        removeInstance();
+    }
+
+    private static void removeInstance() {
         instance = null;
     }
 
-    public String scheduleTask(BackgroundTask task, String createdBy, Date runAt, long period, JSONObject configuration) {
+    public String scheduleTask(BackgroundTask task, String createdBy, Instant runAt, long period, JSONObject configuration) {
         Boolean recurring = (period != 0);
         String id = stateStorage.newState(task.getClass().getName(), createdBy, runAt, recurring, period, configuration);
 
         // Schedule task to run.
-        Date now = new Date();
-        long delay = now.getTime() - runAt.getTime();
+        Instant now = Instant.now();
+        long delay = Duration.between(runAt, now).toMillis();
 
         try {
             stateStorage.updateState(id, SCHEDULED, this.getClass().getName(), null, null, null, null);
@@ -150,37 +155,38 @@ public CompletableFuture completableFuture(String taskId) {
     }
 
     public TaskManager stopTask(String id, String requesterName) {
-        stateUpdateLock.lock();
+        try {
+            stateUpdateLock.lock();
 
-        TaskState state = stateStorage.getState(id);
-        if(state == null) {
-            return this;
-        }
+            TaskState state = stateStorage.getState(id);
+            if (state == null) {
+                return this;
+            }
 
-        Pair<ScheduledFuture<?>, BackgroundTask> pair = instantiatedTasks.get(id);
-        String name = this.getClass().getName();
+            Pair<ScheduledFuture<?>, BackgroundTask> pair = instantiatedTasks.get(id);
+            String name = this.getClass().getName();
 
-        synchronized (pair) {
-            if(state.status() == SCHEDULED || (state.status() == COMPLETED && state.isRecurring())) {
-                LOG.info("Stopping a currently scheduled task "+id);
-                pair.getKey().cancel(true);
-                stateStorage.updateState(id, STOPPED, name,null, null, null, null);
-            }
-            else if(state.status() == RUNNING) {
-                LOG.info("Stopping running task "+id);
+            synchronized (pair) {
+                if (state.status() == SCHEDULED || (state.status() == COMPLETED && state.isRecurring())) {
+                    LOG.info("Stopping a currently scheduled task " + id);
+                    pair.getKey().cancel(true);
+                    stateStorage.updateState(id, STOPPED, name, null, null, null, null);
+                } else if (state.status() == RUNNING) {
+                    LOG.info("Stopping running task " + id);
 
-                BackgroundTask task = pair.getValue();
-                if(task != null) {
-                    task.stop();
-                }
+                    BackgroundTask task = pair.getValue();
+                    if (task != null) {
+                        task.stop();
+                    }
 
-                stateStorage.updateState(id, STOPPED, name, null, null, null, null);
-            }
-            else {
-                LOG.warn("Task not running - "+id);
+                    stateStorage.updateState(id, STOPPED, name, null, null, null, null);
+                } else {
+                    LOG.warn("Task not running - " + id);
+                }
             }
+        } finally {
+            stateUpdateLock.unlock();
         }
-        stateUpdateLock.unlock();
 
         return this;
     }