More DFGv1 checked and Blobstore updates

Author: Affie

src/DataBlobs/entities/BlobStores.jl

@@ -1 +1,31 @@
+"""
+    AbstractBlobstore{T}
+
+Abstract supertype for all blobstore implementations.
+
+# Usage
+
+Subtypes of `AbstractBlobstore{T}` must implement the required interface for blob storage and retrieval, such as:
+
+- `add!(store, blobId, blob)`: Add a new blob to the store.
+- `get(store, blobId)`: Retrieve a blob by its ID.
+- `list(store)`: List all blob IDs in the store.
+
+The parameter `T` represents the type of blobs stored (e.g., `Vector{UInt8}` or a custom `Blob` type).
+
+See concrete implementations for details.
+
+Design Notes
+- `blobId` is not considered unique across blobstores with different labels only within a single blobstore.
+- We cannot guarantee that `blobId` is unique across different blobstores with the same label and this is up to the end user.
+- Within a single blobstore `addBlob!` will fail if there is a UUID collision.
+- TODO: We should consider using uuid7 for `blobId`s (requires jl v1.12).
+- `Blobstrores`are identified by a `label::Symbol`, which allows for multiple blobstores to coexist in the same system.
+
+TODO: If we want to make the `blobId`=>Blob pair immutable:
+- We can use the tombstone pattern to mark a blob as deleted. See FolderStore in PR#TODO.
+
+Design goal: all `Blobstore`s with the same `label` can contain the same `blobId`=>`Blob` pair and the blobs should be identical since they are immutable.
+
+"""
 abstract type AbstractBlobstore{T} end

src/DataBlobs/services/BlobEntry.jl

@@ -168,7 +168,7 @@ function addBlobentry!(var::VariableDFG, entry::Blobentry)
     return entry
 end
 
-function addBlobentry!(dfg::AbstractDFG, vLbl::Symbol, entry::Blobentry;)
+function addBlobentry!(dfg::AbstractDFG, vLbl::Symbol, entry::Blobentry)
     return addBlobentry!(getVariable(dfg, vLbl), entry)
 end
 
@@ -238,7 +238,7 @@ end
 """
     $SIGNATURES
 
-Does a blob entry (element) exist with `blobLabel`.
+Does a blob entry exist with `blobLabel`.
 """
 hasBlobentry(var::AbstractDFGVariable, blobLabel::Symbol) = haskey(var.dataDict, blobLabel)
 

src/DataBlobs/services/BlobStores.jl

@@ -13,7 +13,7 @@ $(METHODLIST)
 function getBlob end
 
 """
-Adds a blob to the blob store or dfg with the given entry.
+Adds a blob to the blob store or dfg with the blobId.
 
 Related
 [`addBlobentry!`](@ref)
@@ -81,34 +81,11 @@ 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.
+#TODO maybe we should generalize and move the cached blobstore to DFG.
 function getBlob(dfg::AbstractDFG, entry::Blobentry)
-    stores = getBlobstores(dfg)
-    storekeys = collect(keys(stores))
-    # first check the saved blobstore and then fall back to the rest
-    fidx = findfirst(==(entry.blobstore), storekeys)
-    if !isnothing(fidx)
-        skey = storekeys[fidx]
-        popat!(storekeys, fidx)
-        pushfirst!(storekeys, skey)
-    end
-    for k in storekeys
-        store = stores[k]
-        try
-            blob = getBlob(store, entry)
-            return blob
-        catch err
-            if !(err isa KeyError)
-                throw(err)
-            end
-        end
-    end
-    throw(
-        KeyError(
-            "could not find $(entry.label), uuid $(entry.blobId) in any of the listed blobstores:\n $([s->getLabel(s) for (s,v) in stores]))",
-        ),
-    )
+    storeLabel = entry.blobstore
+    store = getBlobstore(dfg, storeLabel)
+    return getBlob(store, entry.blobId)
 end
 
 function getBlob(store::AbstractBlobstore, entry::Blobentry)
@@ -204,19 +181,18 @@ function getBlob(store::FolderStore{T}, blobId::UUID) where {T}
             return read(f)
         end
     else
-        throw(KeyError("Could not find file '$(blobfilename)'."))
+        throw(IdNotFoundError("Blob", blobId))
     end
 end
 
 function addBlob!(store::FolderStore{T}, blobId::UUID, data::T) where {T}
     blobfilename = joinpath(store.folder, string(store.label), string(blobId))
     if isfile(blobfilename)
-        throw(KeyError("Key '$blobId' blob already exists."))
+        throw(IdExistsError("Blob", blobId))
     else
         open(blobfilename, "w") do f
             return write(f, data)
         end
-        # return data
         return blobId
     end
 end
@@ -235,11 +211,13 @@ end
 
 function deleteBlob!(store::FolderStore{T}, blobId::UUID) where {T}
     blobfilename = joinpath(store.folder, string(store.label), string(blobId))
+    if !isfile(blobfilename)
+        throw(IdNotFoundError("Blob", blobId))
+    end
     rm(blobfilename)
     return 1
 end
 
-#hasBlob or existsBlob?
 function hasBlob(store::FolderStore, blobId::UUID)
     blobfilename = joinpath(store.folder, string(store.label), string(blobId))
     return isfile(blobfilename)
@@ -265,12 +243,15 @@ function InMemoryBlobstore(storeKey::Symbol = :default_inmemory_store)
 end
 
 function getBlob(store::InMemoryBlobstore, blobId::UUID)
+    if !haskey(store.blobs, blobId)
+        throw(IdNotFoundError("Blob", blobId))
+    end
     return store.blobs[blobId]
 end
 
 function addBlob!(store::InMemoryBlobstore{T}, blobId::UUID, data::T) where {T}
     if haskey(store.blobs, blobId)
-        error("Key '$blobId' blob already exists.")
+        throw(IdExistsError("Blob", blobId))
     end
     store.blobs[blobId] = data
     return blobId
@@ -284,6 +265,9 @@ function updateBlob!(store::InMemoryBlobstore{T}, blobId::UUID, data::T) where {
 end
 
 function deleteBlob!(store::InMemoryBlobstore, blobId::UUID)
+    if !haskey(store.blobs, blobId)
+        throw(IdNotFoundError("Blob", blobId))
+    end
     pop!(store.blobs, blobId)
     return 1
 end
@@ -319,25 +303,28 @@ end
 
 function getBlob(store::LinkStore, blobId::UUID)
     fname = get(store.cache, blobId, nothing)
+    if isnothing(fname)
+        throw(IdNotFoundError("Blob", blobId))
+    end
     return read(fname)
 end
 
 function addBlob!(store::LinkStore, entry::Blobentry, linkfile::String)
-    return addBlob!(store, entry.blobId, nothing, linkfile::String)
+    return addBlob!(store, entry.blobId, linkfile)
 end
 
-function addBlob!(store::LinkStore, blobId::UUID, blob::Any, linkfile::String)
+function addBlob!(store::LinkStore, blobId::UUID, linkfile::String)
     if haskey(store.cache, blobId)
-        error("blobId $blobId already exists in the store")
+        throw(IdExistsError("Blob", blobId))
     end
     push!(store.cache, blobId => linkfile)
     open(store.csvfile, "a") do f
         return println(f, blobId, ",", linkfile)
     end
-    return getBlob(store, blobId)
+    return blobId
 end
 
-function deleteBlob!(store::LinkStore, args...)
+function deleteBlob!(store::LinkStore)
     return error("deleteDataBlob(::LinkStore) not supported")
 end
 
@@ -404,12 +391,15 @@ Tables.rows(store::RowBlobstore) = values(store.blobs)
 
 ##
 function getBlob(store::RowBlobstore, blobId::UUID)
+    if !haskey(store.blobs, blobId)
+        throw(IdNotFoundError("Blob", blobId))
+    end
     return getfield(store.blobs[blobId], :blob)
 end
 
 function addBlob!(store::RowBlobstore{T}, blobId::UUID, blob::T) where {T}
     if haskey(store.blobs, blobId)
-        error("Key '$blobId' blob already exists.")
+        throw(IdExistsError("Blob", blobId))
     end
     store.blobs[blobId] = RowBlob(blobId, blob)
     return blobId
@@ -423,7 +413,10 @@ function updateBlob!(store::RowBlobstore{T}, blobId::UUID, blob::T) where {T}
 end
 
 function deleteBlob!(store::RowBlobstore, blobId::UUID)
-    getfield(pop!(store.blobs, blobId), :blob)
+    if !haskey(store.blobs, blobId)
+        throw(IdNotFoundError("Blob", blobId))
+    end
+    pop!(store.blobs, blobId)
     return 1
 end
 
@@ -469,3 +462,4 @@ if false
     Tables.rowtable(sstore)
 end
 ##
+##

src/DistributedFactorGraphs.jl

@@ -60,11 +60,28 @@ using InteractiveUtils: subtypes
 # v1 name, signiture, return, and error checked
 export getFactor, getBlobentry, getGraphBlobentry
 
+export addVariable!, addFactor!, addBlobentry!
+
+export deleteVariable!
+
 # v1 name, signiture, and return
 
 # v1 name only
 export getVariable, getBlob, addBlob!
 
+export hasVariable, hasFactor
+
+# getBlob TODO do we want all of them easy portable vs convenience?
+# getBlob(::AbstractBlobstore, ::UUID)
+# getBlob(::AbstractBlobstore, ::Blobentry)
+# getBlob(::AbstractDFG, ::Blobentry)
+
+# TODO get,add,delete|Blob still needs immutability discussion. but errors checked, tests needs updating though.
+
+# TODO not yet implemented in DFG
+# addAgentBlobentry!
+# addGraphBlobentry!
+
 ##
 const DFG = DistributedFactorGraphs
 export DFG
@@ -121,9 +138,7 @@ export InMemoryDFGTypes, LocalDFG
 
 # AbstractDFG Interface
 export exists,
-    addVariable!,
     addVariables!,
-    addFactor!,
     addFactors!,
     mergeVariable!,
     mergeFactor!,

src/GraphsDFG/GraphsDFG.jl

@@ -24,7 +24,8 @@ import ...DistributedFactorGraphs:
     getAddHistory,
     addFactor!,
     getSolverParams,
-    exists,
+    hasVariable,
+    hasFactor,
     isVariable,
     isFactor,
     mergeVariable!,

src/GraphsDFG/services/GraphsDFG.jl

@@ -6,24 +6,14 @@ function getDFGMetadata(fg::GraphsDFG)
     return metaprops
 end
 
-function exists(
-    dfg::GraphsDFG{P, V, F},
-    node::V,
-) where {P <: AbstractParams, V <: AbstractDFGVariable, F <: AbstractDFGFactor}
-    return haskey(dfg.g.variables, node.label)
+function hasVariable(dfg::GraphsDFG, label::Symbol)
+    return haskey(dfg.g.variables, label)
 end
 
-function exists(
-    dfg::GraphsDFG{P, V, F},
-    node::F,
-) where {P <: AbstractParams, V <: AbstractDFGVariable, F <: AbstractDFGFactor}
-    return haskey(dfg.g.factors, node.label)
+function hasFactor(dfg::GraphsDFG, label::Symbol)
+    return haskey(dfg.g.factors, label)
 end
 
-exists(dfg::GraphsDFG, nId::Symbol) = haskey(dfg.g.labels, nId)
-
-exists(dfg::GraphsDFG, node::DFGNode) = exists(dfg, node.label)
-
 function isVariable(
     dfg::GraphsDFG{P, V, F},
     sym::Symbol,
@@ -109,7 +99,7 @@ function addFactor!(
     # @assert FactorGraphs.addFactor!(dfg.g, getVariableOrder(factor), factor)
     variableLabels = Symbol[factor._variableOrderSymbols...]
     for vlabel in variableLabels
-        !exists(dfg, vlabel) && throw(LabelNotFoundError("Variable", vlabel))
+        !hasVariable(dfg, vlabel) && throw(LabelNotFoundError("Variable", vlabel))
     end
     @assert FactorGraphs.addFactor!(dfg.g, variableLabels, factor)
     return factor
@@ -290,7 +280,7 @@ function listNeighbors(dfg::GraphsDFG, node::DFGNode; solvable::Int = 0)
 end
 
 function listNeighbors(dfg::GraphsDFG, label::Symbol; solvable::Int = 0)
-    if !exists(dfg, label)
+    if !(hasVariable(dfg, label) || hasFactor(dfg, label))
         throw(LabelNotFoundError(label))
     end
 
@@ -468,7 +458,8 @@ function findShortestPathDijkstra(
         dfg
     end
 
-    if !exists(dfg_, from) || !exists(dfg_, to)
+    if !(asVariable(dfg_, from) || hasFactor(dfg_, from)) ||
+       !(asVariable(dfg_, to) || hasFactor(dfg_, to))
         # assume filters excluded either `to` or `from` and hence no shortest path
         return Symbol[]
     end

src/errors.jl

@@ -43,6 +43,44 @@ function Base.showerror(io::IO, ex::LabelExistsError)
     )
 end
 
+"""
+    IdNotFoundError(Id, available)
+
+Error thrown when a requested Id is not found.
+"""
+struct IdNotFoundError <: Exception
+    name::String
+    Id::UUID
+    available::Vector{UUID}
+end
+
+IdNotFoundError(name::String, Id::UUID) = IdNotFoundError(name, Id, UUID[])
+IdNotFoundError(Id::UUID) = IdNotFoundError("Node", Id, UUID[])
+
+function Base.showerror(io::IO, ex::IdNotFoundError)
+    print(io, "IdNotFoundError: ", ex.name, " Id '", ex.Id, "' not found.")
+    if !isempty(ex.available)
+        println(io, " Available Ids:")
+        show(io, ex.available)
+    end
+end
+
+"""
+    IdExistsError(Id)
+
+Error thrown when attempting to add an Id that already exists in the collection.
+"""
+struct IdExistsError <: Exception
+    name::String
+    Id::UUID
+end
+
+IdExistsError(Id::UUID) = IdExistsError("Node", Id)
+
+function Base.showerror(io::IO, ex::IdExistsError)
+    return print(io, "IdExistsError: ", ex.name, " Id '", ex.Id, "' already exists.")
+end
+
 """
     SerializationError(msg)