fix: Fix deletion checks (maybe not very well tested)

src/datastore/src/Server/DataStore.lua

@@ -69,11 +69,12 @@ local DataStoreDeleteToken = require("DataStoreDeleteToken")
 local DataStorePromises = require("DataStorePromises")
 local DataStoreStage = require("DataStoreStage")
 local Maid = require("Maid")
+local Math = require("Math")
 local Promise = require("Promise")
+local Rx = require("Rx")
 local Signal = require("Signal")
-local Math = require("Math")
+local Symbol = require("Symbol")
 local ValueObject = require("ValueObject")
-local Rx = require("Rx")
 
 local DEFAULT_DEBUG_WRITING = false
 
@@ -118,8 +119,7 @@ function DataStore.new(robloxDataStore, key)
 	@prop Saving Signal<Promise>
 	@within DataStore
 ]=]
-	self.Saving = Signal.new() -- :Fire(promise)
-	self._maid:GiveTask(self.Saving)
+	self.Saving = self._maid:Add(Signal.new()) -- :Fire(promise)
 
 	self:_setupAutoSaving()
 
@@ -222,6 +222,7 @@ end
 ]=]
 function DataStore:SetUserIdList(userIdList)
 	assert(type(userIdList) == "table" or userIdList == nil, "Bad userIdList")
+	assert(not Symbol.isSymbol(userIdList), "Should not be symbol")
 
 	self._userIdList = userIdList
 end
@@ -376,6 +377,8 @@ function DataStore:_doDataSync(writer, doMergeNewData)
 				result = {}
 			end
 
+			self:_checkSnapshotIntegrity("writer:WriteMerge(original)", result)
+
 			if self._debugWriting then
 				print("[DataStore] - Writing", result)
 			end
@@ -417,7 +420,7 @@ end
 function DataStore:_promiseGetAsyncNoCache()
 	return self._maid:GivePromise(DataStorePromises.getAsync(self._robloxDataStore, self._key))
 		:Catch(function(err)
-			warn(string.format("DataStorePromises.getAsync(%q) -> warning - ", self._key), err)
+			warn(string.format("DataStorePromises.getAsync(%q) -> warning - %s", tostring(self._key), tostring(err or "empty error")))
 			return Promise.rejected(err)
 		end)
 		:Then(function(data)
@@ -427,7 +430,7 @@ function DataStore:_promiseGetAsyncNoCache()
 			self:MergeDiffSnapshot(diffSnapshot)
 
 			if self._debugWriting then
-				print(string.format("DataStorePromises.getAsync(%q) -> Got ", self._key), data, "with diff snapshot", diffSnapshot, "to view", self._viewSnapshot)
+				print(string.format("DataStorePromises.getAsync(%q) -> Got ", tostring(self._key)), data, "with diff snapshot", diffSnapshot, "to view", self._viewSnapshot)
 				-- print(string.format("DataStorePromises.getAsync(%q) -> Got ", self._key), data)
 			end
 		end)

src/datastore/src/Server/Modules/DataStoreSnapshotUtils.lua

@@ -4,10 +4,12 @@
 
 local require = require(script.Parent.loader).load(script)
 
+local Symbol = require("Symbol")
+
 local DataStoreSnapshotUtils = {}
 
 function DataStoreSnapshotUtils.isEmptySnapshot(snapshot)
-	return type(snapshot) == "table" and next(snapshot) == nil
+	return not Symbol.isSymbol(snapshot) and type(snapshot) == "table" and next(snapshot) == nil
 end
 
 return DataStoreSnapshotUtils

src/datastore/src/Server/Modules/DataStoreStage.lua

@@ -18,6 +18,7 @@ local require = require(script.Parent.loader).load(script)
 
 local BaseObject = require("BaseObject")
 local DataStoreDeleteToken = require("DataStoreDeleteToken")
+local DataStoreSnapshotUtils = require("DataStoreSnapshotUtils")
 local DataStoreWriter = require("DataStoreWriter")
 local GoodSignal = require("GoodSignal")
 local Maid = require("Maid")
@@ -26,10 +27,10 @@ local ObservableSubscriptionTable = require("ObservableSubscriptionTable")
 local Promise = require("Promise")
 local PromiseUtils = require("PromiseUtils")
 local Set = require("Set")
+local Symbol = require("Symbol")
 local Table = require("Table")
-local DataStoreSnapshotUtils = require("DataStoreSnapshotUtils")
 
-local SLOW_INTEGRITY_CHECK_ENABLED = true
+local SLOW_INTEGRITY_CHECK_ENABLED = false
 
 local DataStoreStage = setmetatable({}, BaseObject)
 DataStoreStage.ClassName = "DataStoreStage"
@@ -172,10 +173,10 @@ function DataStoreStage:GetSubStore(key)
 	end
 
 	local maid = Maid.new()
-	local newStore = DataStoreStage.new(key, self)
-	maid:GiveTask(newStore)
+	local newStore = maid:Add(DataStoreStage.new(key, self))
 
-	if type(self._baseDataSnapshot) == "table" then
+	-- TODO: Transfer delete tokens too?
+	if type(self._baseDataSnapshot) == "table" and not Symbol.isSymbol(self._baseDataSnapshot) then
 		local baseDataToTransfer = self._baseDataSnapshot[key]
 		if baseDataToTransfer ~= nil then
 			local newSnapshot = table.clone(self._baseDataSnapshot)
@@ -186,7 +187,7 @@ function DataStoreStage:GetSubStore(key)
 	end
 
 	-- Transfer save data to substore
-	if type(self._saveDataSnapshot) == "table" then
+	if type(self._saveDataSnapshot) == "table" and not Symbol.isSymbol(self._saveDataSnapshot) then
 		local saveDataToTransfer = self._saveDataSnapshot[key]
 
 		if saveDataToTransfer ~= nil then
@@ -507,7 +508,7 @@ function DataStoreStage:Overwrite(data)
 		data = DataStoreDeleteToken
 	end
 
-	if type(data) == "table" then
+	if type(data) == "table" and data ~= DataStoreDeleteToken then
 		local newSaveSnapshot = {}
 
 		local remaining = Set.fromKeys(self._stores)
@@ -719,6 +720,8 @@ function DataStoreStage:_updateStoresAndComputeBaseDataSnapshotFromDiffSnapshot(
 	if diffSnapshot == DataStoreDeleteToken then
 		return nil
 	elseif type(diffSnapshot) == "table" then
+		assert(not Symbol.isSymbol(diffSnapshot), "diffSnapshot should not be symbol")
+
 		local newBaseDataSnapshot
 		if type(self._baseDataSnapshot) == "table" then
 			newBaseDataSnapshot = table.clone(self._baseDataSnapshot)
@@ -976,6 +979,42 @@ function DataStoreStage:_storeAtKey(key, value)
 	self:_checkIntegrity()
 end
 
+function DataStoreStage:_checkSnapshotIntegrity(label, result)
+	assert(type(label) == "string", "Bad label")
+
+	if not SLOW_INTEGRITY_CHECK_ENABLED then
+		return
+	end
+
+	if result == DataStoreDeleteToken then
+		error(string.format("%s should not be a DataStoreDeleteToken", label))
+	end
+
+	local function recurse(innerLabel, value)
+		if type(value) ~= "table" then
+			return
+		end
+
+		for key, item in pairs(value) do
+			local keyLabel = innerLabel .. "." .. tostring(key)
+
+			if not (type(key) == "string" or type(key) == "number") then
+				 error(string.format("%s should be a number or string", keyLabel))
+			end
+
+			if item == DataStoreDeleteToken then
+				error(string.format("%s should not contain a DataStoreDeleteToken", keyLabel))
+			end
+
+			if type(item) == "table" then
+				recurse(keyLabel, item)
+			end
+		end
+	end
+
+	recurse(label, result)
+end
+
 function DataStoreStage:_checkIntegrity()
 	if not SLOW_INTEGRITY_CHECK_ENABLED then
 		return
@@ -986,6 +1025,7 @@ function DataStoreStage:_checkIntegrity()
 
 	if type(self._baseDataSnapshot) == "table" then
 		assert(table.isfrozen(self._baseDataSnapshot), "Base snapshot should be frozen")
+		self:_checkSnapshotIntegrity("self._baseDataSnapshot", self._baseDataSnapshot)
 	end
 
 	if type(self._saveDataSnapshot) == "table" then
@@ -994,6 +1034,7 @@ function DataStoreStage:_checkIntegrity()
 
 	if type(self._viewSnapshot) == "table" then
 		assert(table.isfrozen(self._viewSnapshot), "View snapshot should be frozen")
+		self:_checkSnapshotIntegrity("self._viewSnapshot", self._viewSnapshot)
 	end
 
 	for key, _ in pairs(self._stores) do
@@ -1005,16 +1046,6 @@ function DataStoreStage:_checkIntegrity()
 			error(string.format("[DataStoreStage] - Duplicate saveData at key %q", key))
 		end
 	end
-
-	if type(self._viewSnapshot) == "table" then
-		for key, value in pairs(self._viewSnapshot) do
-			assert(type(key) == "string" or type(key) == "number", "Bad key")
-			if value == DataStoreDeleteToken then
-				error(string.format("[DataStoreStage] - View at key %q is delete token", key))
-			end
-		end
-	end
 end
 
-
 return DataStoreStage

src/datastore/src/Server/Modules/DataStoreWriter.lua

@@ -46,7 +46,7 @@ function DataStoreWriter:SetSaveDataSnapshot(saveDataSnapshot)
 
 	if saveDataSnapshot == DataStoreDeleteToken then
 		self._saveDataSnapshot = DataStoreDeleteToken
-	elseif type(saveDataSnapshot) == "table" then
+	elseif type(saveDataSnapshot) == "table" and not Symbol.isSymbol(saveDataSnapshot) then
 		self._saveDataSnapshot = Table.deepCopy(saveDataSnapshot)
 	else
 		self._saveDataSnapshot = saveDataSnapshot
@@ -67,6 +67,7 @@ end
 
 function DataStoreWriter:SetFullBaseDataSnapshot(fullBaseDataSnapshot)
 	assert(type(fullBaseDataSnapshot) ~= "table" or table.isfrozen(fullBaseDataSnapshot), "fullBaseDataSnapshot should be frozen")
+	assert(not Symbol.isSymbol(fullBaseDataSnapshot), "fullBaseDataSnapshot should not be symbol")
 
 	if fullBaseDataSnapshot == DataStoreDeleteToken then
 		error("[DataStoreWriter] - fullBaseDataSnapshot should not be a delete token")
@@ -109,6 +110,7 @@ end
 ]=]
 function DataStoreWriter:ComputeDiffSnapshot(incoming)
 	assert(incoming ~= DataStoreDeleteToken, "Incoming value should not be DataStoreDeleteToken")
+	assert(not Symbol.isSymbol(incoming), "Incoming should not be symbol")
 
 	if type(incoming) == "table" then
 		local keys = Set.union(Set.fromKeys(self._writers), Set.fromKeys(incoming))
@@ -147,6 +149,8 @@ end
 function DataStoreWriter:_computeValueDiff(original, incoming)
 	assert(original ~= DataStoreDeleteToken, "original cannot be DataStoreDeleteToken")
 	assert(incoming ~= DataStoreDeleteToken, "incoming cannot be DataStoreDeleteToken")
+	assert(not Symbol.isSymbol(original), "original should not be symbol")
+	assert(not Symbol.isSymbol(incoming), "incoming should not be symbol")
 
 	if original == incoming then
 		return nil
@@ -162,6 +166,8 @@ end
 function DataStoreWriter:_computeTableDiff(original, incoming)
 	assert(type(original) == "table", "Bad original")
 	assert(type(incoming) == "table", "Bad incoming")
+	assert(not Symbol.isSymbol(original), "original should not be symbol")
+	assert(not Symbol.isSymbol(incoming), "incoming should not be symbol")
 
 	local keys = Set.union(Set.fromKeys(original), Set.fromKeys(incoming))
 
@@ -203,15 +209,17 @@ end
 
 function DataStoreWriter:_writeMergeWriters(original)
 	local copy
-	if type(original) == "table" then
+	if Symbol.isSymbol(original) then
+		copy = original
+	elseif type(original) == "table" then
 		copy = table.clone(original)
 	else
 		copy = original
 	end
 
 	if next(self._writers) ~= nil then
 		-- Original was not a table. We need to swap to one.
-		if type(copy) ~= "table" then
+		if type(copy) ~= "table" or Symbol.isSymbol(original) then
 			copy = {}
 		end
 
@@ -227,7 +235,7 @@ function DataStoreWriter:_writeMergeWriters(original)
 	end
 
 	-- Write our save data next
-	if type(self._saveDataSnapshot) == "table" and next(self._saveDataSnapshot) ~= nil then
+	if not Symbol.isSymbol(self._saveDataSnapshot) and type(self._saveDataSnapshot) == "table" and next(self._saveDataSnapshot) ~= nil then
 		-- Original was not a table. We need to swap to one.
 		if type(copy) ~= "table" then
 			copy = {}
@@ -248,7 +256,7 @@ function DataStoreWriter:_writeMergeWriters(original)
 
 	-- Handle empty table scenario..
 	-- This would also imply our original is nil somehow...
-	if type(copy) == "table" and next(copy) == nil then
+	if not Symbol.isSymbol(copy) and type(copy) == "table" and next(copy) == nil then
 		if type(self._saveDataSnapshot) ~= "table" then
 			return nil
 		end