From 71c6a998001bd0296ce2a7e9bf9b05f304e91acc Mon Sep 17 00:00:00 2001 From: James Onnen Date: Wed, 6 Nov 2024 14:11:30 -0800 Subject: [PATCH] fix: Fix deletion checks (maybe not very well tested) --- src/datastore/src/Server/DataStore.lua | 15 +++-- .../Server/Modules/DataStoreSnapshotUtils.lua | 4 +- .../src/Server/Modules/DataStoreStage.lua | 65 ++++++++++++++----- .../src/Server/Modules/DataStoreWriter.lua | 18 +++-- 4 files changed, 73 insertions(+), 29 deletions(-) diff --git a/src/datastore/src/Server/DataStore.lua b/src/datastore/src/Server/DataStore.lua index abc3a639d9..e9638c4a38 100644 --- a/src/datastore/src/Server/DataStore.lua +++ b/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 @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) diff --git a/src/datastore/src/Server/Modules/DataStoreSnapshotUtils.lua b/src/datastore/src/Server/Modules/DataStoreSnapshotUtils.lua index 11ad83b3e2..24ca342d26 100644 --- a/src/datastore/src/Server/Modules/DataStoreSnapshotUtils.lua +++ b/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 \ No newline at end of file diff --git a/src/datastore/src/Server/Modules/DataStoreStage.lua b/src/datastore/src/Server/Modules/DataStoreStage.lua index 3df7beaff8..bfc9f86a4f 100644 --- a/src/datastore/src/Server/Modules/DataStoreStage.lua +++ b/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 \ No newline at end of file diff --git a/src/datastore/src/Server/Modules/DataStoreWriter.lua b/src/datastore/src/Server/Modules/DataStoreWriter.lua index ab92eaaea7..0695a8c319 100644 --- a/src/datastore/src/Server/Modules/DataStoreWriter.lua +++ b/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,7 +209,9 @@ 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 @@ -211,7 +219,7 @@ function DataStoreWriter:_writeMergeWriters(original) 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