diff --git a/src/Runtime/BinaryStorage.cs b/src/Runtime/BinaryStorage.cs index 9094f6c..de8a416 100644 --- a/src/Runtime/BinaryStorage.cs +++ b/src/Runtime/BinaryStorage.cs @@ -13,8 +13,10 @@ public partial class BinaryStorage : IDisposable, IBinaryStorage private readonly string _storageFilePath; private readonly StoragePersistence _persistence; private readonly IReadOnlyList _supportedTypes; + private readonly Dictionary _sectionIndexByType; private readonly Dictionary _data = new(); private readonly Dictionary _collections = new(); + private readonly Action _decreaseCounter; private int _changeScopeCounter; private bool _hasUnsavedChanges; @@ -45,6 +47,12 @@ internal BinaryStorage(string storageFilePath, IReadOnlyList supp { _storageFilePath = storageFilePath; _supportedTypes = supportedTypes; + _sectionIndexByType = new Dictionary(supportedTypes.Count); + for (var i = 0; i < supportedTypes.Count; i++) + { + _sectionIndexByType[supportedTypes[i].Type] = i; + } + _decreaseCounter = DecreaseCounter; _persistence = new StoragePersistence(storageFilePath, supportedTypes, saveOnBackgroundThread); } @@ -104,7 +112,7 @@ public virtual bool SetRaw(string key, object value, TypeMismatchBehaviour? over } var valueType = value.GetType(); - if (valueType.IsCollection()) + if (CollectionTypeCache.IsCollection(valueType)) { throw new IncorrectUsageOfCollectionException(nameof(SetRaw), valueType); } @@ -128,23 +136,17 @@ public virtual bool SetRaw(string key, object value, TypeMismatchBehaviour? over return true; } - var mismatchBehaviour = overrideTypeMismatchBehaviour ?? TypeMismatchBehaviour; - switch (mismatchBehaviour) - { - case TypeMismatchBehaviour.OverrideValueAndType: - using (new ChangeScope(this)) - { - RemoveRecord(key); - AddRawRecord(key, value, valueType); - } - return true; - case TypeMismatchBehaviour.ThrowException: - throw new UnexpectedTypeException(key, nameof(SetRaw), record.Type, valueType); - case TypeMismatchBehaviour.Ignore: - return false; - default: - throw new UnexpectedEnumException(typeof(TypeMismatchBehaviour), mismatchBehaviour); + if (!ShouldReplaceMismatchedRecord(key, record, valueType, overrideTypeMismatchBehaviour)) + { + return false; } + + using (new ChangeScope(this)) + { + RemoveRecord(key); + AddRawRecord(key, value, valueType); + } + return true; } /// Determines whether the specified key exists in the storage. @@ -236,23 +238,17 @@ public virtual bool Set(string key, T value, TypeMismatchBehaviour? overrideT return ChangeRecord(key, typedRecord, value); } - var mismatchBehaviour = overrideTypeMismatchBehaviour ?? TypeMismatchBehaviour; - switch (mismatchBehaviour) - { - case TypeMismatchBehaviour.OverrideValueAndType: - using (new ChangeScope(this)) - { - RemoveRecord(key); - AddRecord(key, value); - } - return true; - case TypeMismatchBehaviour.ThrowException: - throw new UnexpectedTypeException(key, nameof(Set), record.Type, typeof(T)); - case TypeMismatchBehaviour.Ignore: - return false; - default: - throw new UnexpectedEnumException(typeof(TypeMismatchBehaviour), mismatchBehaviour); + if (!ShouldReplaceMismatchedRecord(key, record, typeof(T), overrideTypeMismatchBehaviour)) + { + return false; + } + + using (new ChangeScope(this)) + { + RemoveRecord(key); + AddRecord(key, value); } + return true; } /// @@ -320,7 +316,7 @@ public IDisposable MultipleChangeScope() { ThrowIfDisposed(); _changeScopeCounter++; - return new DisposableScope(DecreaseCounter); + return new DisposableScope(_decreaseCounter); } #region Collections @@ -448,12 +444,7 @@ private Record AddRecord(string key, T value) var record = new Record(value, typeIndex); section.Count++; _data.Add(key, record); - var rc = record.AsReactiveCollection(); - if (rc != null) - { - _collections.Add(rc, key); - rc.OnChanged += ReactiveCollectionChanged; - } + TrackCollectionOf(record, key); MarkChanged(); OnKeyAdded?.Invoke(key); return record; @@ -466,15 +457,7 @@ private Record AddRecord(string key, T value) /// Thrown if the type is not registered. private void AddRawRecord(string key, object value, Type valueType) { - var typeIndex = -1; - for (var i = 0; i < _supportedTypes.Count; i++) - { - if (_supportedTypes[i].Type == valueType) - { - typeIndex = i; - break; - } - } + var typeIndex = IndexOfSection(valueType); if (typeIndex == -1) { throw new UnregisteredTypeException(valueType); @@ -518,13 +501,7 @@ private bool RemoveRecord(string key) { return false; } - var rc = value.AsReactiveCollection(); - if (rc != null) - { - rc.OnChanged -= ReactiveCollectionChanged; - rc.Dispose(); - _collections.Remove(rc); - } + UntrackCollectionOf(value); _supportedTypes[value.TypeIndex].Count--; _data.Remove(key); MarkChanged(); @@ -540,14 +517,7 @@ private void RemoveAllRecords() { foreach (var record in _data.Values) { - var rc = record.AsReactiveCollection(); - if (rc == null) - { - continue; - } - rc.OnChanged -= ReactiveCollectionChanged; - rc.Dispose(); - _collections.Remove(rc); + UntrackCollectionOf(record); } _data.Clear(); for (var i = 0; i < _supportedTypes.Count; i++) @@ -573,14 +543,50 @@ private Record GetRecord(string key) private int IndexOfSection() { - for (var i = 0; i < _supportedTypes.Count; i++) + return IndexOfSection(typeof(T)); + } + + private int IndexOfSection(Type type) + { + return _sectionIndexByType.TryGetValue(type, out var index) ? index : -1; + } + + /// Decides whether a record whose stored type differs from the type being written has to be replaced. + /// True if the record has to be replaced; false if the write has to be ignored. + /// Thrown if the mismatch behavior is to throw. + private bool ShouldReplaceMismatchedRecord(string key, Record record, Type valueType, TypeMismatchBehaviour? overrideTypeMismatchBehaviour, [CallerMemberName] string action = null) + { + var mismatchBehaviour = overrideTypeMismatchBehaviour ?? TypeMismatchBehaviour; + return mismatchBehaviour switch { - if (_supportedTypes[i] is TypedBinarySection) - { - return i; - } + TypeMismatchBehaviour.OverrideValueAndType => true, + TypeMismatchBehaviour.Ignore => false, + TypeMismatchBehaviour.ThrowException => throw new UnexpectedTypeException(key, action, record.Type, valueType), + _ => throw new UnexpectedEnumException(typeof(TypeMismatchBehaviour), mismatchBehaviour) + }; + } + + private void TrackCollectionOf(Record record, string key) + { + var collection = record.AsReactiveCollection(); + if (collection == null) + { + return; } - return -1; + _collections.Add(collection, key); + collection.OnChanged += ReactiveCollectionChanged; + } + + private void UntrackCollectionOf(Record record) + { + var collection = record.AsReactiveCollection(); + if (collection == null) + { + return; + } + collection.OnChanged -= ReactiveCollectionChanged; + collection.Dispose(); + _collections.Remove(collection); } /// @@ -594,14 +600,11 @@ private void DecreaseCounter() return; } _changeScopeCounter--; - if (IsDisposed) + if (IsDisposed || !_hasUnsavedChanges) { return; } - if (_changeScopeCounter == 0 && _hasUnsavedChanges && AutoSave) - { - SaveDataOnDisk(false); - } + MarkChanged(); } /// Reacts to a change in a reactive collection. @@ -689,14 +692,7 @@ private void Dispose(bool disposing) // Always dispose IReactiveCollection instances foreach (var record in _data.Values) { - var rc = record.AsReactiveCollection(); - if (rc == null) - { - continue; - } - rc.OnChanged -= ReactiveCollectionChanged; - rc.Dispose(); - _collections.Remove(rc); + UntrackCollectionOf(record); } OnKeyAdded = null; @@ -731,12 +727,7 @@ private void LoadDataFromDisk(KeyLoadFailedBehaviour keyLoadFailedBehaviour) _persistence.Load(_data, keyLoadFailedBehaviour); foreach (var pair in _data) { - var rc = pair.Value.AsReactiveCollection(); - if (rc != null) - { - _collections.Add(rc, pair.Key); - rc.OnChanged += ReactiveCollectionChanged; - } + TrackCollectionOf(pair.Value, pair.Key); } } diff --git a/src/Runtime/Collections/ReactiveCollection.cs b/src/Runtime/Collections/ReactiveCollection.cs new file mode 100644 index 0000000..5e4a0cc --- /dev/null +++ b/src/Runtime/Collections/ReactiveCollection.cs @@ -0,0 +1,38 @@ +using System; + +namespace Appegy.Storage +{ + internal abstract class ReactiveCollection : IReactiveCollection + { + public bool IsDisposed { get; private set; } + + public event Action OnChanged; + + protected abstract string ObjectName { get; } + + public abstract void Clear(); + + public void Dispose() + { + if (IsDisposed) + { + return; + } + Clear(); + IsDisposed = true; + } + + protected void SetDirty() + { + OnChanged?.Invoke(this); + } + + protected void ThrowIfDisposed() + { + if (IsDisposed) + { + throw new ObjectDisposedException(ObjectName); + } + } + } +} diff --git a/src/Runtime/Collections/ReactiveCollection.cs.meta b/src/Runtime/Collections/ReactiveCollection.cs.meta new file mode 100644 index 0000000..157ee7d --- /dev/null +++ b/src/Runtime/Collections/ReactiveCollection.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 4cdab2a8cf363ed409cecb771610aac0 \ No newline at end of file diff --git a/src/Runtime/Collections/ReactiveDictionary.cs b/src/Runtime/Collections/ReactiveDictionary.cs index 47c6b1c..ffc2133 100644 --- a/src/Runtime/Collections/ReactiveDictionary.cs +++ b/src/Runtime/Collections/ReactiveDictionary.cs @@ -1,42 +1,16 @@ -using System; -using System.Collections; +using System.Collections; using System.Collections.Generic; namespace Appegy.Storage { - internal class ReactiveDictionary : IReactiveCollection, IDictionary, IReadOnlyDictionary + internal class ReactiveDictionary : ReactiveCollection, IDictionary, IReadOnlyDictionary { private readonly Dictionary _dictionary = new(); - public bool IsDisposed { get; private set; } - - public event Action OnChanged; - - private void SetDirty() - { - OnChanged?.Invoke(this); - } - - private void ThrowIfDisposed() - { - if (IsDisposed) - { - throw new ObjectDisposedException(nameof(ReactiveDictionary)); - } - } + protected override string ObjectName => nameof(ReactiveDictionary); #region Mutable functionallity - public void Dispose() - { - if (IsDisposed) - { - return; - } - Clear(); - IsDisposed = true; - } - public TValue this[TKey key] { get @@ -59,7 +33,7 @@ public void Add(KeyValuePair item) SetDirty(); } - public void Clear() + public override void Clear() { ThrowIfDisposed(); _dictionary.Clear(); diff --git a/src/Runtime/Collections/ReactiveList.cs b/src/Runtime/Collections/ReactiveList.cs index f847366..788a2ef 100644 --- a/src/Runtime/Collections/ReactiveList.cs +++ b/src/Runtime/Collections/ReactiveList.cs @@ -1,42 +1,16 @@ -using System; using System.Collections; using System.Collections.Generic; namespace Appegy.Storage { - internal class ReactiveList : IReactiveCollection, IList, IReadOnlyList + internal class ReactiveList : ReactiveCollection, IList, IReadOnlyList { private readonly List _list = new(); - public bool IsDisposed { get; private set; } - - public event Action OnChanged; - - private void SetDirty() - { - OnChanged?.Invoke(this); - } - - private void ThrowIfDisposed() - { - if (IsDisposed) - { - throw new ObjectDisposedException(nameof(ReactiveList)); - } - } + protected override string ObjectName => nameof(ReactiveList); #region Mutable functionallity - public void Dispose() - { - if (IsDisposed) - { - return; - } - Clear(); - IsDisposed = true; - } - public T this[int index] { get @@ -59,7 +33,7 @@ public void Add(T item) SetDirty(); } - public void Clear() + public override void Clear() { ThrowIfDisposed(); if (_list.Count > 0) diff --git a/src/Runtime/Collections/ReactiveSet.cs b/src/Runtime/Collections/ReactiveSet.cs index 870bb8b..c19fda7 100644 --- a/src/Runtime/Collections/ReactiveSet.cs +++ b/src/Runtime/Collections/ReactiveSet.cs @@ -1,43 +1,17 @@ -using System; -using System.Collections; +using System.Collections; using System.Collections.Generic; using JetBrains.Annotations; namespace Appegy.Storage { - internal class ReactiveSet : IReactiveCollection, ISet, IReadOnlyCollection + internal class ReactiveSet : ReactiveCollection, ISet, IReadOnlyCollection { private readonly HashSet _set = new(); - public bool IsDisposed { get; private set; } - - public event Action OnChanged; - - private void SetDirty() - { - OnChanged?.Invoke(this); - } - - private void ThrowIfDisposed() - { - if (IsDisposed) - { - throw new ObjectDisposedException(nameof(ReactiveSet)); - } - } + protected override string ObjectName => nameof(ReactiveSet); #region Mutable functionallity - public void Dispose() - { - if (IsDisposed) - { - return; - } - Clear(); - IsDisposed = true; - } - public void ExceptWith(IEnumerable other) { ThrowIfDisposed(); @@ -95,7 +69,7 @@ public bool Add([CanBeNull] T item) return added; } - public void Clear() + public override void Clear() { ThrowIfDisposed(); var count = Count; diff --git a/src/Runtime/Internals/NestedBinaryStorage.cs b/src/Runtime/Internals/NestedBinaryStorage.cs index 6a3864b..553a460 100644 --- a/src/Runtime/Internals/NestedBinaryStorage.cs +++ b/src/Runtime/Internals/NestedBinaryStorage.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq; using UnityEngine.Pool; namespace Appegy.Storage @@ -25,10 +24,16 @@ public IReadOnlyCollection Keys { get { - return _root.Keys - .Where(k => k.StartsWith(_prefix, StringComparison.Ordinal)) - .Select(k => k.Substring(_prefix.Length)) - .ToArray(); + var rootKeys = _root.Keys; + var keys = new List(rootKeys.Count); + foreach (var key in rootKeys) + { + if (TryExtractKey(key, out var extracted)) + { + keys.Add(extracted); + } + } + return keys; } } @@ -68,9 +73,14 @@ private void ForgetKeysMissingFromRoot() _keysAllowedBeforeCleanup = Math.Max(MinKeysAddedBetweenCleanups, _prefixedKeys.Count); } + private bool HasPrefix(string key) + { + return key.StartsWith(_prefix, StringComparison.Ordinal); + } + private bool TryExtractKey(string key, out string value) { - if (key.StartsWith(_prefix, StringComparison.Ordinal)) + if (HasPrefix(key)) { value = key.Substring(_prefix.Length); return true; @@ -126,7 +136,7 @@ public int Remove(Func predicate) public int RemoveAll() { - return _root.Remove(key => key.StartsWith(_prefix, StringComparison.Ordinal)); + return _root.Remove(HasPrefix); } public void Save() diff --git a/src/Runtime/Internals/Record.cs b/src/Runtime/Internals/Record.cs index eb4524e..d35aa2d 100644 --- a/src/Runtime/Internals/Record.cs +++ b/src/Runtime/Internals/Record.cs @@ -15,7 +15,7 @@ internal class Record : Record { private static readonly bool _valueCanBeReactiveCollection = typeof(IReactiveCollection).IsAssignableFrom(typeof(T)); - public override Type Type { get; } + public override Type Type => typeof(T); public override int TypeIndex { get; } public override Object Object => Value; public T Value { get; set; } @@ -27,7 +27,6 @@ public override IReactiveCollection AsReactiveCollection() public Record(T value, int typeIndex) { - Type = typeof(T); TypeIndex = typeIndex; Value = value; } diff --git a/src/Runtime/Utilities/CollectionExtensions.cs b/src/Runtime/Utilities/CollectionExtensions.cs deleted file mode 100644 index e8d290a..0000000 --- a/src/Runtime/Utilities/CollectionExtensions.cs +++ /dev/null @@ -1,100 +0,0 @@ -using System; -using System.Collections.Generic; - -namespace Appegy.Storage -{ - internal static class CollectionExtensions - { - #region AddRange - - public static void AddRange(this ICollection source, T item1, T item2) - { - source.Add(item1); - source.Add(item2); - } - - public static void AddRange(this ICollection source, T item1, T item2, T item3) - { - source.Add(item1); - source.Add(item2); - source.Add(item3); - } - - public static void AddRange(this ICollection source, T item1, T item2, T item3, T item4) - { - source.Add(item1); - source.Add(item2); - source.Add(item3); - source.Add(item4); - } - - public static void AddRange(this ICollection source, params T[] items) - { - items.ForEach(source.Add); - } - - public static void AddRange(this IDictionary source, (TKey Key, TValue Value) item1, (TKey Key, TValue Value) item2) - { - source.Add(item1.Key, item1.Value); - source.Add(item2.Key, item2.Value); - } - - public static void AddRange(this IDictionary source, (TKey Key, TValue Value) item1, (TKey Key, TValue Value) item2, (TKey Key, TValue Value) item3) - { - source.Add(item1.Key, item1.Value); - source.Add(item2.Key, item2.Value); - source.Add(item3.Key, item3.Value); - } - - public static void AddRange(this IDictionary source, (TKey Key, TValue Value) item1, (TKey Key, TValue Value) item2, (TKey Key, TValue Value) item3, - (TKey Key, TValue Value) item4) - { - source.Add(item1.Key, item1.Value); - source.Add(item2.Key, item2.Value); - source.Add(item3.Key, item3.Value); - source.Add(item4.Key, item4.Value); - } - - public static void AddRange(this IDictionary source, params (TKey Key, TValue Value)[] items) - { - items.ForEach(item => source.Add(item.Key, item.Value)); - } - - #endregion - - public static bool IsCollection(this Type type) - { - return CollectionTypeCache.IsCollection(type); - } - - public static void ForEach(this Span source, Action predicate) - { - foreach (var item in source) - { - predicate(item); - } - } - - public static void ForEach(this IEnumerable source, Action predicate) - { - foreach (var item in source) - { - predicate(item); - } - } - - public static int FindIndex(this IEnumerable source, Func predicate) - { - var i = 0; - foreach (var item in source) - { - if (predicate(item)) - { - return i; - } - i++; - } - return -1; - } - } -} diff --git a/src/Runtime/Utilities/CollectionExtensions.cs.meta b/src/Runtime/Utilities/CollectionExtensions.cs.meta deleted file mode 100644 index f58ab83..0000000 --- a/src/Runtime/Utilities/CollectionExtensions.cs.meta +++ /dev/null @@ -1,3 +0,0 @@ -fileFormatVersion: 2 -guid: 80e17a16a9f9446e86e5240c4c80fb82 -timeCreated: 1708167224 \ No newline at end of file diff --git a/src/Tests/BaseStorageTests.cs b/src/Tests/BaseStorageTests.cs index 03fd4aa..2ef29d3 100644 --- a/src/Tests/BaseStorageTests.cs +++ b/src/Tests/BaseStorageTests.cs @@ -19,6 +19,18 @@ public void CleanStorageBetweenTests() BinaryStorage.Delete(StoragePath); } + protected BinaryStorage Open(bool autoSave = false, bool saveOnBackgroundThread = true) + { + var builder = BinaryStorage.Construct(StoragePath) + .AddPrimitiveTypes() + .SaveOnBackgroundThread(saveOnBackgroundThread); + if (autoSave) + { + builder = builder.EnableAutoSaveOnChange(); + } + return builder.Build(); + } + /// Serializes and publishes the given records on the calling thread, the way a storage without a background writer does. internal static void SaveOnDisk(string filePath, IReadOnlyList sections, Dictionary data) { diff --git a/src/Tests/CollectionTests/TestCollectionExtensions.cs b/src/Tests/CollectionTests/TestCollectionExtensions.cs new file mode 100644 index 0000000..3393c90 --- /dev/null +++ b/src/Tests/CollectionTests/TestCollectionExtensions.cs @@ -0,0 +1,23 @@ +using System.Collections.Generic; + +namespace Appegy.Storage +{ + internal static class TestCollectionExtensions + { + public static void AddRange(this ICollection source, params T[] items) + { + foreach (var item in items) + { + source.Add(item); + } + } + + public static void AddRange(this IDictionary source, params (TKey Key, TValue Value)[] items) + { + foreach (var item in items) + { + source.Add(item.Key, item.Value); + } + } + } +} diff --git a/src/Tests/CollectionTests/TestCollectionExtensions.cs.meta b/src/Tests/CollectionTests/TestCollectionExtensions.cs.meta new file mode 100644 index 0000000..c8823d2 --- /dev/null +++ b/src/Tests/CollectionTests/TestCollectionExtensions.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 804dfad480d1ee24eb69f99a12c66cd8 \ No newline at end of file diff --git a/src/Tests/Persistence/BackgroundWriterTests.cs b/src/Tests/Persistence/BackgroundWriterTests.cs index bdcdc4c..b1cc2eb 100644 --- a/src/Tests/Persistence/BackgroundWriterTests.cs +++ b/src/Tests/Persistence/BackgroundWriterTests.cs @@ -46,6 +46,20 @@ public void WhenDisposedWithPendingChanges_ThenTheyReachDisk() ReadValueFromDisk().Should().Be(7); } + [Test] + public void WhenManyChangesQueuedAndDisposedWithoutSave_ThenDiskHoldsTheLastState() + { + using (var storage = Open(autoSave: true)) + { + for (var i = 1; i <= 200; i++) + { + storage.Set("value", i); + } + } + + ReadValueFromDisk().Should().Be(200); + } + [Test] public void WhenStorageEmptied_ThenFilesAreRemoved() { @@ -70,11 +84,7 @@ public void WhenStorageEmptied_ThenFilesAreRemoved() [Test] public void WhenBackgroundWriterDisabled_ThenAutoSaveWritesBeforeSetReturns() { - using var storage = BinaryStorage.Construct(StoragePath) - .AddPrimitiveTypes() - .EnableAutoSaveOnChange() - .SaveOnBackgroundThread(false) - .Build(); + using var storage = Open(autoSave: true, saveOnBackgroundThread: false); storage.Set("value", 11); @@ -111,16 +121,6 @@ public void WhenReopenedAfterBackgroundSave_ThenDataSurvives() reopened.Get("text").Should().Be("kept"); } - private BinaryStorage Open(bool autoSave = false) - { - var builder = BinaryStorage.Construct(StoragePath).AddPrimitiveTypes(); - if (autoSave) - { - builder = builder.EnableAutoSaveOnChange(); - } - return builder.Build(); - } - private int ReadValueFromDisk() { return ReadValueFrom(StoragePath); diff --git a/src/Tests/Persistence/BackupRecoveryTests.cs b/src/Tests/Persistence/BackupRecoveryTests.cs index ed1bea7..6a93c2d 100644 --- a/src/Tests/Persistence/BackupRecoveryTests.cs +++ b/src/Tests/Persistence/BackupRecoveryTests.cs @@ -113,10 +113,5 @@ private void WriteTwoGenerations() storage.Set("generation", 2); storage.Save(); } - - private BinaryStorage Open() - { - return BinaryStorage.Construct(StoragePath).AddPrimitiveTypes().Build(); - } } }