deprecate getBlobentryFirst -> getfirstBlobentry, fix blobId bugs, update folderstore, and remove some exports (#1144)

* deprecate getBlobentryFirst -> getfirstBlobentry, fix blobId bugs, update folderstore, and remove some exports
* fix mergeVairableState! bug

Author: Affie

NEWS.md

@@ -19,6 +19,9 @@ Listing news on any major breaking changes in DFG.  For regular changes, see int
 - Deprecate listVariableSolverData -> listVariableStates
 - Deprecate getVariableSolverDataAll -> getVariableStates
 - Deprecate getSolverData -> getVariableState/getFactorState
+- Deprecate getBlobentryFirst -> getfirstBlobentry, see #1114
+- OrderedDict is no longer exported
+- FolderStore path now includes the store label.
 
 # v0.26
 - Graph structure plotting now uses GraphMakie.jl instead of GraphPlot.jl. Update by replacing `using GraphPlot` with `using GraphMakie`.

Project.toml

@@ -21,7 +21,6 @@ Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
 ProgressMeter = "92933f4c-e287-5a05-a399-4b506db050ca"
 Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
 RecursiveArrayTools = "731186ca-8d62-57ce-b412-fbd966d074cd"
-Reexport = "189a3867-3050-52da-a836-e630ba90ab69"
 SHA = "ea8e919c-243c-51af-8825-aaa63cd721ce"
 SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
 StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
@@ -64,7 +63,6 @@ Pkg = "1.4, 1.5"
 ProgressMeter = "1"
 Random = "1.10"
 RecursiveArrayTools = "2, 3"
-Reexport = "1"
 SHA = "0.7, 1"
 SparseArrays = "1.10"
 StaticArrays = "1"

README.md

@@ -2,13 +2,13 @@
 
 Click on badges to follow links:
 
-| Release v0.25 | Dev | Coverage | Documentation |
+| Release v0.26 | Dev | Coverage | Documentation |
 |---------------|-----|----------|---------------|
 | [![CI][dfg-ci-stb-img]][dfg-ci-stb-url] | [![CI][dfg-ci-dev-img]][dfg-ci-dev-url] <br> [![Average time to resolve an issue](https://isitmaintained.com/badge/resolution/JuliaRobotics/DistributedFactorGraphs.jl.svg)](https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/issues) | [![codecov.io][dfg-cov-img]][dfg-cov-url] <br> [![Percentage of issues still open](https://isitmaintained.com/badge/open/JuliaRobotics/DistributedFactorGraphs.jl.svg)](https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/issues) | [![docs](https://img.shields.io/badge/DFGDocs-latest-blue.svg)](http://juliarobotics.github.io/DistributedFactorGraphs.jl/latest/) <br> [![docs](https://img.shields.io/badge/CaesarDocs-latest-blue.svg)](http://juliarobotics.github.io/Caesar.jl/latest/)
 
 [dfg-ci-dev-img]: https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/actions/workflows/ci.yml/badge.svg
 [dfg-ci-dev-url]: https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/actions/workflows/ci.yml
-[dfg-ci-stb-img]: https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/actions/workflows/ci.yml/badge.svg?branch=release%2Fv0.25
+[dfg-ci-stb-img]: https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/actions/workflows/ci.yml/badge.svg?branch=release%2Fv0.26
 [dfg-ci-stb-url]: https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/actions/workflows/ci.yml
 [dfg-cov-img]: https://codecov.io/github/JuliaRobotics/DistributedFactorGraphs.jl/coverage.svg?branch=develop
 [dfg-cov-url]: https://codecov.io/github/JuliaRobotics/DistributedFactorGraphs.jl?branch=develop

src/DataBlobs/entities/BlobEntry.jl

@@ -17,7 +17,7 @@ Base.@kwdef struct Blobentry
     id::Union{UUID, Nothing} = nothing
     """ Machine friendly and globally unique identifier of the 'Blob', usually assigned from a common point in the system.  This can be used to guarantee unique retrieval of the large data blob. """
     blobId::Union{UUID, Nothing} = uuid4() #Deprecated in v0.25 TODO  remove union, blobId is mandatory
-    """ Machine friendly and locally assigned identifier of the 'Blob'.  `.originId`s are mandatory upon first creation at the origin regardless of network access.  Separate from `.blobId` since some architectures do not allow edge processes to assign a uuid4 to data store elements. """
+    """ TBD if Deprecated. Machine friendly and locally assigned identifier of the 'Blob'."""
     originId::Union{UUID, Nothing} = blobId #Deprecated in v0.25 TODO remove union or remove originId completely
     """ Human friendly label of the `Blob` and also used as unique identifier per node on which a `Blobentry` is added.  E.g. do "LEFTCAM_1", "LEFTCAM_2", ... of you need to repeat a label on the same variable. """
     label::Symbol

src/DataBlobs/services/BlobEntry.jl

@@ -44,7 +44,6 @@ function Base.show(io::IO, ::MIME"text/plain", entry::Blobentry)
     println(io, "Blobentry {")
     println(io, "  id:            ", entry.id)
     println(io, "  blobId:        ", entry.blobId)
-    println(io, "  originId:      ", entry.originId)
     println(io, "  label:         ", entry.label)
     println(io, "  blobstore:     ", entry.blobstore)
     println(io, "  hash:          ", entry.hash)
@@ -84,23 +83,26 @@ function getBlobentry(var::VariableDFG, key::Symbol)
     return var.blobEntries[findfirst(x -> x.label == key, var.blobEntries)]
 end
 
-#TODO maybe rename to getBlobentryFirst
-function getBlobentry(var::AbstractDFGVariable, blobId::UUID)
+"""
+    $(SIGNATURES)
+Finds and returns the first blob entry that matches the filter.
+
+Also see: [`getBlobentry`](@ref)
+"""
+function getfirstBlobentry(var::AbstractDFGVariable, blobId::UUID)
     for (k, v) in var.dataDict
-        if blobId in [v.originId, v.blobId]
+        if blobId == v.blobId
             return v
         end
     end
     throw(KeyError("No blobEntry with blobId $(blobId) found in variable $(getLabel(var))"))
 end
 
-"""
-    $(SIGNATURES)
-Finds and returns the first blob entry that matches the regex.
+function getfirstBlobentry(dfg::AbstractDFG, label::Symbol, blobId::UUID)
+    return getfirstBlobentry(getVariable(dfg, label), blobId)
+end
 
-Also see: [`getBlobentry`](@ref)
-"""
-function getBlobentryFirst(var::AbstractDFGVariable, key::Regex)
+function getfirstBlobentry(var::AbstractDFGVariable, key::Regex)
     for (k, v) in var.dataDict
         if occursin(key, string(v.label))
             return v
@@ -113,15 +115,15 @@ function getBlobentryFirst(var::AbstractDFGVariable, key::Regex)
     )
 end
 
-function getBlobentryFirst(var::VariableDFG, key::Regex)
+function getfirstBlobentry(var::VariableDFG, key::Regex)
     firstIdx = findfirst(x -> contains(string(x.label), key), var.blobEntries)
     if isnothing(firstIdx)
         throw(KeyError("$key"))
     end
     return var.blobEntries[firstIdx]
 end
 
-function getBlobentryFirst(dfg::AbstractDFG, label::Symbol, key::Regex)
+function getfirstBlobentry(dfg::AbstractDFG, label::Symbol, key::Regex)
     els = listBlobentries(dfg, label)
     firstIdx = findfirst(contains(key), string.(els))
     isnothing(firstIdx) && throw(
@@ -136,13 +138,13 @@ end
 # verbNoun(dfg::VariableCompute, label::Symbol, args...; kwargs...) = verbNoun(getVariable(dfg, label), args...; kwargs...)
 # with something like:
 # getvariablemethod = [
-#     :getBlobentryFirst,
+#     :getfirstBlobentry,
 # ]
 # for met in methodstooverload  
 #     @eval DistributedFactorGraphs $met(dfg::AbstractDFG, label::Symbol, args...; kwargs...) = $met(getVariable(dfg, label), args...; kwargs...)
 # end
 
-function getBlobentry(dfg::AbstractDFG, label::Symbol, key::Union{Symbol, UUID})
+function getBlobentry(dfg::AbstractDFG, label::Symbol, key::Symbol)
     return getBlobentry(getVariable(dfg, label), key)
 end
 # getBlobentry(dfg::AbstractDFG, label::Symbol, key::Symbol) = getBlobentry(getVariable(dfg, label), key)

src/DataBlobs/services/BlobStores.jl

@@ -81,7 +81,8 @@ end
 ##==============================================================================
 ## AbstractBlobstore derived CRUD for Blob 
 ##==============================================================================
-
+#TODO looking in all the blobstores does not make sense since since there is a chance that the blobId is not unique across blobstores.
+# using the cached blobstore is the right way to go here.
 function getBlob(dfg::AbstractDFG, entry::Blobentry)
     stores = getBlobstores(dfg)
     storekeys = collect(keys(stores))
@@ -111,8 +112,7 @@ function getBlob(dfg::AbstractDFG, entry::Blobentry)
 end
 
 function getBlob(store::AbstractBlobstore, entry::Blobentry)
-    blobId = isnothing(entry.blobId) ? entry.originId : entry.blobId
-    return getBlob(store, blobId)
+    return getBlob(store, entry.blobId)
 end
 
 #add 
@@ -121,11 +121,10 @@ function addBlob!(dfg::AbstractDFG, entry::Blobentry, data)
 end
 
 function addBlob!(store::AbstractBlobstore{T}, entry::Blobentry, data::T) where {T}
-    blobId = isnothing(entry.blobId) ? entry.originId : entry.blobId
-    return addBlob!(store, blobId, data)
+    return addBlob!(store, entry.blobId, data)
 end
 
-# also creates an originId as uuid4
+# also creates an blobId as uuid4
 addBlob!(store::AbstractBlobstore, data) = addBlob!(store, uuid4(), data)
 
 #update
@@ -142,13 +141,15 @@ function deleteBlob!(dfg::AbstractDFG, entry::Blobentry)
 end
 
 function deleteBlob!(store::AbstractBlobstore, entry::Blobentry)
-    blobId = isnothing(entry.blobId) ? entry.originId : entry.blobId
-    return deleteBlob!(store, blobId)
+    return deleteBlob!(store, entry.blobId)
 end
 
 #has
+function hasBlob(store::AbstractBlobstore, entry::Blobentry)
+    return hasBlob(store, entry.blobId)
+end
 function hasBlob(dfg::AbstractDFG, entry::Blobentry)
-    return hasBlob(getBlobstore(dfg, entry.blobstore), entry.originId)
+    return hasBlob(getBlobstore(dfg, entry.blobstore), entry.blobId)
 end
 
 #TODO
@@ -182,19 +183,22 @@ end
 #TODO added in v0.25 to avoid a breaking change in deserialization old DFGs, remove.
 StructTypes.StructType(::Type{<:FolderStore}) = StructTypes.OrderedStruct()
 
-function FolderStore(foldername::String; label = :default_folder_store, createfolder = true)
-    if createfolder && !isdir(foldername)
-        @info "Folder '$foldername' doesn't exist - creating."
+function FolderStore(foldername::String; label::Symbol = :default, createfolder = true)
+    storepath = joinpath(foldername, string(label))
+    if createfolder && !isdir(storepath)
+        @info "Folder '$storepath' doesn't exist - creating."
         # create new folder
-        mkpath(foldername)
+        mkpath(storepath)
     end
     return FolderStore{Vector{UInt8}}(label, foldername)
 end
 
-blobfilename(store::FolderStore, blobId::UUID) = joinpath(store.folder, string(blobId))
+function blobfilename(store::FolderStore, blobId::UUID)
+    return joinpath(store.folder, string(store.label), string(blobId))
+end
 
 function getBlob(store::FolderStore{T}, blobId::UUID) where {T}
-    blobfilename = joinpath(store.folder, string(blobId))
+    blobfilename = joinpath(store.folder, string(store.label), string(blobId))
     if isfile(blobfilename)
         open(blobfilename) do f
             return read(f)
@@ -205,7 +209,7 @@ function getBlob(store::FolderStore{T}, blobId::UUID) where {T}
 end
 
 function addBlob!(store::FolderStore{T}, blobId::UUID, data::T) where {T}
-    blobfilename = joinpath(store.folder, string(blobId))
+    blobfilename = joinpath(store.folder, string(store.label), string(blobId))
     if isfile(blobfilename)
         throw(KeyError("Key '$blobId' blob already exists."))
     else
@@ -218,7 +222,7 @@ function addBlob!(store::FolderStore{T}, blobId::UUID, data::T) where {T}
 end
 
 function updateBlob!(store::FolderStore{T}, blobId::UUID, data::T) where {T}
-    blobfilename = joinpath(store.folder, string(blobId))
+    blobfilename = joinpath(store.folder, string(store.label), string(blobId))
     if !isfile(blobfilename)
         @warn "Key '$blobId' doesn't exist."
     else
@@ -230,18 +234,18 @@ function updateBlob!(store::FolderStore{T}, blobId::UUID, data::T) where {T}
 end
 
 function deleteBlob!(store::FolderStore{T}, blobId::UUID) where {T}
-    blobfilename = joinpath(store.folder, string(blobId))
+    blobfilename = joinpath(store.folder, string(store.label), string(blobId))
     rm(blobfilename)
     return 1
 end
 
 #hasBlob or existsBlob?
 function hasBlob(store::FolderStore, blobId::UUID)
-    blobfilename = joinpath(store.folder, string(blobId))
+    blobfilename = joinpath(store.folder, string(store.label), string(blobId))
     return isfile(blobfilename)
 end
 
-hasBlob(store::FolderStore, entry::Blobentry) = hasBlob(store, entry.originId)
+hasBlob(store::FolderStore, entry::Blobentry) = hasBlob(store, entry.blobId)
 
 listBlobs(store::FolderStore) = readdir(store.folder)
 ##==============================================================================
@@ -319,7 +323,7 @@ function getBlob(store::LinkStore, blobId::UUID)
 end
 
 function addBlob!(store::LinkStore, entry::Blobentry, linkfile::String)
-    return addBlob!(store, entry.originId, nothing, linkfile::String)
+    return addBlob!(store, entry.blobId, nothing, linkfile::String)
 end
 
 function addBlob!(store::LinkStore, blobId::UUID, blob::Any, linkfile::String)

src/DataBlobs/services/HelpersDataWrapEntryBlob.jl

@@ -87,7 +87,7 @@ function getData(
     getlast::Bool = true,
 )
     _getblobentr(g, v, k) = getBlobentries(g, v, k)
-    _getblobentr(g, v, k::UUID) = [getBlobentry(g, v, k);]
+    _getblobentr(g, v, k::UUID) = [getfirstBlobentry(g, v, k);]
     de_ = _getblobentr(dfg, vlabel, key)
     lbls = (s -> s.label).(de_)
     idx = sortperm(lbls; rev = getlast)
@@ -181,8 +181,8 @@ function addData!(
     metadata = "",
     mimeType::String = "application/octet-stream",
     id::Union{UUID, Nothing} = nothing,
-    blobId::Union{UUID, Nothing} = nothing, #only assign if blobstore issued you an id
-    originId::UUID = uuid4(),
+    blobId::UUID = uuid4(),
+    originId::UUID = blobId,
     hashfunction = sha256,
 )
     #

src/Deprecated.jl

@@ -62,7 +62,11 @@ const VariableNodeData = VariableState
 @deprecate listAgentBlobEntries(args...; kwargs...) listAgentBlobentries(args...; kwargs...)
 @deprecate hasBlobEntry(args...; kwargs...) hasBlobentry(args...; kwargs...)
 @deprecate getBlobEntry(args...; kwargs...) getBlobentry(args...; kwargs...)
-@deprecate getBlobEntryFirst(args...; kwargs...) getBlobentryFirst(args...; kwargs...)
+@deprecate getBlobEntryFirst(args...; kwargs...) getfirstBlobentry(args...; kwargs...)
+@deprecate getBlobentry(var::AbstractDFGVariable, blobId::UUID) getfirstBlobentry(
+    var::AbstractDFGVariable,
+    blobId::UUID,
+)
 @deprecate addBlobEntry!(args...; kwargs...) addBlobentry!(args...; kwargs...)
 @deprecate addBlobEntries!(args...; kwargs...) addBlobentries!(args...; kwargs...)
 @deprecate mergeBlobEntry!(args...; kwargs...) mergeBlobentry!(args...; kwargs...)