Skip to content
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

Fix the exception message for refreshing an immutable Realm #5065

Merged
merged 1 commit into from
Nov 24, 2021
Merged

Fix the exception message for refreshing an immutable Realm #5065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* The client reset callbacks now pass out SharedRealm objects instead of TransactionRefs. ([#5048](https://github.com/realm/realm-core/issues/5048), since v11.5.0)
* A client reset in DiscardLocal mode would revert to Manual mode on the next restart of the session. ([#5050](https://github.com/realm/realm-core/issues/5050), since v11.5.0)
* `@sum` and `@avg` queries on Dictionaries of floats or doubles used too much precision for intermediates, resulting in incorrect rounding (since v10.2.0).
* Change the exception message for calling refresh on an immutable Realm from "Continuous transaction through DB object without history information." to "Can't refresh a read-only Realm." ([#5061](https://github.com/realm/realm-core/issues/5061), old exception message was since 10.7.2 via https://github.com/realm/realm-core/pull/4688).

### Breaking changes
* None.
Expand Down
4 changes: 4 additions & 0 deletions src/realm/object-store/shared_realm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,10 @@ bool Realm::do_refresh()
return false;
}

if (m_config.immutable()) {
throw std::logic_error("Can't refresh a read-only Realm.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we instead return false like we do for frozen Realms?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cocoa throws an exception if the user explicitly calls refresh on an immutable Realm because it's almost certainly a sign that the developer is trying to do something that won't/can't work and we want to let them know. It makes it awkward if they write a function which accepts an arbitrary Realm and they want to call refresh() on it, but that's a terrible idea so making it awkward isn't necessarily a bad thing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, but isn't it the case for frozen realms as well? I guess in my mind those two are very similar and it just triggers my OCD a little that we have different behavior here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing calling refresh() on frozen Realms does seem like a mistake to me.

}

// can't be any new changes if we're in a write transaction
if (is_in_transaction()) {
return false;
Expand Down
4 changes: 4 additions & 0 deletions test/object-store/realm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,10 @@ TEST_CASE("SharedRealm: get_shared_realm()") {
REQUIRE(it->persisted_properties.size() == 1);
REQUIRE(it->persisted_properties[0].name == "value");
REQUIRE(it->persisted_properties[0].column_key == table->get_column_key("value"));

SECTION("refreshing an immutable Realm throws") {
REQUIRE_THROWS_WITH(realm->refresh(), "Can't refresh a read-only Realm.");
}
}

SECTION("should support using different table subsets on different threads") {
Expand Down