Minor fixes

- Remove commented out code
- Uncomment debug log
- Use Set backed by ConcurrentHashMap instead of Synchronized Set
- Correct grammatical mistakes
- Add ThreadSafe annotation

Author: Vickey Singh

pom.xml

@@ -4,7 +4,7 @@
          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
 
     <groupId>com.flipkart.databuilderframework</groupId>
-    <version>0.5.12</version>
+    <version>0.5.13-SNAPSHOT</version>
     <modelVersion>4.0.0</modelVersion>
 
     <artifactId>databuilderframework</artifactId>

src/main/java/com/flipkart/databuilderframework/annotations/ThreadSafe.java

@@ -0,0 +1,15 @@
+package com.flipkart.databuilderframework.annotations;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Annotation to mark a class as thread safe.
+ */
+@Retention(RetentionPolicy.SOURCE)
+@Target(ElementType.TYPE)
+public @interface ThreadSafe {
+
+}

src/main/java/com/flipkart/databuilderframework/engine/DataBuilderContext.java

@@ -45,7 +45,7 @@ public DataSet getDataSet(DataBuilder builder) {
 
     /**
      * Access the data set. Ideally a builder should only access data as declared.
-     * Deprecated: This methpd will be removed in the near future. Do not use this in newer projects.
+     * Deprecated: This method will be removed in the near future. Do not use this in newer projects.
      * @return The full data set.
      */
     @Deprecated

src/main/java/com/flipkart/databuilderframework/engine/ExecutionGraphGenerator.java

@@ -79,7 +79,6 @@ public ExecutionGraph generateGraph(final DataFlow dataFlow) throws DataBuilderF
         List<List<DataBuilderMeta>> dependencyHierarchy
                 = TimedExecutor.run("ExecutionGraphGenerator::buildHierarchy", () -> buildHierarchy(dependencyInfoManager, maxHeight));
 
-        //Return
         return new ExecutionGraph(dependencyHierarchy);
     }
 

src/main/java/com/flipkart/databuilderframework/engine/MultiThreadedDataFlowExecutor.java

@@ -1,5 +1,6 @@
 package com.flipkart.databuilderframework.engine;
 
+import com.flipkart.databuilderframework.annotations.ThreadSafe;
 import com.flipkart.databuilderframework.model.*;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
@@ -14,6 +15,7 @@
 /**
  * The executor for a {@link com.flipkart.databuilderframework.model.DataFlow}.
  */
+@ThreadSafe
 public class MultiThreadedDataFlowExecutor extends DataFlowExecutor {
     private static final Logger logger = LoggerFactory.getLogger(MultiThreadedDataFlowExecutor.class.getSimpleName());
     private final ExecutorService executorService;
@@ -49,7 +51,7 @@ protected DataExecutionResponse run(DataBuilderContext dataBuilderContext,
         }
         List<List<DataBuilderMeta>> dependencyHierarchy = executionGraph.getDependencyHierarchy();
         Set<String> newlyGeneratedData = Sets.newHashSet();
-        Set<DataBuilderMeta> processedBuilders = Collections.synchronizedSet(Sets.<DataBuilderMeta>newHashSet());
+        Set<DataBuilderMeta> processedBuilders = ConcurrentHashMap.newKeySet();
         while(true) {
             for (List<DataBuilderMeta> levelBuilders : dependencyHierarchy) {
                 List<Future<DataContainer>> dataFutures = Lists.newArrayList();
@@ -109,18 +111,13 @@ protected DataExecutionResponse run(DataBuilderContext dataBuilderContext,
                 }
             }
             if(newlyGeneratedData.contains(dataFlow.getTargetData())) {
-                //logger.debug("Finished running this instance of the flow. Exiting.");
+                logger.debug("Finished running this instance of the flow. Exiting.");
                 break;
             }
             if(newlyGeneratedData.isEmpty()) {
-                //logger.debug("Nothing happened in this loop, exiting..");
+                logger.debug("Nothing happened in this loop, exiting..");
                 break;
             }
-//            StringBuilder stringBuilder = new StringBuilder();
-//            for(String data : newlyGeneratedData) {
-//                stringBuilder.append(data + ", ");
-//            }
-            //logger.info("Newly generated: " + stringBuilder);
             activeDataSet.clear();
             activeDataSet.addAll(newlyGeneratedData);
             newlyGeneratedData.clear();

src/main/java/com/flipkart/databuilderframework/engine/OptimizedMultiThreadedDataFlowExecutor.java

@@ -1,5 +1,6 @@
 package com.flipkart.databuilderframework.engine;
 
+import com.flipkart.databuilderframework.annotations.ThreadSafe;
 import com.flipkart.databuilderframework.model.*;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
@@ -15,6 +16,7 @@
 /**
  * The executor for a {@link com.flipkart.databuilderframework.model.DataFlow}.
  */
+@ThreadSafe
 public class OptimizedMultiThreadedDataFlowExecutor extends DataFlowExecutor {
     private static final Logger logger = LoggerFactory.getLogger(OptimizedMultiThreadedDataFlowExecutor.class.getSimpleName());
     private final ExecutorService executorService;
@@ -50,11 +52,11 @@ protected DataExecutionResponse run(DataBuilderContext dataBuilderContext,
         }
         List<List<DataBuilderMeta>> dependencyHierarchy = executionGraph.getDependencyHierarchy();
         Set<String> newlyGeneratedData = Sets.newHashSet();
-        Set<DataBuilderMeta> processedBuilders = Collections.synchronizedSet(Sets.<DataBuilderMeta>newHashSet());
+        Set<DataBuilderMeta> processedBuilders = ConcurrentHashMap.newKeySet();
         while(true) {
             for (List<DataBuilderMeta> levelBuilders : dependencyHierarchy) {
                 List<Future<DataContainer>> dataFutures = Lists.newArrayList();
-                BuilderRunner singleRef = null; //refrence to builderRunner when size of levelBuilders == 1 to avoid running it behind thread
+                BuilderRunner singleRef = null; //reference to builderRunner when size of levelBuilders == 1 to avoid running it behind thread
                 for (DataBuilderMeta builderMeta : levelBuilders) {
                     if (processedBuilders.contains(builderMeta)) {
                         continue;
@@ -128,18 +130,13 @@ protected DataExecutionResponse run(DataBuilderContext dataBuilderContext,
                 }
             }
             if(newlyGeneratedData.contains(dataFlow.getTargetData())) {
-                //logger.debug("Finished running this instance of the flow. Exiting.");
+                logger.debug("Finished running this instance of the flow. Exiting.");
                 break;
             }
             if(newlyGeneratedData.isEmpty()) {
-                //logger.debug("Nothing happened in this loop, exiting..");
+                logger.debug("Nothing happened in this loop, exiting..");
                 break;
             }
-//            StringBuilder stringBuilder = new StringBuilder();
-//            for(String data : newlyGeneratedData) {
-//                stringBuilder.append(data + ", ");
-//            }
-            //logger.info("Newly generated: " + stringBuilder);
             activeDataSet.clear();
             activeDataSet.addAll(newlyGeneratedData);
             newlyGeneratedData.clear();

src/main/java/com/flipkart/databuilderframework/engine/SimpleDataFlowExecutor.java

@@ -1,5 +1,6 @@
 package com.flipkart.databuilderframework.engine;
 
+import com.flipkart.databuilderframework.annotations.ThreadSafe;
 import com.flipkart.databuilderframework.model.*;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Maps;
@@ -16,6 +17,7 @@
 /**
  * The executor for a {@link com.flipkart.databuilderframework.model.DataFlow}.
  */
+@ThreadSafe
 public class SimpleDataFlowExecutor extends DataFlowExecutor {
     private static final Logger logger = LoggerFactory.getLogger(SimpleDataFlowExecutor.class.getSimpleName());
 
@@ -145,18 +147,14 @@ protected DataExecutionResponse run(DataBuilderContext dataBuilderContext,
                 }
             }
             if(newlyGeneratedData.contains(dataFlow.getTargetData())) {
-                //logger.debug("Finished running this instance of the flow. Exiting.");
+                // Found the target data node
+                logger.debug("Finished running this instance of the flow. Exiting.");
                 break;
             }
             if(newlyGeneratedData.isEmpty()) {
-                //logger.debug("Nothing happened in this loop, exiting..");
+                logger.debug("Nothing happened in this loop, exiting..");
                 break;
             }
-//            StringBuilder stringBuilder = new StringBuilder();
-//            for(String data : newlyGeneratedData) {
-//                stringBuilder.append(data + ", ");
-//            }
-            //logger.info("Newly generated: " + stringBuilder);
             activeDataSet.clear();
             activeDataSet.addAll(newlyGeneratedData);
             newlyGeneratedData.clear();

src/main/java/com/flipkart/databuilderframework/model/Data.java

@@ -5,15 +5,17 @@
 import lombok.ToString;
 
 import javax.validation.constraints.NotNull;
+import java.io.Serializable;
 
 /**
  * Data base class. All data classes to be considered by a DataBuilder need to inherit from this
  */
 @JsonTypeInfo(use= JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "data")
 @lombok.Data
 @ToString
-public abstract class Data {
+public abstract class Data implements Serializable {
 
+    private static final long serialVersionUID = 3237890729688723887L;
     /**
      * The type-name for data. This is an app defined tag for this data.
      * Needs to be set by the inheriting class.

src/main/java/com/flipkart/databuilderframework/model/DataFlow.java

@@ -4,11 +4,13 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.flipkart.databuilderframework.engine.DataBuilderFactory;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
 import lombok.Builder;
 import org.hibernate.validator.constraints.NotEmpty;
 
 import javax.validation.constraints.NotNull;
 import java.io.Serializable;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
@@ -105,13 +107,13 @@ public DataFlow() {
 
     public DataFlow deepCopy() {
         return new DataFlow(name,
-                            description,
-                            targetData,
-                            resolutionSpecs,
-                            executionGraph.deepCopy(),
-                            transients,
-                            enabled,
-                            loopingEnabled,
-                            dataBuilderFactory);
+                description,
+                targetData,
+                Maps.newHashMap(resolutionSpecs),
+                executionGraph.deepCopy(),
+                Sets.newHashSet(transients),
+                enabled,
+                loopingEnabled,
+                dataBuilderFactory);
     }
 }

src/main/java/com/flipkart/databuilderframework/model/DataFlowInstance.java

@@ -4,14 +4,16 @@
 import org.hibernate.validator.constraints.NotEmpty;
 
 import javax.validation.constraints.NotNull;
+import java.io.Serializable;
 
 /**
  * A instance of the {@link com.flipkart.databuilderframework.model.DataFlow} object to be used for execution.
  * This class represents a running instance of the flow. It contains the flow itself as well as the
  * {@link com.flipkart.databuilderframework.model.DataSet} required for execution.
  */
-public class DataFlowInstance {
+public class DataFlowInstance implements Serializable {
 
+    private static final long serialVersionUID = 7886384565411534126L;
     /**
      * The ID of the flow instance.
      */

src/main/java/com/flipkart/databuilderframework/model/DataSet.java

@@ -7,13 +7,16 @@
 import org.hibernate.validator.constraints.NotEmpty;
 
 import javax.validation.constraints.NotNull;
+import java.io.Serializable;
 import java.util.Map;
 
 /**
  * Working set for data
  * Contains all the data that has been provided by the system as well as those generated by the system.
  */
-public class DataSet {
+public class DataSet implements Serializable {
+
+    private static final long serialVersionUID = -7453209452035943145L;
     /**
      * All data present for this flow execution.
      */