Immutable Folderstore id=>blob pair (#1160)

* Immutable Folderstore id=>blob pair
* updateBlob! is obsolete
* test listBlobs

Author: Affie

src/DataBlobs/services/BlobStores.jl

@@ -76,14 +76,6 @@ end
 # also creates an blobid as uuid4
 addBlob!(store::AbstractBlobstore, data) = addBlob!(store, uuid4(), data)
 
-#update
-function updateBlob!(dfg::AbstractDFG, entry::Blobentry, data)
-    return updateBlob!(getBlobstore(dfg, entry.blobstore), entry.blobid, data)
-end
-
-function updateBlob!(store::AbstractBlobstore, entry::Blobentry, data)
-    return updateBlob!(store, entry.blobid, data)
-end
 #delete
 function deleteBlob!(dfg::AbstractDFG, entry::Blobentry)
     return deleteBlob!(getBlobstore(dfg, entry.blobstore), entry)
@@ -147,7 +139,10 @@ end
 
 function getBlob(store::FolderStore{T}, blobid::UUID) where {T}
     blobfilename = joinpath(store.folder, string(store.label), string(blobid))
-    if isfile(blobfilename)
+    tombstonefile = blobfilename * ".deleted"
+    if isfile(tombstonefile)
+        throw(IdNotFoundError("Blob (deleted)", blobid))
+    elseif isfile(blobfilename)
         open(blobfilename) do f
             return read(f)
         end
@@ -168,25 +163,25 @@ function addBlob!(store::FolderStore{T}, blobid::UUID, data::T) where {T}
     end
 end
 
-function updateBlob!(store::FolderStore{T}, blobid::UUID, data::T) where {T}
-    blobfilename = joinpath(store.folder, string(store.label), string(blobid))
-    if !isfile(blobfilename)
-        @warn "Key '$blobid' doesn't exist."
-    else
-        open(blobfilename, "w") do f
-            return write(f, data)
-        end
-        return data
-    end
-end
-
 function deleteBlob!(store::FolderStore{T}, blobid::UUID) where {T}
+    # Tombstone pattern: instead of deleting the file, create a tombstone marker file
     blobfilename = joinpath(store.folder, string(store.label), string(blobid))
-    if !isfile(blobfilename)
+    tombstonefile = blobfilename * ".deleted"
+    if isfile(blobfilename)
+        # Remove the actual blob file
+        rm(blobfilename)
+        # Create a tombstone marker
+        open(tombstonefile, "w") do f
+            return write(f, "deleted")
+        end
+        return 1
+    elseif isfile(tombstonefile)
+        # Already deleted
+        return 0
+    else
+        # Not found
         throw(IdNotFoundError("Blob", blobid))
     end
-    rm(blobfilename)
-    return 1
 end
 
 function hasBlob(store::FolderStore, blobid::UUID)
@@ -196,7 +191,18 @@ end
 
 hasBlob(store::FolderStore, entry::Blobentry) = hasBlob(store, entry.blobid)
 
-listBlobs(store::FolderStore) = readdir(store.folder)
+function listBlobs(store::FolderStore)
+    folder = joinpath(store.folder, string(store.label))
+    # Parse folder to only include UUIDs automatically excluding tombstone files this way.
+    blobids = UUID[]
+    for filename in readdir(folder)
+        id = tryparse(UUID, filename)
+        isnothing(id) && continue
+        push!(blobids, id)
+    end
+    return blobids
+end
+
 ##==============================================================================
 ## InMemoryBlobstore
 ##==============================================================================
@@ -228,13 +234,6 @@ function addBlob!(store::InMemoryBlobstore{T}, blobid::UUID, data::T) where {T}
     return blobid
 end
 
-function updateBlob!(store::InMemoryBlobstore{T}, blobid::UUID, data::T) where {T}
-    if haskey(store.blobs, blobid)
-        @warn "Key '$blobid' doesn't exist."
-    end
-    return store.blobs[blobid] = data
-end
-
 function deleteBlob!(store::InMemoryBlobstore, blobid::UUID)
     if !haskey(store.blobs, blobid)
         throw(IdNotFoundError("Blob", blobid))

src/Deprecated.jl

@@ -344,6 +344,10 @@ function getFactorState(args...)
     )
 end
 
+function updateBlob!(args...)
+    return error("updateBlob! is obsolete as blobid=>Blob pairs are immutable.")
+end
+
 ## ================================================================================
 ## Deprecated in v0.28
 ##=================================================================================

src/DistributedFactorGraphs.jl

@@ -427,8 +427,6 @@ const unstable_functions::Vector{Symbol} = [
     :mergeGraph!,
     :buildSubgraph,
     :incrDataLabelSuffix,# TODO somewhat used, do we deprecate?
-    :updateMetadata!,## TODO deprecated or obsolete
-    :updateBlob!,## TODO deprecated or obsolete
 
     # set # TODO what to do here, maybe `ref` verb + setproperty.  
     :setSolverParams!,
@@ -440,6 +438,8 @@ const unstable_functions::Vector{Symbol} = [
     # no set on these
 
     #deprecated in v0.29
+    :updateMetadata!,## TODO deprecated or obsolete
+    :updateBlob!,## TODO deprecated or obsolete
     :getFactorState, # FIXME getFactorState were questioned and being reviewed again for name, other than that they are checked.
     :packDistribution,
     :unpackDistribution,

test/testBlocks.jl

@@ -911,12 +911,15 @@ function blobsStoresTestBlock!(fg)
     blobid = addBlob!(fs, testData)
     @test blobid isa UUID
     @test hasBlob(fs, blobid)
+    @test listBlobs(fs) == [blobid]
     @test_throws DFG.IdExistsError addBlob!(fs, blobid, testData)
     @test getBlob(fs, blobid) == testData
     @test_throws DFG.IdNotFoundError getBlob(fs, uuid4())
     @test_throws DFG.IdNotFoundError deleteBlob!(fs, uuid4())
     @test deleteBlob!(fs, blobid) == 1
     @test_throws DFG.IdNotFoundError getBlob(fs, blobid)
+    @test deleteBlob!(fs, blobid) == 0
+    @test listBlobs(fs) == UUID[]
 
     # Blob Wrappers
     # on Variable