From 61db0edc0dcac6b5ac532294c070f4184f3c4cd8 Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Wed, 12 Jun 2024 14:14:39 +0200 Subject: [PATCH] RNET-1154, RNET-1151, RNET-1139: Compact-related fixes (#3618) --- CHANGELOG.md | 7 +- .../Configurations/RealmConfiguration.cs | 21 ---- .../Configurations/RealmConfigurationBase.cs | 21 ++++ Realm/Realm/Handles/SharedRealmHandle.cs | 9 +- Tests/Realm.Tests/Database/InstanceTests.cs | 107 +++++++----------- .../Sync/SynchronizedInstanceTests.cs | 78 +++++++++---- wrappers/realm-core | 2 +- 7 files changed, 126 insertions(+), 119 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f74c98e53..e190dc9c9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,16 +1,17 @@ ## vNext (TBD) ### Enhancements -* None +* Allow `ShouldCompactOnLaunch` to be set on `SyncConfiguration`, not only `RealmConfiguration`. (Issue [#3617](https://github.com/realm/realm-dotnet/issues/3617)) ### Fixed -* None +* 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) ### Compatibility * Realm Studio: 15.0.0 or later. ### Internal -* Using Core x.y.z. +* Using Core 14.9.0. ## 12.2.0 (2024-05-22) diff --git a/Realm/Realm/Configurations/RealmConfiguration.cs b/Realm/Realm/Configurations/RealmConfiguration.cs index b9ea5ab0d3..cb88d9f5ed 100644 --- a/Realm/Realm/Configurations/RealmConfiguration.cs +++ b/Realm/Realm/Configurations/RealmConfiguration.cs @@ -49,17 +49,6 @@ public class RealmConfiguration : RealmConfigurationBase /// public delegate void MigrationCallbackDelegate(Migration migration, ulong oldSchemaVersion); - /// - /// 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. - /// - /// Total file size (data + free space). - /// Total data size. - /// true to indicate that an attempt to compact the file should be made. - /// The compaction will be skipped if another process is accessing it. - public delegate bool ShouldCompactDelegate(ulong totalBytes, ulong bytesUsed); - /// /// Gets or sets a value indicating whether the database will be deleted if the /// mismatches the one in the code. Use this when debugging and developing your app but never release it with @@ -84,15 +73,6 @@ public class RealmConfiguration : RealmConfigurationBase /// public MigrationCallbackDelegate? MigrationCallback { get; set; } - /// - /// Gets or sets the compact on launch callback. - /// - /// - /// The 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. - /// - public ShouldCompactDelegate? ShouldCompactOnLaunch { get; set; } - /// /// Gets or sets the key, used to encrypt the entire Realm. Once set, must be specified each time the file is used. /// @@ -150,7 +130,6 @@ 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; diff --git a/Realm/Realm/Configurations/RealmConfigurationBase.cs b/Realm/Realm/Configurations/RealmConfigurationBase.cs index 5f3c6c91a5..5fc1de87af 100644 --- a/Realm/Realm/Configurations/RealmConfigurationBase.cs +++ b/Realm/Realm/Configurations/RealmConfigurationBase.cs @@ -39,6 +39,17 @@ public abstract class RealmConfigurationBase internal delegate void InitialDataDelegate(Realm realm); + /// + /// 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. + /// + /// Total file size (data + free space). + /// Total data size. + /// true to indicate that an attempt to compact the file should be made. + /// The compaction will be skipped if another process is accessing it. + public delegate bool ShouldCompactDelegate(ulong totalBytes, ulong bytesUsed); + /// /// Gets the filename to be combined with the platform-specific document directory. /// @@ -69,6 +80,15 @@ public abstract class RealmConfigurationBase /// true if the Realm will be opened in dynamic mode; false otherwise. public bool IsDynamic { get; set; } + /// + /// Gets or sets the compact on launch callback. + /// + /// + /// The 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. + /// + public ShouldCompactDelegate? ShouldCompactOnLaunch { get; set; } + internal bool EnableCache = true; /// @@ -233,6 +253,7 @@ internal virtual Configuration CreateNativeConfiguration(Arena arena) invoke_initial_data_callback = PopulateInitialData != null, managed_config = GCHandle.ToIntPtr(managedConfig), encryption_key = MarshaledVector.AllocateFrom(EncryptionKey, arena), + invoke_should_compact_callback = ShouldCompactOnLaunch != null, }; return config; diff --git a/Realm/Realm/Handles/SharedRealmHandle.cs b/Realm/Realm/Handles/SharedRealmHandle.cs index ff223c3e46..9b438f2247 100644 --- a/Realm/Realm/Handles/SharedRealmHandle.cs +++ b/Realm/Realm/Handles/SharedRealmHandle.cs @@ -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 cause they never get a reference in the first place + // except of course dot notation users that cannot dispose because they never get a reference in the first place lock (_unbindListLock) { UnbindLockedList(); @@ -608,8 +608,7 @@ public void WriteCopy(RealmConfigurationBase config) public RealmSchema GetSchema() { RealmSchema? result = null; - Action callback = schema => result = RealmSchema.CreateFromObjectStoreSchema(schema); - var callbackHandle = GCHandle.Alloc(callback); + var callbackHandle = GCHandle.Alloc((Action)SchemaCallback); try { NativeMethods.get_schema(this, GCHandle.ToIntPtr(callbackHandle), out var nativeException); @@ -621,6 +620,8 @@ public RealmSchema GetSchema() } return result!; + + void SchemaCallback(Native.Schema schema) => result = RealmSchema.CreateFromObjectStoreSchema(schema); } public ObjectHandle CreateObject(TableKey tableKey) @@ -868,7 +869,7 @@ private static IntPtr ShouldCompactOnLaunchCallback(IntPtr managedConfigHandle, try { var configHandle = GCHandle.FromIntPtr(managedConfigHandle); - var config = (RealmConfiguration)configHandle.Target!; + var config = (RealmConfigurationBase)configHandle.Target!; shouldCompact = config.ShouldCompactOnLaunch!.Invoke(totalSize, dataSize); return IntPtr.Zero; diff --git a/Tests/Realm.Tests/Database/InstanceTests.cs b/Tests/Realm.Tests/Database/InstanceTests.cs index 91b956c96b..e920e1c9e7 100644 --- a/Tests/Realm.Tests/Database/InstanceTests.cs +++ b/Tests/Realm.Tests/Database/InstanceTests.cs @@ -321,50 +321,6 @@ 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().Count(), Is.EqualTo(DummyDataSize / 2)); - } - } - [TestCase(false, true)] [TestCase(false, false)] [TestCase(true, true)] @@ -454,7 +410,7 @@ public void Compact_WhenResultsAreOpen_ShouldReturnFalse() { using var realm = GetRealm(); - var token = realm.All().SubscribeForNotifications((sender, changes) => + var token = realm.All().SubscribeForNotifications((_, changes) => { Console.WriteLine(changes?.InsertedIndices); }); @@ -463,6 +419,32 @@ 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().Count(), Is.EqualTo(1)); + Assert.That(realm.All().Single().FirstName, Is.EqualTo("Peter")); + } + } + [Test] public void RealmChangedShouldFireForEveryInstance() { @@ -472,13 +454,13 @@ public void RealmChangedShouldFireForEveryInstance() using var realm2 = GetRealm(); var changed1 = 0; - realm1.RealmChanged += (sender, e) => + realm1.RealmChanged += (_, _) => { changed1++; }; var changed2 = 0; - realm2.RealmChanged += (sender, e) => + realm2.RealmChanged += (_, _) => { changed2++; }; @@ -628,7 +610,7 @@ public void GetInstanceAsync_ExecutesMigrationsInBackground() var threadId = Environment.CurrentManagedThreadId; var hasCompletedMigration = false; config.SchemaVersion = 2; - config.MigrationCallback = (migration, oldSchemaVersion) => + config.MigrationCallback = (migration, _) => { Assert.That(Environment.CurrentManagedThreadId, Is.Not.EqualTo(threadId)); Task.Delay(300).Wait(); @@ -914,8 +896,7 @@ public void FrozenRealm_CannotSubscribeForNotifications() using var realm = GetRealm(); using var frozenRealm = realm.Freeze(); - Assert.Throws(() => frozenRealm.RealmChanged += (_, __) => { }); - Assert.Throws(() => frozenRealm.RealmChanged -= (_, __) => { }); + Assert.Throws(() => frozenRealm.RealmChanged += (_, _) => { }); } [Test] @@ -1046,7 +1027,7 @@ await TestHelpers.EnsureObjectsAreCollected(() => using var realm = Realm.GetInstance(); var state = stateAccessor.GetValue(realm)!; - return new object[] { state }; + return new[] { state }; }); }); } @@ -1093,7 +1074,7 @@ public void GetInstance_WithManualSchema_CanReadAndWrite() { Schema = new RealmSchema.Builder { - new ObjectSchema.Builder("MyType", ObjectSchema.ObjectType.RealmObject) + new ObjectSchema.Builder("MyType") { Property.Primitive("IntValue", RealmValueType.Int), Property.PrimitiveList("ListValue", RealmValueType.Date), @@ -1104,7 +1085,7 @@ public void GetInstance_WithManualSchema_CanReadAndWrite() Property.ObjectSet("ObjectSetValue", "OtherObject"), Property.ObjectDictionary("ObjectDictionaryValue", "OtherObject"), }, - new ObjectSchema.Builder("OtherObject", ObjectSchema.ObjectType.RealmObject) + new ObjectSchema.Builder("OtherObject") { Property.Primitive("Id", RealmValueType.String, isPrimaryKey: true), Property.Backlinks("MyTypes", "MyType", "ObjectValue") @@ -1230,13 +1211,10 @@ public void GetInstance_WithTypedSchemaWithMissingProperties_ThrowsException() using var realm = GetRealm(config); - var person = realm.Write(() => + var person = realm.Write(() => realm.Add(new Person { - return realm.Add(new Person - { - LastName = "Smith" - }); - }); + LastName = "Smith" + })); var exGet = Assert.Throws(() => _ = person.FirstName)!; Assert.That(exGet.Message, Does.Contain(nameof(Person))); @@ -1255,13 +1233,10 @@ public void RealmWithFrozenObjects_WhenDeleted_DoesNotThrow() { var config = new RealmConfiguration(Guid.NewGuid().ToString()); var realm = GetRealm(config); - var frozenObj = realm.Write(() => + var frozenObj = realm.Write(() => realm.Add(new IntPropertyObject { - return realm.Add(new IntPropertyObject - { - Int = 1 - }).Freeze(); - }); + Int = 1 + }).Freeze()); frozenObj.Realm!.Dispose(); realm.Dispose(); @@ -1275,7 +1250,7 @@ public void RealmWithFrozenObjects_WhenDeleted_DoesNotThrow() public void BeginWrite_CalledMultipleTimes_Throws() { using var realm = GetRealm(); - var ts = realm.BeginWrite(); + using var ts = realm.BeginWrite(); Assert.That(() => realm.BeginWrite(), Throws.TypeOf()); } diff --git a/Tests/Realm.Tests/Sync/SynchronizedInstanceTests.cs b/Tests/Realm.Tests/Sync/SynchronizedInstanceTests.cs index 7daf37f9b9..00fededf10 100644 --- a/Tests/Realm.Tests/Sync/SynchronizedInstanceTests.cs +++ b/Tests/Realm.Tests/Sync/SynchronizedInstanceTests.cs @@ -30,7 +30,6 @@ using Realms.Sync; using Realms.Sync.ErrorHandling; using Realms.Sync.Exceptions; -using NUnitExplicit = NUnit.Framework.ExplicitAttribute; namespace Realms.Tests.Sync { @@ -77,6 +76,49 @@ public void Compact_ShouldReduceSize([Values(true, false)] bool encrypt, [Values }); } + [Test] + public void ShouldCompact_IsInvokedAfterOpening([Values(true, false)] bool shouldCompact, [Values(true, false)] bool useSync) + { + RealmConfigurationBase config = useSync ? GetFakeConfig() : new RealmConfiguration(Guid.NewGuid().ToString()); + + using (var realm = GetRealm(config)) + { + AddDummyData(realm, singleTransaction: false); + } + + 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().Count(), Is.EqualTo(DummyDataSize / 2)); + } + } + [Test] public void GetInstanceAsync_ShouldDownloadRealm([Values(true, false)] bool singleTransaction) { @@ -176,10 +218,7 @@ public void GetInstanceAsync_WithOnProgressThrowing_ReportsErrorToLogs() Logger.Default = logger; config = await GetIntegrationConfigAsync((string?)config.Partition); - config.OnProgress = (progress) => - { - throw new Exception("Exception in OnProgress"); - }; + config.OnProgress = _ => throw new Exception("Exception in OnProgress"); var realmTask = GetRealmAsync(config); config.OnProgress = null; @@ -357,13 +396,10 @@ public void WriteCopy_CanSynchronizeData([Values(true, false)] bool originalEncr Assert.That(copiedRealm.All().Count(), Is.EqualTo(originalRealm.All().Count())); - var fromCopy = copiedRealm.Write(() => + var fromCopy = copiedRealm.Write(() => copiedRealm.Add(new ObjectIdPrimaryKeyWithValueObject { - return copiedRealm.Add(new ObjectIdPrimaryKeyWithValueObject - { - StringValue = "Added from copy" - }); - }); + StringValue = "Added from copy" + })); await WaitForUploadAsync(copiedRealm); await WaitForDownloadAsync(originalRealm); @@ -372,13 +408,10 @@ public void WriteCopy_CanSynchronizeData([Values(true, false)] bool originalEncr Assert.That(itemInOriginal, Is.Not.Null); Assert.That(itemInOriginal!.StringValue, Is.EqualTo(fromCopy.StringValue)); - var fromOriginal = originalRealm.Write(() => + var fromOriginal = originalRealm.Write(() => originalRealm.Add(new ObjectIdPrimaryKeyWithValueObject { - return originalRealm.Add(new ObjectIdPrimaryKeyWithValueObject - { - StringValue = "Added from original" - }); - }); + StringValue = "Added from original" + })); await WaitForUploadAsync(originalRealm); await WaitForDownloadAsync(copiedRealm); @@ -441,13 +474,10 @@ public void WriteCopy_LocalToSync([Values(true, false)] bool originalEncrypted, Assert.That(anotherUserRealm.All().Count(), Is.EqualTo(addedObjects)); - var addedObject = anotherUserRealm.Write(() => + var addedObject = anotherUserRealm.Write(() => anotherUserRealm.Add(new ObjectIdPrimaryKeyWithValueObject { - return anotherUserRealm.Add(new ObjectIdPrimaryKeyWithValueObject - { - StringValue = "abc" - }); - }); + StringValue = "abc" + })); await WaitForUploadAsync(anotherUserRealm); await WaitForDownloadAsync(copiedRealm); @@ -534,7 +564,7 @@ public void RemoveAll_RemovesAllElements([Values(true, false)] bool originalEncr realmConfig.EncryptionKey = TestHelpers.GetEncryptionKey(42); } - using var realm = GetRealm(realmConfig); + var realm = GetRealm(realmConfig); AddDummyData(realm, true); diff --git a/wrappers/realm-core b/wrappers/realm-core index 14349903d1..f3d7ae5f9f 160000 --- a/wrappers/realm-core +++ b/wrappers/realm-core @@ -1 +1 @@ -Subproject commit 14349903d1315e13758537a735a649bd1c2d2fec +Subproject commit f3d7ae5f9f31d90b327a64536bb7801cc69fd85b