More DFGv1 cleanups and checks and Blobstore updates (#1159)

* _version -> VersionNumber

Author: Affie

src/DataBlobs/entities/BlobEntry.jl

@@ -41,12 +41,10 @@ Base.@kwdef struct Blobentry
     createdTimestamp::Union{ZonedDateTime, Nothing} = nothing
     """ Use carefully, but necessary to support advanced usage such as time synchronization over Blob data. """
     lastUpdatedTimestamp::Union{ZonedDateTime, Nothing} = nothing
-    """ Type version of this Blobentry. TBD.jl consider upgrading to `::VersionNumber`. """
-    _version::String = string(_getDFGVersion())
+    """ Type version of this Blobentry."""
+    _version::VersionNumber = _getDFGVersion()
 end
 
 StructTypes.StructType(::Type{Blobentry}) = StructTypes.UnorderedStruct()
 StructTypes.idproperty(::Type{Blobentry}) = :id
 StructTypes.omitempties(::Type{Blobentry}) = (:id,)
-
-_fixtimezone(cts::NamedTuple) = ZonedDateTime(cts.utc_datetime * "+00")

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/DataBlobs/services/HelpersDataWrapEntryBlob.jl

@@ -57,7 +57,7 @@ function Blobentry(
     timestamp::ZonedDateTime = entry.timestamp,
     createdTimestamp = entry.createdTimestamp,
     lastUpdatedTimestamp = entry.lastUpdatedTimestamp,
-    _version::String = entry._version,
+    _version = entry._version,
 )
     return Blobentry(;
         id,
@@ -267,7 +267,7 @@ function updateData!(
         blobstore = getLabel(blobstore),
         hash = string(bytes2hex(hashfunction(blob))),
         origin = buildSourceString(dfg, label),
-        _version = string(_getDFGVersion()),
+        _version = _getDFGVersion(),
     )
     mergeBlobentry!(dfg, label, newEntry)
     updateBlob!(blobstore, newEntry, blob)

src/Deprecated.jl

@@ -21,10 +21,10 @@ const InferenceType = AbstractPackedFactorObservation
 const PackedSamplableBelief = PackedBelief
 
 export setSolverData!
-"""
-    $SIGNATURES
-Set solver data structure stored in a variable.
-"""
+# """
+#     $SIGNATURES
+# Set solver data structure stored in a variable.
+# """
 function setSolverData!(v::VariableCompute, data::VariableState, key::Symbol = :default)
     Base.depwarn(
         "setSolverData!(v::VariableCompute, data::VariableState, key::Symbol = :default) is deprecated, use mergeVariableState! instead.",
@@ -51,6 +51,104 @@ function mergeGraphVariableData!(args...)
     )
 end
 
+#NOTE List types funcction do not fit verb noun and will be deprecated.
+# should return types
+
+# """
+#     $SIGNATURES
+
+# Return `Vector{Symbol}` of all unique variable types in factor graph.
+# """
+function lsTypes(dfg::AbstractDFG)
+    vars = getVariables(dfg)
+    alltypes = Set{Symbol}()
+    for v in vars
+        varType = Symbol(typeof(getVariableType(v)))
+        push!(alltypes, varType)
+    end
+    return collect(alltypes)
+end
+
+# """
+#     $SIGNATURES
+
+# Return `::Dict{Symbol, Vector{Symbol}}` of all unique variable types with labels in a factor graph.
+# """
+function lsTypesDict(dfg::AbstractDFG)
+    vars = getVariables(dfg)
+    alltypes = Dict{Symbol, Vector{Symbol}}()
+    for v in vars
+        varType = Symbol(typeof(getVariableType(v)))
+        d = get!(alltypes, varType, Symbol[])
+        push!(d, v.label)
+    end
+    return alltypes
+end
+
+# """
+#     $SIGNATURES
+
+# Return `Vector{Symbol}` of all unique factor types in factor graph.
+# """
+function lsfTypes(dfg::AbstractDFG)
+    facs = getFactors(dfg)
+    alltypes = Set{Symbol}()
+    for f in facs
+        facType = typeof(getFactorType(f)) |> nameof
+        push!(alltypes, facType)
+    end
+    return collect(alltypes)
+end
+
+# """
+#     $SIGNATURES
+
+# Return `::Dict{Symbol, Vector{Symbol}}` of all unique factors types with labels in a factor graph.
+# """
+function lsfTypesDict(dfg::AbstractDFG)
+    facs = getFactors(dfg)
+    alltypes = Dict{Symbol, Vector{Symbol}}()
+    for f in facs
+        facType = typeof(getFactorType(f)) |> nameof
+        d = get!(alltypes, facType, Symbol[])
+        push!(d, f.label)
+    end
+    return alltypes
+end
+
+# solvekey is deprecated and sync!/copyto! is the better verb.
+#TODO replace with syncVariableStates! or similar
+# """
+#     $SIGNATURES
+# Duplicate a `solveKey`` into a destination from a source.
+
+# Notes
+# - Can copy between graphs, or to different solveKeys within one graph.
+# """
+function cloneSolveKey!(
+    dest_dfg::AbstractDFG,
+    dest::Symbol,
+    src_dfg::AbstractDFG,
+    src::Symbol;
+    solvable::Int = 0,
+    labels = intersect(ls(dest_dfg; solvable = solvable), ls(src_dfg; solvable = solvable)),
+    verbose::Bool = false,
+)
+    #
+    for x in labels
+        sd = deepcopy(getVariableState(getVariable(src_dfg, x), src))
+        copytoVariableState!(dest_dfg, x, dest, sd)
+    end
+
+    return nothing
+end
+
+function cloneSolveKey!(dfg::AbstractDFG, dest::Symbol, src::Symbol; kw...)
+    #
+    @assert dest != src "Must copy to a different solveKey within the same graph, $dest."
+    return cloneSolveKey!(dfg, dest, dfg, src; kw...)
+end
+
 ## ================================================================================
 ## Deprecated in v0.27
 ##=================================================================================