Skip to content

Commit

Permalink
Update captured_transferrable when downloadable bytes value changes.
Browse files Browse the repository at this point in the history
  • Loading branch information
@leemaguire
leemaguire committed Oct 25, 2021
1 parent 053c22a commit 962e4d5
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* Calling `size()` on a Results newly constructed via `.as_results().distinct()` on a Collection would give the size of the Collection rather than the distinct count. ([Cocoa #7481](https://github.com/realm/realm-cocoa/issues/7481), since v11.0.0).
* Calling `clear()` on a Results newly constructed via `.as_results().distinct()` on a Collection would delete all objects in the Collection rather than just the distinct objects in the Results (since v11.0.0).
* Calling `clear()` on a Results constructed via `.as_results().distinct()` on a Collection after calling `get()` or `size()` would not re-evaluate the distinct until after the next mutation to the table occurred.
* Sync progress notifiers would report the incorrect `downloadable` bytes size if server side compaction was run during a sync session.

### Breaking changes
* `App::Config::transport_factory` was replaced with `App::Config::transport`. It should now be an instance of `GenericNetworkTransport` rather than a factory for making instances. This allows the SDK to control which thread constructs the transport layer. ([#4903](https://github.com/realm/realm-core/pull/4903))
Expand Down
9 changes: 9 additions & 0 deletions src/realm/object-store/sync/sync_session.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,15 @@ std::function<void()> SyncProgressNotifier::NotifierPackage::create_invocation(P
transferrable = is_download ? current_progress.downloadable : current_progress.uploadable;
}
else if (captured_transferrable) {
// We need to account for when server side compaction runs. This will change the
// value of `current_progress.downloadable` so any math that checks sync progress will
// then be invalid because of the old value. The server will also send a value of `0` when the sync progress
// has complete but we should not set 0 to `transferrable` as the SDK's will end up doing a
// divide by 0 when doing sync progress math.
if (is_download && (*captured_transferrable != current_progress.downloadable) &&
(current_progress.downloadable != 0)) {
captured_transferrable = current_progress.downloadable;
}
transferrable = *captured_transferrable;
}
else {
Expand Down
34 changes: 16 additions & 18 deletions test/object-store/sync/session/progress_notifications.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,21 +351,21 @@ TEST_CASE("progress notification", "[sync]") {
progress.update(current_transferred, current_transferrable, 25, 26, 1, 1);
CHECK(callback_was_called);
CHECK(transferred == current_transferred);
CHECK(transferrable == original_transferrable);
CHECK(transferrable == current_transferrable);

// Second callback
callback_was_called = false;
current_transferred = original_transferrable + 100;
current_transferrable = 1021;
current_transferrable = current_transferred;
progress.update(current_transferred, current_transferrable, 68, 191, 1, 1);
CHECK(callback_was_called);
CHECK(transferred == current_transferred);
CHECK(transferrable == original_transferrable);
CHECK(transferrable == current_transferrable);

// The notifier should be unregistered at this point, and not fire.
callback_was_called = false;
current_transferred = original_transferrable + 250;
current_transferrable = 1228;
current_transferrable = current_transferred;
progress.update(current_transferred, current_transferrable, 199, 591, 1, 1);
CHECK(!callback_was_called);
}
Expand Down Expand Up @@ -461,7 +461,6 @@ TEST_CASE("progress notification", "[sync]") {
uint64_t current_downloaded = 68;
uint64_t current_downloadable = 182;
const uint64_t original_uploadable = current_uploadable;
const uint64_t original_downloadable = current_downloadable;
progress.update(current_downloaded, current_downloadable, current_uploaded, current_uploadable, 1, 1);

progress.register_callback(
Expand Down Expand Up @@ -499,7 +498,7 @@ TEST_CASE("progress notification", "[sync]") {
CHECK(transferrable == original_uploadable);
CHECK(callback_was_called_2);
CHECK(downloaded == current_downloaded);
CHECK(downloadable == original_downloadable);
CHECK(downloadable == current_downloadable);

// Second callback, last one for the upload notifier
callback_was_called = false;
Expand All @@ -514,27 +513,27 @@ TEST_CASE("progress notification", "[sync]") {
CHECK(transferrable == original_uploadable);
CHECK(callback_was_called_2);
CHECK(downloaded == current_downloaded);
CHECK(downloadable == original_downloadable);
CHECK(downloadable == current_downloadable);

// Third callback, last one for the download notifier
callback_was_called = false;
callback_was_called_2 = false;
current_uploaded = 218;
current_uploadable = 310;
current_downloaded = 182;
current_downloadable = 196;
current_downloadable = current_downloaded;
progress.update(current_downloaded, current_downloadable, current_uploaded, current_uploadable, 1, 1);
CHECK(!callback_was_called);
CHECK(callback_was_called_2);
CHECK(downloaded == current_downloaded);
CHECK(downloadable == original_downloadable);
CHECK(downloadable == current_downloadable);

// Fourth callback, last one for the download notifier
callback_was_called_2 = false;
current_uploaded = 220;
current_uploadable = 410;
current_downloaded = 192;
current_downloadable = 591;
current_downloadable = current_downloaded;
progress.update(current_downloaded, current_downloadable, current_uploaded, current_uploadable, 1, 1);
CHECK(!callback_was_called);
CHECK(!callback_was_called_2);
Expand All @@ -546,7 +545,6 @@ TEST_CASE("progress notification", "[sync]") {
uint64_t current_uploadable = 201;
uint64_t current_downloaded = 68;
uint64_t current_downloadable = 182;
const uint64_t original_downloadable = current_downloadable;
progress.update(current_downloaded, current_downloadable, current_uploaded, current_uploadable, 1, 1);

progress.register_callback(
Expand All @@ -567,13 +565,12 @@ TEST_CASE("progress notification", "[sync]") {
progress.update(current_downloaded, current_downloadable, current_uploaded, current_uploadable, 1, 1);
CHECK(callback_was_called);
CHECK(transferred == current_downloaded);
CHECK(transferrable == original_downloadable);
CHECK(transferrable == current_downloadable);

// Register a second notifier.
bool callback_was_called_2 = false;
uint64_t downloaded = 0;
uint64_t downloadable = 0;
const uint64_t original_downloadable_2 = current_downloadable;
progress.register_callback(
[&](auto xferred, auto xferable) {
downloaded = xferred;
Expand All @@ -594,31 +591,32 @@ TEST_CASE("progress notification", "[sync]") {
progress.update(current_downloaded, current_downloadable, current_uploaded, current_uploadable, 1, 1);
CHECK(callback_was_called);
CHECK(transferred == current_downloaded);
CHECK(transferrable == original_downloadable);
CHECK(transferrable == current_downloadable);
CHECK(callback_was_called_2);
CHECK(downloaded == current_downloaded);
CHECK(downloadable == original_downloadable_2);
CHECK(downloadable == current_downloadable);

// Third callback, last one for second notifier
callback_was_called = false;
callback_was_called_2 = false;
current_uploaded = 36;
current_uploadable = 310;
current_downloaded = 189;
current_downloadable = 250;
current_downloadable = current_downloaded;
progress.update(current_downloaded, current_downloadable, current_uploaded, current_uploadable, 1, 1);
CHECK(!callback_was_called);
CHECK(callback_was_called_2);
CHECK(downloaded == current_downloaded);
CHECK(downloadable == original_downloadable_2);
CHECK(downloadable == current_downloadable);

// Fourth callback
callback_was_called = false;
callback_was_called_2 = false;
current_uploaded = 36;
current_uploadable = 310;
current_downloaded = 201;
current_downloadable = 289;
progress.update(current_downloaded, current_downloadable, current_uploaded, current_uploadable, 1, 1);
CHECK(!callback_was_called);
CHECK(!callback_was_called_2);
}
}
Expand Down

0 comments on commit 962e4d5

Please sign in to comment.