Skip to content

Commit

Permalink
Revert "Compact-related fixes"
Browse files Browse the repository at this point in the history
This reverts commit c06a453.
  • Loading branch information
nirinchev committed Jun 6, 2024
1 parent c06a453 commit e61576e
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 126 deletions.
7 changes: 3 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
## vNext (TBD)

### Enhancements
* Allow `ShouldCompactOnLaunch` to be set on `SyncConfiguration`, not only `RealmConfiguration`. (Issue [#3617](https://github.com/realm/realm-dotnet/issues/3617))
* None

### Fixed
* A `ForCurrentlyOutstandingWork` progress notifier would not immediately call its callback after registration. Instead you would have to wait for some data to be received to get your first update - if you were already caught up when you registered the notifier you could end up waiting a long time for the server to deliver a download that would call/expire your notifier. (Core 14.8.0)
* After compacting, a file upgrade would be triggered. This could cause loss of data if `ShouldDeleteIfMigrationNeeded` is set to `true`. (Issue [#3583](https://github.com/realm/realm-dotnet/issues/3583), Core 14.9.0)
* None

### Compatibility
* Realm Studio: 15.0.0 or later.

### Internal
* Using Core 14.9.0.
* Using Core x.y.z.

## 12.2.0 (2024-05-22)

Expand Down
21 changes: 21 additions & 0 deletions Realm/Realm/Configurations/RealmConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ public class RealmConfiguration : RealmConfigurationBase
/// </param>
public delegate void MigrationCallbackDelegate(Migration migration, ulong oldSchemaVersion);

/// <summary>
/// A callback, invoked when opening a Realm for the first time during the life
/// of a process to determine if it should be compacted before being returned
/// to the user.
/// </summary>
/// <param name="totalBytes">Total file size (data + free space).</param>
/// <param name="bytesUsed">Total data size.</param>
/// <returns><c>true</c> to indicate that an attempt to compact the file should be made.</returns>
/// <remarks>The compaction will be skipped if another process is accessing it.</remarks>
public delegate bool ShouldCompactDelegate(ulong totalBytes, ulong bytesUsed);

/// <summary>
/// Gets or sets a value indicating whether the database will be deleted if the <see cref="RealmSchema"/>
/// mismatches the one in the code. Use this when debugging and developing your app but never release it with
Expand All @@ -73,6 +84,15 @@ public class RealmConfiguration : RealmConfigurationBase
/// </value>
public MigrationCallbackDelegate? MigrationCallback { get; set; }

/// <summary>
/// Gets or sets the compact on launch callback.
/// </summary>
/// <value>
/// The <see cref="ShouldCompactDelegate"/> that will be invoked when opening a Realm for the first time
/// to determine if it should be compacted before being returned to the user.
/// </value>
public ShouldCompactDelegate? ShouldCompactOnLaunch { get; set; }

/// <summary>
/// Gets or sets the key, used to encrypt the entire Realm. Once set, must be specified each time the file is used.
/// </summary>
Expand Down Expand Up @@ -130,6 +150,7 @@ internal override Configuration CreateNativeConfiguration(Arena arena)
result.delete_if_migration_needed = ShouldDeleteIfMigrationNeeded;
result.read_only = IsReadOnly;
result.invoke_migration_callback = MigrationCallback != null;
result.invoke_should_compact_callback = ShouldCompactOnLaunch != null;
result.automatically_migrate_embedded = true;

return result;
Expand Down
21 changes: 0 additions & 21 deletions Realm/Realm/Configurations/RealmConfigurationBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,6 @@ public abstract class RealmConfigurationBase

internal delegate void InitialDataDelegate(Realm realm);

/// <summary>
/// A callback, invoked when opening a Realm for the first time during the life
/// of a process to determine if it should be compacted before being returned
/// to the user.
/// </summary>
/// <param name="totalBytes">Total file size (data + free space).</param>
/// <param name="bytesUsed">Total data size.</param>
/// <returns><c>true</c> to indicate that an attempt to compact the file should be made.</returns>
/// <remarks>The compaction will be skipped if another process is accessing it.</remarks>
public delegate bool ShouldCompactDelegate(ulong totalBytes, ulong bytesUsed);

/// <summary>
/// Gets the filename to be combined with the platform-specific document directory.
/// </summary>
Expand Down Expand Up @@ -80,15 +69,6 @@ public abstract class RealmConfigurationBase
/// <value><c>true</c> if the Realm will be opened in dynamic mode; <c>false</c> otherwise.</value>
public bool IsDynamic { get; set; }

/// <summary>
/// Gets or sets the compact on launch callback.
/// </summary>
/// <value>
/// The <see cref="ShouldCompactDelegate"/> that will be invoked when opening a Realm for the first time
/// to determine if it should be compacted before being returned to the user.
/// </value>
public ShouldCompactDelegate? ShouldCompactOnLaunch { get; set; }

internal bool EnableCache = true;

/// <summary>
Expand Down Expand Up @@ -253,7 +233,6 @@ internal virtual Configuration CreateNativeConfiguration(Arena arena)
invoke_initial_data_callback = PopulateInitialData != null,
managed_config = GCHandle.ToIntPtr(managedConfig),
encryption_key = MarshaledVector<byte>.AllocateFrom(EncryptionKey, arena),
invoke_should_compact_callback = ShouldCompactOnLaunch != null,
};

return config;
Expand Down
9 changes: 4 additions & 5 deletions Realm/Realm/Handles/SharedRealmHandle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ public virtual void AddChild(RealmHandle childHandle)
// if we get !=0 and the real value was in fact 0, then we will just skip and then catch up next time around.
// however, doing things this way will save lots and lots of locks when the list is empty, which it should be if people have
// been using the dispose pattern correctly, or at least have been eager at disposing as soon as they can
// except of course dot notation users that cannot dispose because they never get a reference in the first place
// except of course dot notation users that cannot dispose cause they never get a reference in the first place
lock (_unbindListLock)
{
UnbindLockedList();
Expand Down Expand Up @@ -608,7 +608,8 @@ public void WriteCopy(RealmConfigurationBase config)
public RealmSchema GetSchema()
{
RealmSchema? result = null;
var callbackHandle = GCHandle.Alloc((Action<Native.Schema>)SchemaCallback);
Action<Native.Schema> callback = schema => result = RealmSchema.CreateFromObjectStoreSchema(schema);
var callbackHandle = GCHandle.Alloc(callback);
try
{
NativeMethods.get_schema(this, GCHandle.ToIntPtr(callbackHandle), out var nativeException);
Expand All @@ -620,8 +621,6 @@ public RealmSchema GetSchema()
}

return result!;

void SchemaCallback(Native.Schema schema) => result = RealmSchema.CreateFromObjectStoreSchema(schema);
}

public ObjectHandle CreateObject(TableKey tableKey)
Expand Down Expand Up @@ -869,7 +868,7 @@ private static IntPtr ShouldCompactOnLaunchCallback(IntPtr managedConfigHandle,
try
{
var configHandle = GCHandle.FromIntPtr(managedConfigHandle);
var config = (RealmConfigurationBase)configHandle.Target!;
var config = (RealmConfiguration)configHandle.Target!;

shouldCompact = config.ShouldCompactOnLaunch!.Invoke(totalSize, dataSize);
return IntPtr.Zero;
Expand Down
107 changes: 66 additions & 41 deletions Tests/Realm.Tests/Database/InstanceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,50 @@ public void RealmObjectClassesOnlyAllowRealmObjects()
Assert.That(ex.Message, Does.Contain("must descend directly from either RealmObject, EmbeddedObject, or AsymmetricObject"));
}

[TestCase(true)]
[TestCase(false)]
public void ShouldCompact_IsInvokedAfterOpening(bool shouldCompact)
{
var config = (RealmConfiguration)RealmConfiguration.DefaultConfiguration;

using (var realm = GetRealm(config))
{
AddDummyData(realm);
}

var oldSize = new FileInfo(config.DatabasePath).Length;
long projectedNewSize = 0;
var hasPrompted = false;
config.ShouldCompactOnLaunch = (totalBytes, bytesUsed) =>
{
Assert.That(totalBytes, Is.EqualTo(oldSize));
hasPrompted = true;
projectedNewSize = (long)bytesUsed;
return shouldCompact;
};

using (var realm = GetRealm(config))
{
Assert.That(hasPrompted, Is.True);
var newSize = new FileInfo(config.DatabasePath).Length;
if (shouldCompact)
{
// Less than or equal because of the new online compaction mechanism - it's possible
// that the Realm was already at the optimal size.
Assert.That(newSize, Is.LessThanOrEqualTo(oldSize));

// Less than 20% error in projections
Assert.That((newSize - projectedNewSize) / newSize, Is.LessThan(0.2));
}
else
{
Assert.That(newSize, Is.EqualTo(oldSize));
}

Assert.That(realm.All<IntPrimaryKeyWithValueObject>().Count(), Is.EqualTo(DummyDataSize / 2));
}
}

[TestCase(false, true)]
[TestCase(false, false)]
[TestCase(true, true)]
Expand Down Expand Up @@ -410,7 +454,7 @@ public void Compact_WhenResultsAreOpen_ShouldReturnFalse()
{
using var realm = GetRealm();

var token = realm.All<Person>().SubscribeForNotifications((_, changes) =>
var token = realm.All<Person>().SubscribeForNotifications((sender, changes) =>
{
Console.WriteLine(changes?.InsertedIndices);
});
Expand All @@ -419,32 +463,6 @@ public void Compact_WhenResultsAreOpen_ShouldReturnFalse()
token.Dispose();
}

[Test]
public void Compact_WhenShouldDeleteIfMigrationNeeded_PreservesObjects()
{
var config = (RealmConfiguration)RealmConfiguration.DefaultConfiguration;
config.ShouldDeleteIfMigrationNeeded = true;

using (var realm = GetRealm(config))
{
realm.Write(() =>
{
realm.Add(new Person
{
FirstName = "Peter"
});
});
}

Assert.That(Realm.Compact(config), Is.True);

using (var realm = GetRealm(config))
{
Assert.That(realm.All<Person>().Count(), Is.EqualTo(1));
Assert.That(realm.All<Person>().Single().FirstName, Is.EqualTo("Peter"));
}
}

[Test]
public void RealmChangedShouldFireForEveryInstance()
{
Expand All @@ -454,13 +472,13 @@ public void RealmChangedShouldFireForEveryInstance()
using var realm2 = GetRealm();
var changed1 = 0;
realm1.RealmChanged += (_, _) =>
realm1.RealmChanged += (sender, e) =>
{
changed1++;
};
var changed2 = 0;
realm2.RealmChanged += (_, _) =>
realm2.RealmChanged += (sender, e) =>
{
changed2++;
};
Expand Down Expand Up @@ -610,7 +628,7 @@ public void GetInstanceAsync_ExecutesMigrationsInBackground()
var threadId = Environment.CurrentManagedThreadId;
var hasCompletedMigration = false;
config.SchemaVersion = 2;
config.MigrationCallback = (migration, _) =>
config.MigrationCallback = (migration, oldSchemaVersion) =>
{
Assert.That(Environment.CurrentManagedThreadId, Is.Not.EqualTo(threadId));
Task.Delay(300).Wait();
Expand Down Expand Up @@ -896,7 +914,8 @@ public void FrozenRealm_CannotSubscribeForNotifications()
using var realm = GetRealm();
using var frozenRealm = realm.Freeze();

Assert.Throws<RealmFrozenException>(() => frozenRealm.RealmChanged += (_, _) => { });
Assert.Throws<RealmFrozenException>(() => frozenRealm.RealmChanged += (_, __) => { });
Assert.Throws<RealmFrozenException>(() => frozenRealm.RealmChanged -= (_, __) => { });
}

[Test]
Expand Down Expand Up @@ -1027,7 +1046,7 @@ await TestHelpers.EnsureObjectsAreCollected(() =>
using var realm = Realm.GetInstance();
var state = stateAccessor.GetValue(realm)!;
return new[] { state };
return new object[] { state };
});
});
}
Expand Down Expand Up @@ -1074,7 +1093,7 @@ public void GetInstance_WithManualSchema_CanReadAndWrite()
{
Schema = new RealmSchema.Builder
{
new ObjectSchema.Builder("MyType")
new ObjectSchema.Builder("MyType", ObjectSchema.ObjectType.RealmObject)
{
Property.Primitive("IntValue", RealmValueType.Int),
Property.PrimitiveList("ListValue", RealmValueType.Date),
Expand All @@ -1085,7 +1104,7 @@ public void GetInstance_WithManualSchema_CanReadAndWrite()
Property.ObjectSet("ObjectSetValue", "OtherObject"),
Property.ObjectDictionary("ObjectDictionaryValue", "OtherObject"),
},
new ObjectSchema.Builder("OtherObject")
new ObjectSchema.Builder("OtherObject", ObjectSchema.ObjectType.RealmObject)
{
Property.Primitive("Id", RealmValueType.String, isPrimaryKey: true),
Property.Backlinks("MyTypes", "MyType", "ObjectValue")
Expand Down Expand Up @@ -1211,10 +1230,13 @@ public void GetInstance_WithTypedSchemaWithMissingProperties_ThrowsException()

using var realm = GetRealm(config);

var person = realm.Write(() => realm.Add(new Person
var person = realm.Write(() =>
{
LastName = "Smith"
}));
return realm.Add(new Person
{
LastName = "Smith"
});
});

var exGet = Assert.Throws<MissingMemberException>(() => _ = person.FirstName)!;
Assert.That(exGet.Message, Does.Contain(nameof(Person)));
Expand All @@ -1233,10 +1255,13 @@ public void RealmWithFrozenObjects_WhenDeleted_DoesNotThrow()
{
var config = new RealmConfiguration(Guid.NewGuid().ToString());
var realm = GetRealm(config);
var frozenObj = realm.Write(() => realm.Add(new IntPropertyObject
var frozenObj = realm.Write(() =>
{
Int = 1
}).Freeze());
return realm.Add(new IntPropertyObject
{
Int = 1
}).Freeze();
});

frozenObj.Realm!.Dispose();
realm.Dispose();
Expand All @@ -1250,7 +1275,7 @@ public void RealmWithFrozenObjects_WhenDeleted_DoesNotThrow()
public void BeginWrite_CalledMultipleTimes_Throws()
{
using var realm = GetRealm();
using var ts = realm.BeginWrite();
var ts = realm.BeginWrite();

Assert.That(() => realm.BeginWrite(), Throws.TypeOf<RealmInvalidTransactionException>());
}
Expand Down
Loading

0 comments on commit e61576e

Please sign in to comment.