Moving init and checks to constructor for EdgeSpec and VertexSpec. Ad…

…ded test coverage.

src/main/groovy/graph/EdgeSpec.groovy

@@ -9,13 +9,14 @@ class EdgeSpec {
     private Graph graph
     private Edge edge
     private boolean edgePresentInGraph
-    private Object from
-    private Object to
+    Object from
+    Object to
     private Object changeFrom
     private Object changeTo
     private List dslProperties
     private Map entries
     private Closure runnerCodeClosure
+    private boolean applied
 
     /**
      * Gets the graph the {@link Edge} has been added to. This can be used inside the runnerCode to access the graph.
@@ -46,6 +47,12 @@ class EdgeSpec {
     }
 
     EdgeSpec(Graph graph, Map<String, ?> map, Closure closure = null) {
+        if(!graph) {
+            throw new IllegalArgumentException('graph must be set.')
+        }
+        if(!map) {
+            throw new IllegalArgumentException('map must be set.')
+        }
         this.graph = graph
 
         dslProperties = ['from', 'to', 'changeFrom', 'changeTo']
@@ -58,28 +65,7 @@ class EdgeSpec {
         changeTo = map.changeTo
 
         runnerCodeClosure = closure
-    }
-
-    /**
-     * Changes edge.from to changeFrom. This allows edge.from to point to a different {@link graph.Vertex}
-     * @param changeFrom the new id for edge.from
-     */
-    void changeFrom(Object changeFrom) {
-        graph.newEdgeSpec([from:edge.from, to:edge.to, changeFrom:changeFrom]).apply()
-    }
 
-    /**
-     * Changes edge.to to changeTo. This allows edge.two to point to a different {@link graph.Vertex}.
-     * @param changeTo  the new id for edge.to
-     */
-    void changeTo(Object changeTo) {
-        graph.newEdgeSpec([from:edge.from, to:edge.to, changeTo:changeTo]).apply()
-    }
-
-    protected init() {
-        if (edge) {
-            throw new IllegalStateException('Edge already created.')
-        }
         Edge created = graph.newEdge(from:from, to:to)
         Edge existing = graph.edges.find { it == created }
         edgePresentInGraph = existing != null
@@ -96,21 +82,12 @@ class EdgeSpec {
         }
 
         edge = existing ?: created
-    }
-
-    protected void checkConditions() {
         if (!from) {
             throw new IllegalStateException('!from failed. from must be groovy truth.')
         }
         if (!to) {
             throw new IllegalStateException('!to failed. to must be groovy truth.')
         }
-        if (!graph) {
-            throw new IllegalStateException('graph is not set.')
-        }
-        if (!edge) {
-            throw new IllegalStateException('edge is not set.')
-        }
         if (changeFrom || changeTo) {
             Edge renamed = graph.newEdge(from:changeFrom ?: from, to:changeTo ?: to)
             if (graph.edges.find { it == renamed }) {
@@ -119,7 +96,23 @@ class EdgeSpec {
         }
     }
 
-    protected void setupGraph() {
+    /**
+     * Changes edge.from to changeFrom. This allows edge.from to point to a different {@link graph.Vertex}
+     * @param changeFrom the new id for edge.from
+     */
+    void changeFrom(Object changeFrom) {
+        graph.newEdgeSpec([from:edge.from, to:edge.to, changeFrom:changeFrom]).apply()
+    }
+
+    /**
+     * Changes edge.to to changeTo. This allows edge.two to point to a different {@link graph.Vertex}.
+     * @param changeTo  the new id for edge.to
+     */
+    void changeTo(Object changeTo) {
+        graph.newEdgeSpec([from:edge.from, to:edge.to, changeTo:changeTo]).apply()
+    }
+
+    private void setupGraph() {
         if (edgePresentInGraph && (changeFrom || changeTo)) {
             //need to delete and re-add edge to reset hashcode in LinkedHashSet.
             graph.deleteEdge(edge.from, edge.to)
@@ -129,25 +122,25 @@ class EdgeSpec {
         graph.vertex(to)
     }
 
-    protected void initRenameOne() {
+    private void initRenameOne() {
         if (changeFrom) {
             graph.vertex(changeFrom)
             edge.from = changeFrom
         }
     }
 
-    protected void initRenameTwo() {
+    private void initRenameTwo() {
         if (changeTo) {
             graph.vertex(changeTo)
             edge.to = changeTo
         }
     }
 
-    protected void initEntries() {
+    private void initEntries() {
         edge.putAll(entries)
     }
 
-    protected void applyClosure() {
+    private void applyClosure() {
         if (!runnerCodeClosure) {
             return
         }
@@ -157,8 +150,10 @@ class EdgeSpec {
     }
 
     Edge apply() {
-        init()
-        checkConditions()
+        if(applied) {
+            throw new IllegalStateException('spec has already been applied.')
+        }
+        applied = true
         setupGraph()
         initRenameOne()
         initRenameTwo()

src/main/groovy/graph/VertexSpec.groovy

@@ -7,15 +7,16 @@ package graph
  * Vertex.
  */
 class VertexSpec {
-    Graph graph
-    Vertex vertex
+    private Graph graph
+    private Vertex vertex
     Object id
-    Object changeId
-    final Set<Object> connectsToSet = [] as Set<Object>
-    final Set<Object> connectsFromSet = [] as Set<Object>
-    List dslProperties
-    Map entries
-    Closure runnerCodeClosure
+    private Object changeId
+    private final Set<Object> connectsToSet = [] as Set<Object>
+    private final Set<Object> connectsFromSet = [] as Set<Object>
+    private List dslProperties
+    private Map entries
+    private Closure runnerCodeClosure
+    private boolean applied
 
     /**
      * Gets the graph the {@link Vertex} has been added to. This can be used inside the runnerCode to access the graph.
@@ -39,6 +40,12 @@ class VertexSpec {
     }
 
     protected VertexSpec(Graph graph, Map<String, ?> map, Closure closure = null) {
+        if (!graph) {
+            throw new IllegalArgumentException('invalid graph.')
+        }
+        if(!map) {
+            throw new IllegalArgumentException('invalid map.')
+        }
         this.graph = graph
 
         dslProperties = ['id', 'changeId', 'connectsTo', 'connectsFrom']
@@ -77,6 +84,16 @@ class VertexSpec {
             connectsFromSet.add(map.connectsFrom)
         }
         runnerCodeClosure = closure
+
+        if (!graph.vertices[id]) {
+            id = changeId ?: id
+            changeId = null
+        }
+        vertex = graph.vertices[id] ?: graph.newVertex(id:id)
+
+        if (changeId && graph.vertices[changeId]) {
+            throw new IllegalStateException('renamed vertex already exists')
+        }
     }
 
     /**
@@ -113,36 +130,10 @@ class VertexSpec {
         graph.newVertexSpec([id:id], closure).apply()
     }
 
-    protected void init() {
-        if (vertex) {
-            throw new IllegalStateException('vertex already created.')
-        }
-        if (!graph.vertices[id]) {
-            id = changeId ?: id
-            changeId = null
-        }
-        vertex = graph.vertices[id] ?: graph.newVertex(id:id)
-    }
-
-    protected void initEntries() {
+    protected void addEntries() {
         vertex.putAll(entries)
     }
 
-    protected void checkConditions() {
-        if (!id) {
-            throw new IllegalStateException('id is not set.')
-        }
-        if (!graph) {
-            throw new IllegalStateException('graph is not set.')
-        }
-        if (!vertex) {
-            throw new IllegalStateException('vertex is not set.')
-        }
-        if (changeId && graph.vertices[changeId]) {
-            throw new IllegalStateException('renamed vertex already exists')
-        }
-    }
-
     protected void applyChangeKey() {
         if (changeId) {
             graph.changeId(id, changeId)
@@ -170,12 +161,14 @@ class VertexSpec {
     }
 
     Vertex apply() {
-        init()
-        checkConditions()
+        if(applied) {
+            throw new IllegalStateException('spec has already been applied.')
+        }
+        applied = true
         applyChangeKey()
         applyConnectsTo()
         applyConnectsFrom()
-        initEntries()
+        addEntries()
         graph.addVertex(vertex)
         applyClosure()
         vertex

src/test/groovy/api/GraphBaseSpec.groovy

@@ -150,6 +150,15 @@ abstract class GraphBaseSpec extends Specification {
         graph.edges.find { it.from == 'A' && it.to == 'C' }
     }
 
+    def 'cannot create edge with null in connectsTo collection'() {
+        when:
+        graph.vertex('A', [connectsTo:[null]])
+
+        then:
+        IllegalArgumentException e = thrown()
+        e.message == 'Invalid connectsTo item.'
+    }
+
     def 'create edge using connectsFrom in map'() {
         given:
         graph.vertex('A', [connectsFrom:'B'])
@@ -161,6 +170,15 @@ abstract class GraphBaseSpec extends Specification {
         graph.edges.find { it.from == 'B' && it.to == 'A' }
     }
 
+    def 'cannot create edge with null in connectsFrom collection'() {
+        when:
+        graph.vertex('A', [connectsFrom:[null]])
+
+        then:
+        IllegalArgumentException e = thrown()
+        e.message == 'Invalid connectsFrom item.'
+    }
+
     def 'create two edges using connectsFrom in map'() {
         given:
         graph.vertex('A', [connectsFrom:['B', 'C']])

src/test/groovy/graph/type/EdgeSpecSpecImpl.groovy → src/test/groovy/graph/EdgeSpecSpec.groovy

@@ -1,25 +1,29 @@
-package graph.type
+package graph
 
 import graph.Edge
 import graph.EdgeSpec
 import graph.Graph
 import spock.lang.Specification
 
-class EdgeSpecSpecImpl extends Specification {
+class EdgeSpecSpec extends Specification {
     Graph graph = new Graph()
 
+    def 'cannot create with null graph'() {
+        when:
+        new EdgeSpec(null, null)
 
-    def 'init throws IllegalStateException if edge is set'() {
-        given: 'an EdgeSpecImpl with edge set'
-        EdgeSpec spec = new EdgeSpec(graph, [:])
-        spec.edge = new Edge(from:'one', to:'two')
+        then:
+        IllegalArgumentException e = thrown()
+        e.message == 'graph must be set.'
+    }
 
-        when: 'init is called'
-        spec.init()
+    def 'cannot create with null map'() {
+        when:
+        new EdgeSpec(graph, null)
 
-        then: 'IllegalStateException is thrown'
-        IllegalStateException e = thrown()
-        e.message == 'Edge already created.'
+        then:
+        IllegalArgumentException e = thrown()
+        e.message == 'map must be set.'
     }
 
 
@@ -95,4 +99,29 @@ class EdgeSpecSpecImpl extends Specification {
         graph.edges.first().from == 'step1'
         graph.edges.first().to == 'step4'
     }
+
+    def 'cannot apply spec twice'() {
+        when:
+        EdgeSpec spec = new EdgeSpec(graph, [from:'A', to:'B'])
+        spec.apply()
+
+        and:
+        spec.apply()
+
+        then:
+        IllegalStateException e = thrown()
+        e.message == 'spec has already been applied.'
+    }
+
+    def 'missing method calls method on edge'() {
+        given:
+        EdgeSpec spec = new EdgeSpec(graph, [from:'A', to:'B'])
+        spec.apply()
+
+        when:
+        spec.put('prop', 'value')
+
+        then:
+        graph.edges.first().prop == 'value'
+    }
 }