Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Johannes Terblanche <6612981+Affie@users.noreply.github.com>

Author: Affie

NEWS.md

@@ -20,7 +20,7 @@ Listing news on any major breaking changes in DFG.  For regular changes, see int
   - `saveBlob_Variable!` → `saveVariableBlob!`, `loadBlob_Variable` → `loadVariableBlob`
   - Same pattern for Factor, Graph, Agent variants.
   - Old names kept as `const` aliases for backward compatibility.
-  - `deleteBlob_Variable` and similar wrappers no longer exist. Use `deleteVariableBlobentry!` (metadata) and `purgeBlob!(provider, multihash)` (physical) directly. Warning: be carefull when deleting blobs to make sure they are no longer in use anywhere because the multihash id can be shared by multiple blobs.
+  - `deleteBlob_Variable` and similar wrappers no longer exist. Use `deleteVariableBlobentry!` (metadata) and `purgeBlob!(provider, multihash)` (physical) directly. Warning: be careful when deleting blobs to make sure they are no longer in use anywhere because the multihash id can be shared by multiple blobs.
 
 # v0.28
 - Reading or deserialzing of factor graphs created prior to v0.25 are no longer suppoted with the complete removal of User/Robot/Session

src/Blobproviders/Blobproviders.jl

@@ -168,7 +168,7 @@ function fetchBlob(store::CachedBlobprovider, m::Multihash)
     !isnothing(blob) && return blob
     blob = fetchBlob(store.remote_provider, m)
     if !isnothing(blob)
-        putBlob!(store.local_provider, blob)
+        putBlob!(store.local_provider, m, blob)
     end
     return blob
 end

src/entities/Blobentry.jl

@@ -225,7 +225,7 @@ Checks the integrity of a blob against the hashes stored in the given `Blobentry
 - Additionally verifies the `crchash` (crc32c) if present.
 - Returns `true` if all present hashes match.
 - Returns `false` if any hash does not match.
-- Returns `nothing` if only the multihash is present but the algorithm is unregistered.
+- Returns `nothing` if  the multihash algorithm is unregistered.
 """
 function checkHash(entry::Blobentry, blob)
     # Reverse lookup: multicodec code -> hash function
@@ -235,6 +235,7 @@ function checkHash(entry::Blobentry, blob)
     func = get(code_to_func, code, nothing)
     if isnothing(func)
         @warn "checkHash: unregistered multihash algorithm code $(repr(code)), skipping multihash check"
+        return nothing
     else
         func(blob) != stored_digest && return false
     end

src/services/blobprovider_ops.jl

@@ -119,9 +119,10 @@ function getBlob(dfg::AbstractDFG, entry::Blobentry)
     providers = refBlobproviders(dfg)
 
     if isempty(providers)
-        error(
+        @warn(
             "No Blobproviders mounted on DFG. Add one with `addBlobprovider!(dfg, FolderBlobprovider(path))` before storing blobs. Cannot retrieve multihash: $(entry.multihash)",
         )
+        throw(LabelNotFoundError("Blobprovider", entry.provider, collect(keys(providers))))
     end
 
     # 1. Build the search order: Hinted provider first, followed by the rest
@@ -170,11 +171,9 @@ function hasBlob(provider::AbstractBlobprovider, entry::Blobentry)
 end
 function hasBlob(dfg::AbstractDFG, entry::Blobentry)
     # CAS: check all providers — the blob may have been stored via a different route
+    #TODO check entry.provider first
     for (_, provider) in refBlobproviders(dfg)
-        try
-            hasBlob(provider, entry.multihash) && return true
-        catch
-        end
+        hasBlob(provider, entry.multihash) && return true
     end
     return false
 end

test/iifCompareTests.jl

@@ -110,6 +110,6 @@ end
 
     sfg = getSubgraph(GraphsDFG, fg, [:x0; :x1])
 
-    #TODO add proper subraph tests here (or remove testset), spot check for now.
+    #TODO add proper subgraph tests here (or remove testset), spot check for now.
     @test issetequal(listVariables(sfg), [:x0; :x1])
 end