Merge pull request #877 from JuliaRobotics/master

release v0.18.3-rc1

Author: GearsAD

Project.toml

@@ -1,6 +1,6 @@
 name = "DistributedFactorGraphs"
 uuid = "b5cc3c7e-6572-11e9-2517-99fb8daf2f04"
-version = "0.18.2"
+version = "0.18.3"
 
 [deps]
 Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"

src/Common.jl

@@ -67,9 +67,12 @@ sortDFG(vars::Vector{Symbol}; lt=natural_lt, kwargs...) = sort(vars; lt=lt, kwar
 ##==============================================================================
 ## Validation of session, robot, and user IDs.
 ##==============================================================================
-global _invalidIds = ["USER", "ROBOT", "SESSION", "VARIABLE", "FACTOR", "ENVIRONMENT", "PPE", "DATA_ENTRY", "FACTORGRAPH"]
+global _invalidIds = [
+    "USER", "ROBOT", "SESSION", 
+    "VARIABLE", "FACTOR", "ENVIRONMENT", 
+    "PPE", "DATA_ENTRY", "FACTORGRAPH"]
 
-global _validLabelRegex = r"^[a-zA-Z]\w*$"
+global _validLabelRegex = r"^[a-zA-Z][\w\.\@]*$"
 
 """
 $(SIGNATURES)
@@ -80,7 +83,7 @@ function isValidLabel(id::Union{Symbol, String})::Bool
     if typeof(id) == Symbol
         id = String(id)
     end
-    return all(t -> t != uppercase(id), _invalidIds) && match(_validLabelRegex, id) != nothing
+    return all(t -> t != uppercase(id), _invalidIds) && match(_validLabelRegex, id) !== nothing
 end
 
 

src/Neo4jDFG/services/CGStructure.jl

@@ -77,12 +77,12 @@ function createRobot(dfg::Neo4jDFG, robot::Robot)
     !isValidLabel(robot) && error("Node cannot have an ID '$(robot.id)'.")
 
     # Find the parent
-    parents = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:USER:$(dfg.userId))")
+    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 &&
+    length(_getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:ROBOT:`$(dfg.userId)`:`$(robot.id)`)")) != 0 &&
         error("Robot '$(robot.id)' already exists for user '$(robot.userId)'")
 
     props = _convertNodeToDict(robot)
@@ -98,12 +98,12 @@ function createSession(dfg::Neo4jDFG, session::Session)
     !isValidLabel(session) && error("Node cannot have an ID '$(session.id)'.")
 
     # Find the parent
-    parents = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:ROBOT:$(dfg.robotId):$(dfg.userId))")
+    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 &&
+    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)
@@ -145,7 +145,7 @@ Notes
 - Returns `Vector{Session}`
 """
 function lsSessions(dfg::Neo4jDFG)
-    sessionNodes = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:SESSION:$(dfg.robotId):$(dfg.userId))")
+    sessionNodes = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:SESSION:`$(dfg.robotId)`:`$(dfg.userId)`)")
     return map(s -> _convertDictToSession(Neo4j.getnodeproperties(s)), sessionNodes)
 end
 
@@ -158,7 +158,7 @@ Notes
 - Returns `::Vector{Robot}`
 """
 function lsRobots(dfg::Neo4jDFG)
-    robotNodes = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:ROBOT:$(dfg.userId))")
+    robotNodes = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:ROBOT:`$(dfg.userId)`)")
     return map(s -> _convertDictToRobot(Neo4j.getnodeproperties(s)), robotNodes)
 end
 
@@ -187,7 +187,7 @@ function getSession(dfg::Neo4jDFG, userId::Symbol, robotId::Symbol, sessionId::S
     !isValidLabel(userId) && error("Can't retrieve session with user ID '$(userId)'.")
     !isValidLabel(robotId) && error("Can't retrieve session with robot ID '$(robotId)'.")
     !isValidLabel(sessionId) && error("Can't retrieve session with session ID '$(sessionId)'.")
-    sessionNode = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:SESSION:$(sessionId):$(robotId):$(userId))")
+    sessionNode = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:SESSION:`$(sessionId)`:`$(robotId)`:`$(userId)`)")
     length(sessionNode) == 0 && return nothing
     length(sessionNode) > 1 && error("There look to be $(length(sessionNode)) sessions identified for $(sessionId):$(robotId):$(userId)")
     return _convertDictToSession(Neo4j.getnodeproperties(sessionNode[1]))
@@ -216,7 +216,7 @@ Notes
 function getRobot(dfg::Neo4jDFG, userId::Symbol, robotId::Symbol)
     !isValidLabel(userId) && error("Can't retrieve robot with user ID '$(userId)'.")
     !isValidLabel(robotId) && error("Can't retrieve robot with robot ID '$(robotId)'.")
-    robotNode = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:ROBOT:$(robotId):$(userId))")
+    robotNode = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:ROBOT:`$(robotId)`:`$(userId)`)")
     length(robotNode) == 0 && return nothing
     length(robotNode) > 1 && error("There look to be $(length(robotNode)) robots identified for $(robotId):$(userId)")
     return _convertDictToRobot(Neo4j.getnodeproperties(robotNode[1]))
@@ -244,7 +244,7 @@ Notes
 """
 function getUser(dfg::Neo4jDFG, userId::Symbol)
     !isValidLabel(userId) && error("Can't retrieve user with user ID '$(userId)'.")
-    userNode = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:USER:$(userId))")
+    userNode = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:USER:`$(userId)`)")
     length(userNode) == 0 && return nothing
     length(userNode) > 1 && error("There look to be $(length(userNode)) robots identified for $(userId)")
     return _convertDictToUser(Neo4j.getnodeproperties(userNode[1]))
@@ -272,7 +272,7 @@ Notes
 """
 function clearSession!!(dfg::Neo4jDFG)
     # Perform detach+deletion
-    _queryNeo4j(dfg.neo4jInstance, "match (node:$(dfg.userId):$(dfg.robotId):$(dfg.sessionId)) detach delete node ")
+    _queryNeo4j(dfg.neo4jInstance, "match (node:`$(dfg.userId)`:`$(dfg.robotId)`:`$(dfg.sessionId)`) detach delete node ")
 
     # Clearing history
     dfg.addHistory = Symbol[]
@@ -288,7 +288,7 @@ Notes
 """
 function clearRobot!!(dfg::Neo4jDFG)
     # Perform detach+deletion
-    _queryNeo4j(dfg.neo4jInstance, "match (node:$(dfg.userId):$(dfg.robotId)) detach delete node ")
+    _queryNeo4j(dfg.neo4jInstance, "match (node:`$(dfg.userId)`:`$(dfg.robotId)`) detach delete node ")
 
     # Clearing history
     dfg.addHistory = Symbol[]
@@ -304,7 +304,7 @@ Notes
 """
 function clearUser!!(dfg::Neo4jDFG)
     # Perform detach+deletion
-    _queryNeo4j(dfg.neo4jInstance, "match (node:$(dfg.userId)) detach delete node ")
+    _queryNeo4j(dfg.neo4jInstance, "match (node:`$(dfg.userId)`) detach delete node ")
 
     # Clearing history
     dfg.addHistory = Symbol[]

src/Neo4jDFG/services/CommonFunctions.jl

@@ -1,13 +1,11 @@
-alphaOnlyMatchRegex = r"^[a-zA-Z0-9_]*$"
-
 """
 $(SIGNATURES)
 Returns the transaction for a given query.
 NOTE: Must commit(transaction) after you're done.
 """
 function _queryNeo4j(neo4jInstance::Neo4jInstance, query::String; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)
-    @debug "[Query] $(currentTransaction != nothing ? "[TRANSACTION]" : "") $query"
-    if currentTransaction == nothing
+    @debug "[Query] $(currentTransaction !== nothing ? "[TRANSACTION]" : "") $query"
+    if currentTransaction === nothing
         loadtx = transaction(neo4jInstance.connection)
         loadtx(query)
         # Have to finish the transaction
@@ -27,7 +25,7 @@ Note: Using symbols so that the labels obey Neo4j requirements
 function _createNode(neo4jInstance::Neo4jInstance, labels::Vector{String}, properties::Dict{String, Any}, parentNode::Union{Nothing, Neo4j.Node}, relationshipLabel::Symbol=:NOTHING)::Neo4j.Node
     createdNode = Neo4j.createnode(neo4jInstance.graph, properties)
     addnodelabels(createdNode, labels)
-    parentNode == nothing && return createdNode
+    parentNode === nothing && return createdNode
     # Otherwise create the relationship
     createrel(parentNode, createdNode, String(relationshipLabel))
     return createdNode
@@ -56,6 +54,8 @@ $(SIGNATURES)
 Get a node property - returns nothing if not found
 """
 function _getNodeProperty(neo4jInstance::Neo4jInstance, nodeLabels::Vector{String}, property::String; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)
+    # Note that properties should already by backticked, but double checking becase this is used in many places
+    nodeLabels = [!startswith(a, "`") && !endswith(a, "`") ? "`$(a)`" : a for a in nodeLabels]
     query = "match (n:$(join(nodeLabels, ":"))) return n.$property"
     result = _queryNeo4j(neo4jInstance, query, currentTransaction=currentTransaction)
     length(result.results[1]["data"]) != 1 && error("No data returned from the query.")
@@ -71,6 +71,9 @@ function _setNodeProperty(neo4jInstance::Neo4jInstance, nodeLabels::Vector{Strin
     if value isa String
         value = "\""*replace(value, "\"" => "\\\"")*"\"" # Escape strings
     end
+    # Defensively wrap the node labels.
+    nodeLabels = [!startswith(n, "`") && !endswith(n, "`") ? "`$(n)`" : n for n in nodeLabels]    
+
     query = """
     match (n:$(join(nodeLabels, ":")))
     set n.$property = $value
@@ -87,14 +90,14 @@ $(SIGNATURES)
 Get a node's tags
 """
 function _getNodeTags(neo4jInstance::Neo4jInstance, nodeLabels::Vector{String})::Union{Nothing, Vector{String}}
-    query = "match (n:$(join(nodeLabels, ":"))) return labels(n)"
+    query = "match (n:$(join("`".*nodeLabels*"`", ":"))) return labels(n)"
     result = _queryNeo4j(neo4jInstance, query)
     length(result.results[1]["data"]) != 1 && return nothing
     return result.results[1]["data"][1]["row"][1]
 end
 
 function _getNodeCount(neo4jInstance::Neo4jInstance, nodeLabels::Vector{String}; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)::Int
-    query = "match (n:$(join(nodeLabels, ":"))) return count(n)"
+    query = "match (n:$(join("`".*nodeLabels.*"`", ":"))) return count(n)"
     result = _queryNeo4j(neo4jInstance, query, currentTransaction=currentTransaction)
     length(result.results[1]["data"]) != 1 && return 0
     length(result.results[1]["data"][1]["row"]) != 1 && return 0
@@ -218,21 +221,21 @@ function _getLabelsForType(dfg::Neo4jDFG,
     isempty(dfg.sessionId) && error("The DFG object's sessionId is empty, please specify a session ID.")
 
     labels = []
-    type == User && (labels = [dfg.userId, "USER"])
-    type == Robot && (labels = [dfg.userId, dfg.robotId, "ROBOT"])
-    type == Session && (labels = [dfg.userId, dfg.robotId, dfg.sessionId, "SESSION"])
+    type == User && (labels = ["`$(dfg.userId)`", "USER"])
+    type == Robot && (labels = ["`$(dfg.userId)`", "`$(dfg.robotId)`", "ROBOT"])
+    type == Session && (labels = ["`$(dfg.userId)`", "`$(dfg.robotId)`", "`$(dfg.sessionId)`", "SESSION"])
     type <: DFGVariable &&
-        (labels = [dfg.userId, dfg.robotId, dfg.sessionId, "VARIABLE"])
+        (labels = ["`$(dfg.userId)`", "`$(dfg.robotId)`", "`$(dfg.sessionId)`", "VARIABLE"])
     type <: DFGFactor &&
-        (labels = [dfg.userId, dfg.robotId, dfg.sessionId, "FACTOR"])
+        (labels = ["`$(dfg.userId)`", "`$(dfg.robotId)`", "`$(dfg.sessionId)`", "FACTOR"])
     type <: AbstractPointParametricEst &&
-        (labels = [dfg.userId, dfg.robotId, dfg.sessionId, "PPE"])
+        (labels = ["`$(dfg.userId)`", "`$(dfg.robotId)`", "`$(dfg.sessionId)`", "PPE"])
     type <: VariableNodeData &&
-        (labels = [dfg.userId, dfg.robotId, dfg.sessionId, "SOLVERDATA"])
+        (labels = ["`$(dfg.userId)`", "`$(dfg.robotId)`", "`$(dfg.sessionId)`", "SOLVERDATA"])
     type <: AbstractDataEntry &&
-        (labels = [dfg.userId, dfg.robotId, dfg.sessionId, "DATA"])
+        (labels = ["`$(dfg.userId)`", "`$(dfg.robotId)`", "`$(dfg.sessionId)`", "DATA"])
     # Some are children of nodes, so add that in if it's set.
-    parentKey != nothing && push!(labels, String(parentKey))
+    parentKey !== nothing && push!(labels, String(parentKey))
     return labels
 end
 
@@ -260,7 +263,7 @@ function _listVarSubnodesForType(dfg::Neo4jDFG, variablekey::Symbol, dfgType::Ty
     query = "match (subnode:$(join(_getLabelsForType(dfg, dfgType, parentKey=variablekey),':'))) return subnode.$keyToReturn"
     @debug "[Query] _listVarSubnodesForType query:\r\n$query"
     result = nothing
-    if currentTransaction != nothing
+    if currentTransaction !== nothing
         result = currentTransaction(query; submit=true)
     else
         tx = transaction(dfg.neo4jInstance.connection)
@@ -276,7 +279,7 @@ function _getVarSubnodeProperties(dfg::Neo4jDFG, variablekey::Symbol, dfgType::T
     query = "match (subnode:$(join(_getLabelsForType(dfg, dfgType, parentKey=variablekey),':')):$nodeKey) return properties(subnode)"
     @debug "[Query] _getVarSubnodeProperties query:\r\n$query"
     result = nothing
-    if currentTransaction != nothing
+    if currentTransaction !== nothing
         result = currentTransaction(query; submit=true)
     else
         tx = transaction(dfg.neo4jInstance.connection)
@@ -298,7 +301,9 @@ function _matchmergeVariableSubnode!(
         addProps::Dict{String, String}=Dict{String, String}(),
         currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing) where
         {N <: DFGNode, APPE <: AbstractPointParametricEst, ABDE <: AbstractDataEntry, PVND <: PackedVariableNodeData}
-
+    
+    # Defensively wrap the node labels.
+    nodeLabels = [!startswith(n, "`") && !endswith(n, "`") ? "`$(n)`" : n for n in nodeLabels]    
     query = """
                 MATCH (var:$variablekey:$(join(_getLabelsForType(dfg, DFGVariable, parentKey=variablekey),':')))
                 MERGE (var)-[:$relationshipKey]->(subnode:$(join(nodeLabels,':')))
@@ -307,7 +312,7 @@ function _matchmergeVariableSubnode!(
                 RETURN properties(subnode)"""
     @debug "[Query] _matchmergeVariableSubnode! query:\r\n$query"
     result = nothing
-    if currentTransaction != nothing
+    if currentTransaction !== nothing
         result = currentTransaction(query; submit=true) # TODO: Maybe we should submit (; submit = true) for the results to fail early?
     else
         tx = transaction(dfg.neo4jInstance.connection)
@@ -327,6 +332,9 @@ function _deleteVarSubnode!(
         nodeLabels::Vector{String},
         nodekey::Symbol=:default;
         currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)
+    # Defensively wrap the node labels.
+    nodeLabels = [!startswith(n, "`") && !endswith(n, "`") ? "`$(n)`" : n for n in nodeLabels]    
+
     query = """
     MATCH (node:$nodekey:$(join(nodeLabels,':')))
     WITH node, properties(node) as props
@@ -335,7 +343,7 @@ function _deleteVarSubnode!(
     """
     @debug "[Query] _deleteVarSubnode delete query:\r\n$query"
     result = nothing
-    if currentTransaction != nothing
+    if currentTransaction !== nothing
         result = currentTransaction(query; submit=true) # TODO: Maybe we should submit (; submit = true) for the results to fail early?
     else
         tx = transaction(dfg.neo4jInstance.connection)

src/Neo4jDFG/services/Neo4jDFG.jl

@@ -445,7 +445,7 @@ function isConnected(dfg::Neo4jDFG)::Bool
 end
 
 function getNeighbors(dfg::Neo4jDFG, node::T; solvable::Int=0)::Vector{Symbol}  where T <: DFGNode
-    query = "(n:$(dfg.userId):$(dfg.robotId):$(dfg.sessionId):$(node.label))-[r:FACTORGRAPH]-(node) where (node:VARIABLE or node:FACTOR) and node.solvable >= $solvable"
+    query = "(n:`$(dfg.userId)`:`$(dfg.robotId)`:`$(dfg.sessionId)`:$(node.label))-[r:FACTORGRAPH]-(node) where (node:VARIABLE or node:FACTOR) and node.solvable >= $solvable"
     @debug "[Query] $query"
     neighbors = _getLabelsFromCyphonQuery(dfg.neo4jInstance, query)
     # If factor, need to do variable ordering TODO, Do we? does it matter if we always use _variableOrderSymbols in calculations?
@@ -456,7 +456,7 @@ function getNeighbors(dfg::Neo4jDFG, node::T; solvable::Int=0)::Vector{Symbol}
 end
 
 function getNeighbors(dfg::Neo4jDFG, label::Symbol; solvable::Int=0)::Vector{Symbol}
-    query = "(n:$(dfg.userId):$(dfg.robotId):$(dfg.sessionId):$(label))-[r:FACTORGRAPH]-(node) where (node:VARIABLE or node:FACTOR) and node.solvable >= $solvable"
+    query = "(n:`$(dfg.userId)`:`$(dfg.robotId)`:`$(dfg.sessionId)`:$(label))-[r:FACTORGRAPH]-(node) where (node:VARIABLE or node:FACTOR) and node.solvable >= $solvable"
     neighbors = _getLabelsFromCyphonQuery(dfg.neo4jInstance, query)
     # If factor, need to do variable ordering TODO, Do we? does it matter if we always use _variableOrderSymbols in calculations?
     if isFactor(dfg, label)
@@ -473,10 +473,10 @@ function getSubgraphAroundNode(dfg::Neo4jDFG, node::DFGNode, distance::Int64=1,
     # Thank you Neo4j for 0..* awesomeness!!
     neighborList = _getLabelsFromCyphonQuery(dfg.neo4jInstance,
         """
-        (n:$(dfg.userId):$(dfg.robotId):$(dfg.sessionId):$(node.label))-[FACTORGRAPH*0..$distance]-(node:$(dfg.userId):$(dfg.robotId):$(dfg.sessionId))
+        (n:`$(dfg.userId)`:`$(dfg.robotId)`:`$(dfg.sessionId)`:$(node.label))-[FACTORGRAPH*0..$distance]-(node:`$(dfg.userId)`:`$(dfg.robotId)`:`$(dfg.sessionId)`)
         WHERE (n:VARIABLE OR n:FACTOR OR node:VARIABLE OR node:FACTOR)
         and not (node:SESSION)
-        and (node.solvable >= $solvable or node:$(dfg.userId):$(dfg.robotId):$(dfg.sessionId):$(node.label))""" # Always return the root node
+        and (node.solvable >= $solvable or node:`$(dfg.userId)`:`$(dfg.robotId)`:`$(dfg.sessionId)`:$(node.label))""" # Always return the root node
         )
 
     # Copy the section of graph we want

test/CGStructureTests.jl

@@ -1,5 +1,5 @@
 dfg = Neo4jDFG{SolverParams}("localhost", 7474, "neo4j", "test",
-                            "testUser", "testRobot", "testSession",
+                            "[email protected]", "testRobot", "testSession",
                             "Description of test session",
                             solverParams=SolverParams())
 

test/iifInterfaceTests.jl

@@ -46,9 +46,9 @@ end
     T = typeof(dfg)
     if dfg isa Neo4jDFG
         #TODO
-        dfg2 = Neo4jDFG(solverParams=SolverParams(), userId="testUserId")
+        dfg2 = Neo4jDFG(solverParams=SolverParams(), userId="[email protected]")
     else
-        dfg2 = T(solverParams=SolverParams(), userId="testUserId")
+        dfg2 = T(solverParams=SolverParams(), userId="[email protected]")
     end
 
     # Build a new in-memory IIF graph to transfer into the new graph.

test/interfaceTests.jl

@@ -152,7 +152,7 @@ end
 ##
 @testset "Adjacency Matrices" begin
 
-    fg = testDFGAPI(userId="testUserId")
+    fg = testDFGAPI(userId="[email protected]")
     addVariable!(fg, var1)
     setSolvable!(fg, :a, 1)
     addVariable!(fg, var2)
@@ -196,7 +196,7 @@ end
 
 @testset "Copy Functions" begin
     rand(6)
-    fg = testDFGAPI(userId="testUserId")
+    fg = testDFGAPI(userId="[email protected]")
     addVariable!(fg, var1)
     addVariable!(fg, var2)
     addVariable!(fg, var3)
@@ -207,7 +207,7 @@ end
     # @test getVariableOrder(fg,:f1) == getVariableOrder(fgcopy,:f1)
 
     #test copyGraph, deepcopyGraph[!]
-    fgcopy = testDFGAPI(userId="testUserId")
+    fgcopy = testDFGAPI(userId="[email protected]")
     DFG.deepcopyGraph!(fgcopy, fg)
     @test getVariableOrder(fg,:abf1) == getVariableOrder(fgcopy,:abf1)