Merge pull request #839 from JuliaRobotics/815/cgdfg_createsession

Optimizing the creation of a CGDFG.

Author: GearsAD

src/Neo4jDFG/entities/Neo4jDFG.jl

@@ -33,18 +33,10 @@ mutable struct Neo4jDFG{T <: AbstractParams} <: AbstractDFG{T}
         !isValidLabel(robotId) && error("'$robotId' is not a valid Robot ID")
         !isValidLabel(sessionId) && error("'$sessionId' is not a valid Session ID")
 
-        # neo4jConnection = Neo4j.Connection(host, port=port, user=dbUser, password=dbPassword);
-        # graph = Neo4j.getgraph(neo4jConnection)
-        # neo4jInstance = Neo4jInstance(neo4jConnection, graph)
-
         dfg = new{T}(neo4jInstance, userId, robotId, sessionId, description, addHistory, solverParams, blobStores)
         # Create the session if it doesn't already exist
         if createSessionNodes
             createDfgSessionIfNotExist(dfg)
-            setUserData!(dfg, userData)
-            setRobotData!(dfg, robotData)
-            setSessionData!(dfg, sessionData)
-            setDescription!(dfg, description)
         end
 
         return dfg

src/Neo4jDFG/services/CGStructure.jl

@@ -6,7 +6,6 @@ end
 
 # Fastest way I can think to convert the data into a dict
 #TODO: Probably should be made more efficient...definitely should be made more efficient
-#  return ::Dict{String, Any}
 function _convertNodeToDict(abstractNode::N) where N <: AbstractCGNode
     cp = deepcopy(abstractNode)
     data = length(cp.data) != 0 ? JSON2.write(cp.data) : "{}"
@@ -16,8 +15,6 @@ function _convertNodeToDict(abstractNode::N) where N <: AbstractCGNode
     return ser
 end
 
-#TODO: Refactor, #HACK :D (but it works!)
-# return ::Session
 function _convertDictToSession(dict::Dict{String, Any})
     data = JSON2.read(String(base64decode(dict["data"])), Dict{Symbol, String})
     session = Session(
@@ -31,8 +28,7 @@ function _convertDictToSession(dict::Dict{String, Any})
         dict["lastUpdatedTimestamp"])
     return session
 end
-#TODO: Refactor, #HACK :D (but it works!)
-# returns ::Robot
+
 function _convertDictToRobot(dict::Dict{String, Any})
     data = JSON2.read(String(base64decode(dict["data"])), Dict{Symbol, String})
     robot = Robot(
@@ -45,8 +41,7 @@ function _convertDictToRobot(dict::Dict{String, Any})
         dict["lastUpdatedTimestamp"])
     return robot
 end
-#TODO: Refactor, #HACK :D (but it works!)
-# returns ::User
+
 function _convertDictToUser(dict::Dict{String, Any})
     data = JSON2.read(String(base64decode(dict["data"])), Dict{Symbol, String})
     user = User(
@@ -59,81 +54,33 @@ function _convertDictToUser(dict::Dict{String, Any})
     return user
 end
 
-# returns ::User
-function createUser(dfg::Neo4jDFG, user::User)
-    Symbol(dfg.userId) != user.id && error("DFG user ID must match user's ID")
-    !isValidLabel(user) && error("Node cannot have an ID '$(user.id)'.")
-
-    props = _convertNodeToDict(user)
-    # TODO: Switch to _queryNeo4j
-    retNode = _createNode(dfg.neo4jInstance, ["USER", String(user.id)], props, nothing)
-    return user
-end
-
-# returns ::Robot
-function createRobot(dfg::Neo4jDFG, robot::Robot)
-    Symbol(dfg.robotId) != robot.id && error("DFG robot ID must match robot's ID")
-    Symbol(dfg.userId) != robot.userId && error("DFG user ID must match robot's user ID")
-    !isValidLabel(robot) && error("Node cannot have an ID '$(robot.id)'.")
-
-    # Find the parent
-    parents = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:USER:`$(dfg.userId)`)")
-    length(parents) == 0 && error("Cannot find user '$(dfg.userId)'")
-    length(parents) > 1 && error("Found multiple users '$(dfg.userId)'")
-
-    # Already exists?
-    length(_getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:ROBOT:`$(dfg.userId)`:`$(robot.id)`)")) != 0 &&
-        error("Robot '$(robot.id)' already exists for user '$(robot.userId)'")
-
-    props = _convertNodeToDict(robot)
-    # TODO: Switch to _queryNeo4j
-    retNode = _createNode(dfg.neo4jInstance, ["ROBOT", String(robot.userId), String(robot.id)], props, parents[1], :ROBOT)
-    return robot
-end
-
-# returns ::Session
-function createSession(dfg::Neo4jDFG, session::Session)
-    Symbol(dfg.robotId) != session.robotId && error("DFG robot ID must match session's robot ID")
-    Symbol(dfg.userId) != session.userId && error("DFG user ID must match session's->robot's->user ID")
-    !isValidLabel(session) && error("Node cannot have an ID '$(session.id)'.")
-
-    # Find the parent
-    parents = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:ROBOT:`$(dfg.robotId)`:`$(dfg.userId)`)")
-    length(parents) == 0 && error("Cannot find robot '$(dfg.robotId)' for user '$(dfg.userId)'")
-    length(parents) > 1 && error("Found multiple robots '$(dfg.robotId)' for user '$(dfg.userId)'")
-
-    # Already exists?
-    length(_getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:SESSION:`$(session.userId)`:`$(session.robotId)`:`$(session.id)`)")) != 0 &&
-        error("Session '$(session.id)' already exists for robot '$(session.robotId)' and user '$(session.userId)'")
-
-    props = _convertNodeToDict(session)
-    # TODO: Switch to _queryNeo4j
-    retNode = _createNode(dfg.neo4jInstance, ["SESSION", String(session.userId), String(session.robotId), String(session.id)], props, parents[1], :SESSION)
-    return session
-end
-
 """
 $(SIGNATURES)
-Shortcut method to create the user, robot, and session if it doesn't already exist.
-
-Notes
-- return `::Session`
+Efficient shortcut method to create the user, robot, and session if it doesn't already exist.
 """
 function createDfgSessionIfNotExist(dfg::Neo4jDFG)
     strip(dfg.userId) == "" && error("User ID is not populated in DFG.")
     strip(dfg.robotId) == "" && error("Robot ID is not populated in DFG.")
     strip(dfg.sessionId) == "" && error("Session ID is not populated in DFG.")
+    !isValidLabel(dfg.userId) && error("Node cannot have an ID '$(dfg.userId)'.")
+    !isValidLabel(dfg.robotId) && error("Node cannot have an ID '$(dfg.robotId)'.")
+    !isValidLabel(dfg.sessionId) && error("Node cannot have an ID '$(dfg.sessionId)'.")
+
     user = User(Symbol(dfg.userId), dfg.userId, "Description for $(dfg.userId)", Dict{Symbol, String}())
     robot = Robot(Symbol(dfg.robotId), Symbol(dfg.userId), dfg.robotId, "Description for $(dfg.userId):$(dfg.robotId)", Dict{Symbol, String}())
     session = Session(Symbol(dfg.sessionId), Symbol(dfg.robotId), Symbol(dfg.userId), dfg.sessionId, dfg.description, Dict{Symbol, String}())
 
-    _getNodeCount(dfg.neo4jInstance, [dfg.userId, "USER"]) == 0 && createUser(dfg, user)
-    _getNodeCount(dfg.neo4jInstance, [dfg.userId, dfg.robotId, "ROBOT"]) == 0 && createRobot(dfg, robot)
-    if _getNodeCount(dfg.neo4jInstance, [dfg.userId, dfg.robotId, dfg.sessionId, "SESSION"]) == 0
-        return createSession(dfg, session)
-    else
-        return getSession(dfg)
-    end
+    # NOTE that this doesn't get updated then, you need to use the set* (e.g. setSessionData) functions yourself.
+    query = """
+    MERGE (u:USER:`$(String(user.id))`) ON CREATE SET $(join(["u.$k = '$(v)'" for (k,v) in _convertNodeToDict(user)], ", "))\r\n
+    MERGE (r:ROBOT:`$(String(user.id))`:`$(String(robot.id))`) ON CREATE SET $(join(["r.$k = '$(v)'" for (k,v) in _convertNodeToDict(robot)], ", "))\r\n
+    MERGE (s:SESSION:`$(String(user.id))`:`$(String(robot.id))`:`$(String(session.id))`) ON CREATE SET $(join(["s.$k = '$(v)'" for (k,v) in _convertNodeToDict(session)], ", "))
+    MERGE (u)-[:ROBOT]->(r) 
+    MERGE (r)-[:SESSION]->(s) 
+    """
+
+    _queryNeo4j(dfg.neo4jInstance, query)
+    return nothing
 end
 
 """
@@ -320,7 +267,7 @@ Notes
 - Returns `::Neo4jDFG `
 """
 function copySession!(sourceDFG::Neo4jDFG, destDFG::Union{Nothing, <:Neo4jDFG})
-    if destDFG == nothing
+    if destDFG === nothing
         destDFG = _getDuplicatedEmptyDFG(sourceDFG)
     end
     _copyIntoGraph!(sourceDFG, destDFG, union(listVariables(sourceDFG), listFactors(sourceDFG)), true)

src/services/CustomPrinting.jl

@@ -191,9 +191,9 @@ function Base.show(io::IO, dfg::AbstractDFG)
     println(io, "  Description: ", dfg.description)
     println(io, "  Nr variables: ", length(ls(dfg)))
     println(io, "  Nr factors: ",length(lsf(dfg)))
-    println(io, "  User Data: ", keys(dfg.userData))
-    println(io, "  Robot Data: ", keys(dfg.robotData))
-    println(io, "  Session Data: ", keys(dfg.sessionData))
+    println(io, "  User Data: ", keys(getUserData(dfg)))
+    println(io, "  Robot Data: ", keys(getRobotData(dfg)))
+    println(io, "  Session Data: ", keys(getSessionData(dfg)))
 end
 
 Base.show(io::IO, ::MIME"text/plain", dfg::AbstractDFG) = show(io, dfg)

test/CGStructureTests.jl

@@ -15,37 +15,21 @@ createDfgSessionIfNotExist(dfg)
 @test Symbol(dfg.userId) in map(u -> u.id, lsUsers(dfg))
 # Test that we can call it again.
 createDfgSessionIfNotExist(dfg)
-# And nuke it so we can try the longer functions.
-clearUser!!(dfg)
 
 # User, robot, and session
 # TODO: Make easier ways to initialize these.
 user = User(Symbol(dfg.userId), "Bob Zack", "Description", Dict{Symbol, String}())
 robot = Robot(Symbol(dfg.robotId), user.id, "Test robot", "Description", Dict{Symbol, String}())
 session = Session(Symbol(dfg.sessionId), robot.id, user.id, "Test Session", "Description", Dict{Symbol, String}())
 
-@test createUser(dfg, user) == user
-@test createRobot(dfg, robot) == robot
-@test createSession(dfg, session) == session
-@test map(s -> s.id, lsSessions(dfg)) == [Symbol(dfg.sessionId)]
-@test map(s -> s.id, lsRobots(dfg)) == [Symbol(dfg.robotId)]
-@test Symbol(dfg.userId) in map(u -> u.id, lsUsers(dfg))
-
-# Test errors
-dfgError = deepcopy(dfg)
-# User/robot/session ID's can't start with numbers and can't have spaces.
-dfgError.userId = "1testNope"
-user = User(Symbol(dfgError.userId), "Bob Zack", "Description", Dict{Symbol, String}())
-@test_throws Exception createUser(dfgError, user)
-
 @test getUserData(dfg) == Dict{Symbol, String}()
 @test getRobotData(dfg) == Dict{Symbol, String}()
 @test getSessionData(dfg) == Dict{Symbol, String}()
 
 # User/robot/session data
 user.data = Dict{Symbol, String}(:a => "Hello", :b => "Goodbye")
-robot.data = Dict{Symbol, String}(:c => "Hello", :d => "Goodbye")
-session.data = Dict{Symbol, String}(:e => "Hello", :f => "Goodbye")
+robot.data = Dict{Symbol, String}(:c => "ABC", :d => "Goodbye")
+session.data = Dict{Symbol, String}(:e => "ADG", :f => "Goodbye")
 
 setUserData!(dfg, user.data)
 setRobotData!(dfg, robot.data)
@@ -54,6 +38,9 @@ setSessionData!(dfg, session.data)
 @test getRobotData(dfg) == robot.data
 @test getSessionData(dfg) == session.data
 
+# And nuke it so we can try the addVariable etc. functions to make sure they create the nodes.
+clearUser!!(dfg)
+createDfgSessionIfNotExist(dfg)
 # Add some nodes.
 v1 = addVariable!(dfg, :a, ContinuousScalar, tags = [:POSE])
 v2 = addVariable!(dfg, :b, ContinuousScalar, tags = [:POSE])
@@ -69,3 +56,9 @@ dfgLocal = buildSubgraph(LightDFG, dfg, union(ls(dfg), lsf(dfg)))
 # Confirm that with sentinels we still have the same graph (doesn't pull in the sentinels)
 @test symdiff(ls(dfgLocal), ls(dfg)) == []
 @test symdiff(lsf(dfgLocal), lsf(dfg)) == []
+
+# Test errors
+dfgError = deepcopy(dfg)
+# User/robot/session ID's can't start with numbers and can't have spaces.
+dfgError.userId = "I want to fail"
+@test_throws Exception createDfgSessionIfNotExist(dfgError)

test/iifInterfaceTests.jl

@@ -10,13 +10,9 @@ end
 @testset "Building a simple Graph" begin
     global dfg,v1,v2,f1
     # Use IIF to add the variables and factors
-    v1 = addVariable!(dfg, :a, ContinuousScalar, tags = [:POSE], solvable=0)
-    v2 = addVariable!(dfg, :b, ContinuousScalar, tags = [:LANDMARK], solvable=1)
+    v1 = addVariable!(dfg, :a, Position{1}, tags = [:POSE], solvable=0)
+    v2 = addVariable!(dfg, :b, Position{1}, tags = [:LANDMARK], solvable=1)
     f1 = addFactor!(dfg, [:a; :b], LinearRelative(Normal(50.0,2.0)), solvable=0)
-
-    @show dfg
-    @show f1
-    @show v1
 end
 
 println()
@@ -25,15 +21,15 @@ println()
 #test before anything changes
 @testset "Producing Dot Files" begin
     global dfg
-    @show todotstr = toDot(dfg)
+    todotstr = toDot(dfg)
     #TODO consider using a regex, but for now test all orders
     todota = cmp(todotstr, "graph graphname {\n2 [\"label\"=\"a\",\"shape\"=\"ellipse\",\"fillcolor\"=\"red\",\"color\"=\"red\"]\n2 -- 3\n3 [\"label\"=\"abf1\",\"shape\"=\"box\",\"fillcolor\"=\"blue\",\"color\"=\"blue\"]\n1 [\"label\"=\"b\",\"shape\"=\"ellipse\",\"fillcolor\"=\"red\",\"color\"=\"red\"]\n1 -- 3\n}\n") |> abs
     todotb = cmp(todotstr, "graph graphname {\n2 [\"label\"=\"b\",\"shape\"=\"ellipse\",\"fillcolor\"=\"red\",\"color\"=\"red\"]\n2 -- 3\n3 [\"label\"=\"abf1\",\"shape\"=\"box\",\"fillcolor\"=\"blue\",\"color\"=\"blue\"]\n1 [\"label\"=\"a\",\"shape\"=\"ellipse\",\"fillcolor\"=\"red\",\"color\"=\"red\"]\n1 -- 3\n}\n") |> abs
     todotc = cmp(todotstr, "graph G {\na [color=red, shape=ellipse];\nb [color=red, shape=ellipse];\nabf1 [color=blue, shape=box];\na -- abf1\nb -- abf1\n}\n") |> abs
     todotd = cmp(todotstr, "graph G {\na [color=red, shape=ellipse];\nb [color=red, shape=ellipse];\nabf1 [color=blue, shape=box];\nb -- abf1\na -- abf1\n}\n") |> abs
     todote = cmp(todotstr, "graph G {\na [color=red, shape=ellipse];\nb [color=red, shape=ellipse];\nabf1 [color=blue, shape=box, fontsize=8, fixedsize=false, height=0.1, width=0.1];\na -- abf1\nb -- abf1\n}\n") |> abs
     todotf = cmp(todotstr, "graph G {\na [color=red, shape=ellipse];\nb [color=red, shape=ellipse];\nabf1 [color=blue, shape=box, fontsize=8, fixedsize=false, height=0.1, width=0.1];\nb -- abf1\na -- abf1\n}\n") |> abs
-    @show todota, todotb, todotc, todotd, todote, todotf
+    # @show todota, todotb, todotc, todotd, todote, todotf
     @test (todota < 1 || todotb < 1 || todotc < 1 || todotd < 1 || todote < 1 || todotf < 1)
     @test toDotFile(dfg, "something.dot") === nothing
     Base.rm("something.dot")
@@ -53,9 +49,9 @@ end
 
     # Build a new in-memory IIF graph to transfer into the new graph.
     iiffg = initfg()
-    v1 = deepcopy(addVariable!(iiffg, :a, ContinuousScalar))
-    v2 = deepcopy(addVariable!(iiffg, :b, ContinuousScalar))
-    v3 = deepcopy(addVariable!(iiffg, :c, ContinuousScalar))
+    v1 = deepcopy(addVariable!(iiffg, :a, Position{1}))
+    v2 = deepcopy(addVariable!(iiffg, :b, Position{1}))
+    v3 = deepcopy(addVariable!(iiffg, :c, Position{1}))
     f1 = deepcopy(addFactor!(iiffg, [:a; :b], LinearRelative(Normal(50.0,2.0)) ))
     f2 = deepcopy(addFactor!(iiffg, [:b; :c], LinearRelative(Normal(10.0,1.0)) ))
 
@@ -116,7 +112,7 @@ end
 
     # Accessors
     @test getAddHistory(dfg) == [:a, :b] #, :abf1
-    @test getDescription(dfg) != nothing
+    @test getDescription(dfg) !== nothing
     #TODO Deprecate
     # @test_throws ErrorException getLabelDict(dfg)
     # Existence
@@ -151,15 +147,14 @@ end
     @test lsf(dfg, LinearRelative) == [:abf1]
     @test lsfWho(dfg, :LinearRelative) == [:abf1]
 
-    @test getVariableType(v1) isa ContinuousScalar
-    @test getVariableType(dfg,:a) isa ContinuousScalar
+    @test getVariableType(v1) isa Position{1}
+    @test getVariableType(dfg,:a) isa Position{1}
 
     #TODO what is lsTypes supposed to return?
     @test_broken lsTypes(dfg)
 
-    @test issetequal(ls(dfg, ContinuousScalar), [:a, :b])
-
-    @test issetequal(lsWho(dfg, :ContinuousScalar),[:a, :b])
+    @test issetequal(ls(dfg, Position{1}), [:a, :b])
+    @test issetequal(lsWho(dfg, :Position),[:a, :b])
 
     varNearTs = findVariableNearTimestamp(dfg, now())
     @test_skip varNearTs[1][1]  == [:b]
@@ -198,15 +193,15 @@ end
     @test getVariablePPEDict(v1) == v1.ppeDict # changed to .ppeDict -- delete by DFG v0.7
 
 
-    @test typeof(getVariableType(v1)) == ContinuousScalar
-    @test typeof(getVariableType(v2)) == ContinuousScalar
-    @test typeof(getVariableType(v1)) == ContinuousScalar
+    @test typeof(getVariableType(v1)) == Position{1}
+    @test typeof(getVariableType(v2)) == Position{1}
+    @test typeof(getVariableType(v1)) == Position{1}
 
     @test getLabel(f1) == f1.label
     @test getTags(f1) == f1.tags
     @test getSolverData(f1) == f1.solverData
 
-    @test getSolverParams(dfg) != nothing
+    @test getSolverParams(dfg) !== nothing
     @test setSolverParams!(dfg, getSolverParams(dfg)) == getSolverParams(dfg)
 
     #solver data is initialized
@@ -329,10 +324,10 @@ end
         @test isConnected(dfg) == true
         # @test @test_deprecated isFullyConnected(dfg) == true
         # @test @test_deprecated hasOrphans(dfg) == false
-        addVariable!(dfg, :orphan, ContinuousScalar, tags = [:POSE], solvable=0)
+        addVariable!(dfg, :orphan, Position{1}, tags = [:POSE], solvable=0)
         @test isConnected(dfg) == false
     else
-        addVariable!(dfg, :orphan, ContinuousScalar, tags = [:POSE], solvable=0)
+        addVariable!(dfg, :orphan, Position{1}, tags = [:POSE], solvable=0)
         @warn "Neo4jDFG is currently failing with the connectivity test."
     end
 end
@@ -393,7 +388,7 @@ numNodes = 10
 # end
 
 #change solvable and solveInProgress for x7,x8 for improved tests on x7x8f1
-verts = map(n -> addVariable!(dfg, Symbol("x$n"), ContinuousScalar, tags = [:POSE]), 1:numNodes)
+verts = map(n -> addVariable!(dfg, Symbol("x$n"), Position{1}, tags = [:POSE]), 1:numNodes)
 #TODO fix this to use accessors
 setSolvable!(verts[7], 1)
 setSolvable!(verts[8], 0)

test/testBlocks.jl

@@ -172,6 +172,10 @@ function DFGStructureAndAccessors(::Type{T}, solparams::AbstractParams=NoSolverP
     @test getSessionId(fg) == sId
     @test getAddHistory(fg) === fg.addHistory
 
+    # Need to set this for Neo4jDFG
+    @test setUserData!(fg, Dict(ud)) == Dict(ud)
+    @test setRobotData!(fg, Dict(rd)) == Dict(rd)
+    @test setSessionData!(fg, Dict(sd)) == Dict(sd)
     @test getUserData(fg) == Dict(ud)
     @test getRobotData(fg) == Dict(rd)
     @test getSessionData(fg) == Dict(sd)