Skip to content

Commit 5d62dc7

Browse files
AffieAffie
authored
Merge pull request #421 from JuliaRobotics/enhancement/2Q20/nodelabelvalidation
Validation during creation of DFG drivers
2 parents 1605f07 + 0c9804b commit 5d62dc7

File tree

10 files changed

+114
-61
lines changed

10 files changed

+114
-61
lines changed

‎src/CloudGraphsDFG/entities/CloudGraphsDFG.jl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ function CloudGraphsDFG{T}(neo4jConnection::Neo4j.Connection,
3434
rebuildFactorMetadata!;
3535
solverParams::T=NoSolverParams(),
3636
createSessionNodes::Bool=true) where T <: AbstractParams
37+
# Validate the userId, robotId, and sessionId
38+
!isValidLabel(userId) && error("'$userId' is not a valid User ID")
39+
!isValidLabel(robotId) && error("'$robotId' is not a valid Robot ID")
40+
!isValidLabel(sessionId) && error("'$sessionId' is not a valid Session ID")
41+
3742
graph = Neo4j.getgraph(neo4jConnection)
3843
neo4jInstance = Neo4jInstance(neo4jConnection, graph)
3944
dfg = CloudGraphsDFG{T}(neo4jInstance, userId, robotId, sessionId, description, encodePackedTypeFunc, getPackedTypeFunc, decodePackedTypeFunc, rebuildFactorMetadata!, Symbol[], solverParams)

‎src/CloudGraphsDFG/services/CGStructure.jl

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,4 @@
1-
global _invalidIds = ["USER", "ROBOT", "SESSION", "VARIABLE", "FACTOR", "ENVIRONMENT", "PPE", "BIGDATA"]
2-
global _validLabelRegex = r"^[a-zA-Z]\w*$"
3-
4-
function _isValid(id::Union{Symbol, String})::Bool
5-
if typeof(id) == Symbol
6-
id = String(id)
7-
end
8-
return all(t -> t != uppercase(id), _invalidIds) && match(_validLabelRegex, id) != nothing
9-
end
10-
11-
function _isValid(abstractNode::N)::Bool where N <: AbstractCGNode
1+
function isValidLabel(abstractNode::N)::Bool where N <: AbstractCGNode
122
id = String(abstractNode.id)
133
return all(t -> t != uppercase(id), _invalidIds) && match(_validLabelRegex, id) != nothing
144
end
@@ -65,7 +55,7 @@ end
6555

6656
function createUser(dfg::CloudGraphsDFG, user::User)::User
6757
Symbol(dfg.userId) != user.id && error("DFG user ID must match user's ID")
68-
!_isValid(user) && error("Node cannot have an ID '$(user.id)'.")
58+
!isValidLabel(user) && error("Node cannot have an ID '$(user.id)'.")
6959

7060
props = _convertNodeToDict(user)
7161
retNode = _createNode(dfg.neo4jInstance, ["USER", String(user.id)], props, nothing)
@@ -75,7 +65,7 @@ end
7565
function createRobot(dfg::CloudGraphsDFG, robot::Robot)::Robot
7666
Symbol(dfg.robotId) != robot.id && error("DFG robot ID must match robot's ID")
7767
Symbol(dfg.userId) != robot.userId && error("DFG user ID must match robot's user ID")
78-
!_isValid(robot) && error("Node cannot have an ID '$(robot.id)'.")
68+
!isValidLabel(robot) && error("Node cannot have an ID '$(robot.id)'.")
7969

8070
# Find the parent
8171
parents = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:USER:$(dfg.userId))")
@@ -94,7 +84,7 @@ end
9484
function createSession(dfg::CloudGraphsDFG, session::Session)::Session
9585
Symbol(dfg.robotId) != session.robotId && error("DFG robot ID must match session's robot ID")
9686
Symbol(dfg.userId) != session.userId && error("DFG user ID must match session's->robot's->user ID")
97-
!_isValid(session) && error("Node cannot have an ID '$(session.id)'.")
87+
!isValidLabel(session) && error("Node cannot have an ID '$(session.id)'.")
9888

9989
# Find the parent
10090
parents = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:ROBOT:$(dfg.robotId):$(dfg.userId))")
@@ -167,9 +157,9 @@ Get a session specified by userId:robotId:sessionId.
167157
Returns nothing if it isn't found.
168158
"""
169159
function getSession(dfg::CloudGraphsDFG, userId::Symbol, robotId::Symbol, sessionId::Symbol)::Union{Session, Nothing}
170-
!_isValid(userId) && error("Can't receive session with user ID '$(userId)'.")
171-
!_isValid(robotId) && error("Can't receive session with robot ID '$(robotId)'.")
172-
!_isValid(sessionId) && error("Can't receive session with session ID '$(sessionId)'.")
160+
!isValidLabel(userId) && error("Can't retrieve session with user ID '$(userId)'.")
161+
!isValidLabel(robotId) && error("Can't retrieve session with robot ID '$(robotId)'.")
162+
!isValidLabel(sessionId) && error("Can't retrieve session with session ID '$(sessionId)'.")
173163
sessionNode = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:SESSION:$(sessionId):$(robotId):$(userId))")
174164
length(sessionNode) == 0 && return nothing
175165
length(sessionNode) > 1 && error("There look to be $(length(sessionNode)) sessions identified for $(sessionId):$(robotId):$(userId)")
@@ -191,8 +181,8 @@ Get a robot specified by userId:robotId.
191181
Returns nothing if it isn't found.
192182
"""
193183
function getRobot(dfg::CloudGraphsDFG, userId::Symbol, robotId::Symbol)::Union{Robot, Nothing}
194-
!_isValid(userId) && error("Can't receive session with user ID '$(userId)'.")
195-
!_isValid(robotId) && error("Can't receive session with robot ID '$(robotId)'.")
184+
!isValidLabel(userId) && error("Can't retrieve robot with user ID '$(userId)'.")
185+
!isValidLabel(robotId) && error("Can't retrieve robot with robot ID '$(robotId)'.")
196186
robotNode = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:ROBOT:$(robotId):$(userId))")
197187
length(robotNode) == 0 && return nothing
198188
length(robotNode) > 1 && error("There look to be $(length(robotNode)) robots identified for $(robotId):$(userId)")
@@ -214,7 +204,7 @@ Get a user specified by userId.
214204
Returns nothing if it isn't found.
215205
"""
216206
function getUser(dfg::CloudGraphsDFG, userId::Symbol)::Union{User, Nothing}
217-
!_isValid(userId) && error("Can't receive session with user ID '$(userId)'.")
207+
!isValidLabel(userId) && error("Can't retrieve user with user ID '$(userId)'.")
218208
userNode = _getNeoNodesFromCyphonQuery(dfg.neo4jInstance, "(node:USER:$(userId))")
219209
length(userNode) == 0 && return nothing
220210
length(userNode) > 1 && error("There look to be $(length(userNode)) robots identified for $(userId)")

‎src/Common.jl

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,22 @@ ls, lsf
6464
"""
6565
sortDFG(vars::Vector{<:DFGNode}; by=getTimestamp, kwargs...) = sort(vars; by=by, kwargs...)
6666
sortDFG(vars::Vector{Symbol}; lt=natural_lt, kwargs...)::Vector{Symbol} = sort(vars; lt=lt, kwargs...)
67+
68+
##==============================================================================
69+
## Validation of session, robot, and user IDs.
70+
##==============================================================================
71+
72+
global _invalidIds = ["USER", "ROBOT", "SESSION", "VARIABLE", "FACTOR", "ENVIRONMENT", "PPE", "BIGDATA"]
73+
global _validLabelRegex = r"^[a-zA-Z]\w*$"
74+
75+
"""
76+
$(SIGNATURES)
77+
78+
Returns true if the label is valid for a session, robot, or user ID.
79+
"""
80+
function isValidLabel(id::Union{Symbol, String})::Bool
81+
if typeof(id) == Symbol
82+
id = String(id)
83+
end
84+
return all(t -> t != uppercase(id), _invalidIds) && match(_validLabelRegex, id) != nothing
85+
end

‎src/DistributedFactorGraphs.jl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,12 +188,15 @@ export getFactorType, getFactorFunction
188188
export mergeVariableData!, mergeGraphVariableData!
189189

190190
##------------------------------------------------------------------------------
191-
## Other utility funtions
191+
## Other utility functions
192192
##------------------------------------------------------------------------------
193193

194194
# Sort
195195
export natural_lt, sortDFG
196196

197+
# Validation
198+
export isValidLabel
199+
197200
## List
198201
export ls, lsf, ls2
199202
export lsTypes, lsfTypes

‎src/GraphsDFG/GraphsDFG.jl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ module GraphsDFGs
22

33
using Graphs
44
using DocStringExtensions
5+
using UUIDs
56

67
using ...DistributedFactorGraphs
78

‎src/GraphsDFG/entities/GraphsDFG.jl

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,31 +25,43 @@ mutable struct GraphsDFG{T <: AbstractParams} <: AbstractDFG
2525
end
2626

2727

28-
GraphsDFG( g::FGType=Graphs.incdict(GraphsNode,is_directed=false),
29-
d::String="Graphs.jl implementation",
30-
n::Int64=0,
31-
l::Dict{Symbol, Int64}=Dict{Symbol, Int64}(),
32-
a::Vector{Symbol}=Symbol[];
33-
userId::String = "UserID",
34-
robotId::String = "robotID",
35-
sessionId::String = "sessionID",
36-
userData::Dict{Symbol, String} = Dict{Symbol, String}(),
37-
robotData::Dict{Symbol, String} = Dict{Symbol, String}(),
38-
sessionData::Dict{Symbol, String} = Dict{Symbol, String}(),
39-
params::T=NoSolverParams()) where T <: AbstractParams = GraphsDFG{T}(g, d, userId, robotId, sessionId, userData, robotData, sessionData, n, l, a, params)
28+
function GraphsDFG( g::FGType=Graphs.incdict(GraphsNode,is_directed=false),
29+
d::String="Graphs.jl implementation",
30+
n::Int64=0,
31+
l::Dict{Symbol, Int64}=Dict{Symbol, Int64}(),
32+
a::Vector{Symbol}=Symbol[];
33+
userId::String = "DefaultUser",
34+
robotId::String = "DefaultRobot",
35+
sessionId::String = "Session_$(string(uuid4())[1:6])",
36+
userData::Dict{Symbol, String} = Dict{Symbol, String}(),
37+
robotData::Dict{Symbol, String} = Dict{Symbol, String}(),
38+
sessionData::Dict{Symbol, String} = Dict{Symbol, String}(),
39+
params::T=NoSolverParams()) where T <: AbstractParams
40+
# Validate the userId, robotId, and sessionId
41+
!isValidLabel(userId) && error("'$userId' is not a valid User ID")
42+
!isValidLabel(robotId) && error("'$robotId' is not a valid Robot ID")
43+
!isValidLabel(sessionId) && error("'$sessionId' is not a valid Session ID")
44+
return GraphsDFG{T}(g, d, userId, robotId, sessionId, userData, robotData, sessionData, n, l, a, params)
45+
end
4046

41-
GraphsDFG{T}( g::FGType=Graphs.incdict(GraphsNode,is_directed=false),
42-
d::String="Graphs.jl implementation",
43-
n::Int64=0,
44-
l::Dict{Symbol, Int64}=Dict{Symbol, Int64}(),
45-
a::Vector{Symbol}=Symbol[];
46-
userId::String = "UserID",
47-
robotId::String = "robotID",
48-
sessionId::String = "sessionID",
49-
userData::Dict{Symbol, String} = Dict{Symbol, String}(),
50-
robotData::Dict{Symbol, String} = Dict{Symbol, String}(),
51-
sessionData::Dict{Symbol, String} = Dict{Symbol, String}(),
52-
params::T=T()) where T <: AbstractParams = GraphsDFG{T}(g, d, userId, robotId, sessionId, userData, robotData, sessionData, n, l, a, params)
47+
function GraphsDFG{T}( g::FGType=Graphs.incdict(GraphsNode,is_directed=false),
48+
d::String="Graphs.jl implementation",
49+
n::Int64=0,
50+
l::Dict{Symbol, Int64}=Dict{Symbol, Int64}(),
51+
a::Vector{Symbol}=Symbol[];
52+
userId::String = "DefaultUser",
53+
robotId::String = "DefaultRobot",
54+
sessionId::String = "Session_$(string(uuid4())[1:6])",
55+
userData::Dict{Symbol, String} = Dict{Symbol, String}(),
56+
robotData::Dict{Symbol, String} = Dict{Symbol, String}(),
57+
sessionData::Dict{Symbol, String} = Dict{Symbol, String}(),
58+
params::T=T()) where T <: AbstractParams
59+
# Validate the userId, robotId, and sessionId
60+
!isValidLabel(userId) && error("'$userId' is not a valid User ID")
61+
!isValidLabel(robotId) && error("'$robotId' is not a valid Robot ID")
62+
!isValidLabel(sessionId) && error("'$sessionId' is not a valid Session ID")
63+
return GraphsDFG{T}(g, d, userId, robotId, sessionId, userData, robotData, sessionData, n, l, a, params)
64+
end
5365

5466
GraphsDFG(description::String,
5567
userId::String,

‎src/LightDFG/LightDFG.jl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ module LightDFGs
22

33
using LightGraphs
44
using DocStringExtensions
5+
using UUIDs
56

67
using ...DistributedFactorGraphs
78

‎src/LightDFG/entities/LightDFG.jl

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,18 @@ Create an in-memory LightDFG with the following parameters:
3030
"""
3131
function LightDFG{T,V,F}(g::FactorGraph{Int,V,F}=FactorGraph{Int,V,F}();
3232
description::String="LightGraphs.jl implementation",
33-
userId::String="UserID",
34-
robotId::String="RobotID",
35-
sessionId::String="SessionID",
33+
userId::String="DefaultUser",
34+
robotId::String="DefaultRobot",
35+
sessionId::String="Session_$(string(uuid4())[1:6])",
3636
userData::Dict{Symbol, String} = Dict{Symbol, String}(),
3737
robotData::Dict{Symbol, String} = Dict{Symbol, String}(),
3838
sessionData::Dict{Symbol, String} = Dict{Symbol, String}(),
3939
params::T=T()) where {T <: AbstractParams, V <:AbstractDFGVariable, F<:AbstractDFGFactor}
40-
41-
LightDFG{T,V,F}(g, description, userId, robotId, sessionId, userData, robotData, sessionData, Symbol[], params)
40+
# Validate the userId, robotId, and sessionId
41+
!isValidLabel(userId) && error("'$userId' is not a valid User ID")
42+
!isValidLabel(robotId) && error("'$robotId' is not a valid Robot ID")
43+
!isValidLabel(sessionId) && error("'$sessionId' is not a valid Session ID")
44+
return LightDFG{T,V,F}(g, description, userId, robotId, sessionId, userData, robotData, sessionData, Symbol[], params)
4245
end
4346

4447
# LightDFG{T}(; kwargs...) where T <: AbstractParams = LightDFG{T,DFGVariable,DFGFactor}(;kwargs...)

‎test/consolInterfaceDev.jl

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,32 @@ include("testBlocks.jl")
1313
testDFGAPI = CloudGraphsDFG
1414

1515
# TODO maybe move to cloud graphs permanantly as standard easy to use functions
16-
function DFG.CloudGraphsDFG(; params=NoSolverParams())
17-
cgfg = CloudGraphsDFG{typeof(params)}("localhost", 7474, "neo4j", "test",
18-
"testUser", "testRobot", "testSession_$(string(uuid4())[1:6])",
19-
"description",
20-
nothing,
21-
nothing,
22-
(dfg,f)->f,#IncrementalInference.decodePackedType,
23-
(dfg,f)->f,#ncrementalInference.rebuildFactorMetadata!,
24-
solverParams=params)
25-
createDfgSessionIfNotExist(cgfg)
16+
function DFG.CloudGraphsDFG(; hostname="localhost",
17+
port=7474,
18+
username="neo4j",
19+
password="test",
20+
params=NoSolverParams(),
21+
description::String="CloudGraphsDFG implementation",
22+
userId::String="DefaultUser",
23+
robotId::String="DefaultRobot",
24+
sessionId::String="Session_$(string(uuid4())[1:6])",
25+
userData::Dict{Symbol, String} = Dict{Symbol, String}(),
26+
robotData::Dict{Symbol, String} = Dict{Symbol, String}(),
27+
sessionData::Dict{Symbol, String} = Dict{Symbol, String}())
28+
29+
cgfg = CloudGraphsDFG{typeof(params)}(hostname,
30+
port,
31+
username,
32+
password,
33+
userId,
34+
robotId,
35+
sessionId,
36+
description,
37+
nothing,
38+
nothing,
39+
(dfg,f)->f,#IncrementalInference.decodePackedType,
40+
(dfg,f)->f,#ncrementalInference.rebuildFactorMetadata!,
41+
solverParams=params)
2642

2743
setUserData!(cgfg, Dict{Symbol, String}())
2844
setRobotData!(cgfg, Dict{Symbol, String}())
@@ -59,7 +75,6 @@ function DFG.CloudGraphsDFG(description::String,
5975
(dfg,f)->f,#IncrementalInference.rebuildFactorMetadata!,
6076
solverParams=solverParams)
6177

62-
createDfgSessionIfNotExist(cdfg)
6378

6479
setUserData!(cdfg, userData)
6580
setRobotData!(cdfg, robotData)

‎test/testBlocks.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,10 @@ function DFGStructureAndAccessors(::Type{T}, solparams::AbstractParams=NoSolverP
114114
fg = T(params=solparams)
115115
#TODO test something better
116116
@test isa(fg, T)
117+
# Test the validation of the robot, session, and user IDs.
118+
@test_throws ErrorException T(params=solparams, sessionId="!notValid")
119+
@test_throws ErrorException T(params=solparams, robotId="!notValid")
120+
@test_throws ErrorException T(params=solparams, userId="!notValid")
117121

118122
#TODO I don't like, so not exporting, and not recommended to use
119123
# Technically if you set Ids its a new object

0 commit comments

Comments
 (0)