Merge pull request #986 from JuliaRobotics/23Q1/enh/blobhelpers

fixes to Blob and BlobEntry helpers, entry.originId

Author: dehann

NEWS.md

@@ -2,6 +2,9 @@ Listing news on any major breaking changes in DFG.  For regular changes, see int
 
 # v0.20
 
+- Throw `KeyError` if `getBlobEntry` is not found, previously was `ErrorException`.
+- Change return type on `addData!` convenience wrappers to only return new `BlobEntry`.
+- Fix `addBlob!` calls for `FolderStore` and `InMemoryBlobStore` to use `BlobEntry.originId` and not previous bug `entry.id`.
 - Close long running serialization redo (#590) using only JSON3.jl and StructTypes.jl going forward.
 - Standardize BlobEntry=>Blob naming of functions, and keeping convenience wrappers `{get,add,update,delete}Data[!]`.
 - Consolidate to only one `BlobEntry` definition, dropping use of `AbstractBlobEntry`.

Project.toml

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

attic/DataBlobs/FileDataEntryBlob.jl

@@ -42,7 +42,9 @@ function addBlob!(dfg::AbstractDFG, entry::BlobEntry, data::Vector{UInt8})
         open(entryfilename(entry), "w") do f
             JSON.print(f, entry)
         end
-        return getBlob(dfg, entry)::Vector{UInt8}
+        # FIXME update for entry.blobId vs entry.originId
+        return UUID(entry.id)
+        # return getBlob(dfg, entry)::Vector{UInt8}
     end
 end
 
@@ -72,7 +74,8 @@ end
 function addData!(::Type{BlobEntry}, dfg::AbstractDFG, label::Symbol, key::Symbol, folder::String, blob::Vector{UInt8}, timestamp=now(localzone());
                   id::UUID = uuid4(), hashfunction = sha256)
     fde = BlobEntry(key, id, folder, bytes2hex(hashfunction(blob)), timestamp)
-    de = addBlobEntry!(dfg, label, fde)
-    db = addBlob!(dfg, fde, blob)
-    return de=>db
+    blobId = addBlob!(dfg, fde, blob) |> UUID
+    newEntry = BlobEntry(fde; id=blobId, blobId)
+    de = addBlobEntry!(dfg, label, newEntry)
+    return de # de=>db
 end

src/DataBlobs/services/BlobEntry.jl

@@ -77,13 +77,16 @@ Get data entry
 Also see: [`addBlobEntry`](@ref), [`getBlob`](@ref), [`listBlobEntries`](@ref)
 """
 function getBlobEntry(var::AbstractDFGVariable, key::Symbol)
-    !hasBlobEntry(var, key) && error("No dataEntry label $(key) found in variable $(getLabel(var))")
+    if !hasBlobEntry(var, key)
+        throw(KeyError("No dataEntry label $(key) found in variable $(getLabel(var)). Available keys: $(keys(var.dataDict))"))
+    end
     return var.dataDict[key]
 end
 
 function getBlobEntry(var::AbstractDFGVariable, blobId::UUID)
     for (k,v) in var.dataDict
-        if v.id == blobId
+        # FIXME stop using v.id since that has been repurposed for unique BlobEntry indexing
+        if v.originId == blobId || v.blobId == blobId || v.id == blobId
             return v
         end
     end
@@ -117,16 +120,27 @@ getBlobEntry(dfg::AbstractDFG, label::Symbol, key::Union{Symbol, UUID, <:Abstrac
 Add Data Entry to a DFG variable
 Should be extended if DFG variable is not returned by reference.
 
-Also see: [`getBlobEntry`](@ref), [`addBlob`](@ref), [`mergeBlobEntries!`](@ref)
+Also see: [`getBlobEntry`](@ref), [`addBlob!`](@ref), [`mergeBlobEntries!`](@ref)
 """
-function addBlobEntry!(var::AbstractDFGVariable, bde::BlobEntry)
-    haskey(var.dataDict, bde.label) && error("blobEntry $(bde.label) already exists on variable $(getLabel(var))")
-    var.dataDict[bde.label] = bde
-    return bde
+function addBlobEntry!(
+    var::AbstractDFGVariable, 
+    entry::BlobEntry;
+    blobId::Union{UUID,Nothing} = (isnothing(entry.blobId) ? entry.id : entry.blobId),
+    blobSize::Int = (hasfield(BlobEntry, :size) ? entry.size : -1)
+)
+    # see https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/issues/985
+    haskey(var.dataDict, entry.label) && error("blobEntry $(entry.label) already exists on variable $(getLabel(var))")
+    var.dataDict[entry.label] = entry
+    return entry
 end
 
-function addBlobEntry!(dfg::AbstractDFG, label::Symbol, bde::BlobEntry)
-    return addBlobEntry!(getVariable(dfg, label), bde)
+function addBlobEntry!(
+    dfg::AbstractDFG, 
+    vLbl::Symbol, 
+    entry::BlobEntry;
+    kw...
+)
+    return addBlobEntry!(getVariable(dfg, vLbl), entry; kw...)
 end
 
 

src/DataBlobs/services/BlobStores.jl

@@ -165,30 +165,32 @@ function getBlob(store::FolderStore{T}, entry::BlobEntry) where T
 end
 
 function addBlob!(store::FolderStore{T}, entry::BlobEntry, data::T) where T
-    blobfilename = joinpath(store.folder,"$(entry.id).dat")
-    entryfilename = joinpath(store.folder,"$(entry.id).json")
+    blobfilename = joinpath(store.folder,"$(entry.originId).dat")
+    entryfilename = joinpath(store.folder,"$(entry.originId).json")
     if isfile(blobfilename)
-        error("Key '$(entry.id)' blob already exists.")
+        error("Key '$(entry.originId)' blob already exists.")
     elseif isfile(entryfilename)
-        error("Key '$(entry.id)' entry already exists, but no blob.")
+        error("Key '$(entry.originId)' entry already exists, but no blob.")
     else
         open(blobfilename, "w") do f
             write(f, data)
         end
         open(entryfilename, "w") do f
             JSON3.write(f, entry)
         end
-        return data
+        # return data
+        # FIXME update for entry.blobId vs. entry.originId
+        return UUID(entry.originId)
     end
 end
 
 function updateBlob!(store::FolderStore{T},  entry::BlobEntry, data::T) where T
-    blobfilename = joinpath(store.folder,"$(entry.id).dat")
-    entryfilename = joinpath(store.folder,"$(entry.id).json")
+    blobfilename = joinpath(store.folder,"$(entry.originId).dat")
+    entryfilename = joinpath(store.folder,"$(entry.originId).json")
     if !isfile(blobfilename)
-        @warn "Key '$(entry.id)' doesn't exist."
+        @warn "Key '$(entry.originId)' doesn't exist."
     elseif !isfile(entryfilename)
-        @warn "Key '$(entry.id)' doesn't exist."
+        @warn "Key '$(entry.originId)' doesn't exist."
     else
         open(blobfilename, "w") do f
             write(f, data)
@@ -228,19 +230,21 @@ function getBlob(store::InMemoryBlobStore{T}, entry::BlobEntry) where T
 end
 
 function addBlob!(store::InMemoryBlobStore{T}, entry::BlobEntry, data::T) where T
-    if haskey(store.blobs, entry.id)
-        error("Key '$(entry.id)' blob already exists.")
+    if haskey(store.blobs, entry.originId)
+        error("Key '$(entry.originId)' blob already exists.")
     end
-    return store.blobs[entry.id] = data
+    # FIXME update for entry.originId vs .blobId
+    store.blobs[entry.originId] = data
+    return UUID(entry.originId)
 end
 
 function updateBlob!(store::InMemoryBlobStore{T},  entry::BlobEntry, data::T) where T
-    if haskey(store.blobs, entry.id)
-        @warn "Key '$(entry.id)' doesn't exist."
+    if haskey(store.blobs, entry.originId)
+        @warn "Key '$(entry.originId)' doesn't exist."
     end
-    return store.blobs[entry.id] = data
+    return store.blobs[entry.originId] = data
 end
 
 function deleteBlob!(store::InMemoryBlobStore{T}, entry::BlobEntry) where T
-    return pop!(store.blobs, entry.id)
+    return pop!(store.blobs, entry.originId)
 end