Skip to content

refactor: remove duplicated logic from storage internals #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
imurashka wants to merge 5 commits into
base: main
Choose a base branch
from
Open

refactor: remove duplicated logic from storage internals #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
00eff4e
refactor: collapse duplicated record and section handling in BinarySt…
imurashka Aug 10, 2026
7ecf8ed
refactor: give reactive collections a shared base
imurashka Aug 10, 2026
b1cbcd9
refactor: keep the auto-save decision in one place
imurashka Aug 10, 2026
0aa1dde
test: cover a burst of changes that reaches disk on dispose
imurashka Aug 11, 2026
b19b5cd
refactor: pass the prefix check directly instead of caching a delegate
imurashka Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 80 additions & 89 deletions src/Runtime/BinaryStorage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ public partial class BinaryStorage : IDisposable, IBinaryStorage
private readonly string _storageFilePath;
private readonly StoragePersistence _persistence;
private readonly IReadOnlyList<BinarySection> _supportedTypes;
private readonly Dictionary<Type, int> _sectionIndexByType;
private readonly Dictionary<string, Record> _data = new();
private readonly Dictionary<IReactiveCollection, string> _collections = new();
private readonly Action _decreaseCounter;
private int _changeScopeCounter;
private bool _hasUnsavedChanges;

Expand Down Expand Up @@ -45,6 +47,12 @@ internal BinaryStorage(string storageFilePath, IReadOnlyList<BinarySection> supp
{
_storageFilePath = storageFilePath;
_supportedTypes = supportedTypes;
_sectionIndexByType = new Dictionary<Type, int>(supportedTypes.Count);
for (var i = 0; i < supportedTypes.Count; i++)
{
_sectionIndexByType[supportedTypes[i].Type] = i;
}
_decreaseCounter = DecreaseCounter;
_persistence = new StoragePersistence(storageFilePath, supportedTypes, saveOnBackgroundThread);
}

Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
}

/// <summary> Determines whether the specified key exists in the storage. </summary>
Expand Down Expand Up @@ -236,23 +238,17 @@ public virtual bool Set<T>(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;
}

/// <summary>
Expand Down Expand Up @@ -320,7 +316,7 @@ public IDisposable MultipleChangeScope()
{
ThrowIfDisposed();
_changeScopeCounter++;
return new DisposableScope(DecreaseCounter);
return new DisposableScope(_decreaseCounter);
}

#region Collections
Expand Down Expand Up @@ -448,12 +444,7 @@ private Record<T> AddRecord<T>(string key, T value)
var record = new Record<T>(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;
Expand All @@ -466,15 +457,7 @@ private Record<T> AddRecord<T>(string key, T value)
/// <exception cref="UnregisteredTypeException">Thrown if the type is not registered.</exception>
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);
Expand Down Expand Up @@ -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();
Expand All @@ -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++)
Expand All @@ -573,14 +543,50 @@ private Record GetRecord(string key)

private int IndexOfSection<T>()
{
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;
}

/// <summary> Decides whether a record whose stored type differs from the type being written has to be replaced. </summary>
/// <returns>True if the record has to be replaced; false if the write has to be ignored.</returns>
/// <exception cref="UnexpectedTypeException">Thrown if the mismatch behavior is to throw.</exception>
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<T>)
{
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);
}

/// <summary>
Expand All @@ -594,14 +600,11 @@ private void DecreaseCounter()
return;
}
_changeScopeCounter--;
if (IsDisposed)
if (IsDisposed || !_hasUnsavedChanges)
{
return;
}
if (_changeScopeCounter == 0 && _hasUnsavedChanges && AutoSave)
{
SaveDataOnDisk(false);
}
MarkChanged();
}

/// <summary> Reacts to a change in a reactive collection. </summary>
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
38 changes: 38 additions & 0 deletions src/Runtime/Collections/ReactiveCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
using System;

namespace Appegy.Storage
{
internal abstract class ReactiveCollection : IReactiveCollection
{
public bool IsDisposed { get; private set; }

public event Action<IReactiveCollection> 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);
}
}
}
}
2 changes: 2 additions & 0 deletions src/Runtime/Collections/ReactiveCollection.cs.meta
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 4 additions & 30 deletions src/Runtime/Collections/ReactiveDictionary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,16 @@
using System;
using System.Collections;
using System.Collections;
using System.Collections.Generic;

namespace Appegy.Storage
{
internal class ReactiveDictionary<TKey, TValue> : IReactiveCollection, IDictionary<TKey, TValue>, IReadOnlyDictionary<TKey, TValue>
internal class ReactiveDictionary<TKey, TValue> : ReactiveCollection, IDictionary<TKey, TValue>, IReadOnlyDictionary<TKey, TValue>
{
private readonly Dictionary<TKey, TValue> _dictionary = new();

public bool IsDisposed { get; private set; }

public event Action<IReactiveCollection> OnChanged;

private void SetDirty()
{
OnChanged?.Invoke(this);
}

private void ThrowIfDisposed()
{
if (IsDisposed)
{
throw new ObjectDisposedException(nameof(ReactiveDictionary<TKey, TValue>));
}
}
protected override string ObjectName => nameof(ReactiveDictionary<TKey, TValue>);

#region Mutable functionallity

public void Dispose()
{
if (IsDisposed)
{
return;
}
Clear();
IsDisposed = true;
}

public TValue this[TKey key]
{
get
Expand All @@ -59,7 +33,7 @@ public void Add(KeyValuePair<TKey, TValue> item)
SetDirty();
}

public void Clear()
public override void Clear()
{
ThrowIfDisposed();
_dictionary.Clear();
Expand Down
Loading
Loading