Improve FLX/Subscriptions API to be more friendly to direct binding (#…

…6065)

CHANGELOG.md

@@ -9,7 +9,14 @@
 * Fixed `realm_add_realm_refresh_callback` and notify immediately that there is not transaction snapshot to advance to. ([#6075](https://github.com/realm/realm-core/issues/6075), since v12.6.0)
 
 ### Breaking changes
-* None.
+* FLX Subscription API reworked to better match SDK consumption patterns ([#6065](https://github.com/realm/realm-core/pull/6065)). Not all changes are breaking, but listing them all here together.
+  * `Subscription` is now a plain struct with public fields rather than getter functions
+    * `has_name()` and `name()` were merged into a single `optional<string> name` field
+  * `SubscriptionSet` now uses the same types for `iterator` and `const_iterator` since neither was intended to support direct mutability
+  * `SubscriptionSet::get_state_change_notification()` now offers a callback-taking overload
+  * `SubscriptionSet::find()` overloads now return `const Subscription*` with `nullptr` to signal not-found, rather than an at-end iterator
+  * `MutableSubscriptionSet::erase()` now has overloads taking a `StringData name` or a `Query`. Both return a bool to indicate if anything was found and removed.
+  * `MutableSubscriptionSet::commit()` is no longer `&&`-qualified. This means you no longer need to `std::move()` the set when calling it.
 
 ### Compatibility
 * Fileformat: Generates files with format v23. Reads and automatically upgrade from fileformat v5.

src/realm/data_type.hpp

@@ -35,7 +35,6 @@ typedef float Float;
 typedef double Double;
 typedef realm::StringData String;
 typedef realm::BinaryData Binary;
-typedef realm::Timestamp Timestamp;
 typedef realm::Decimal128 Decimal;
 
 struct ColumnType;

src/realm/object-store/c_api/conversion.hpp

@@ -25,6 +25,10 @@ static inline realm_string_t to_capi(StringData data)
     return realm_string_t{data.data(), data.size()};
 }
 
+// Because this is often used as `return to_capi(...);` it is dangerous to pass a temporary string here. If you really
+// need to and know it is correct (eg passing to a C callback), you can explicitly create the StringData wrapper.
+realm_string_t to_capi(const std::string&& str) = delete; // temporary std::string would dangle.
+
 static inline realm_string_t to_capi(const std::string& str)
 {
     return to_capi(StringData{str});

src/realm/object-store/c_api/sync.cpp

@@ -507,41 +507,41 @@ RLM_API void realm_sync_config_set_resync_mode(realm_sync_config_t* config,
 RLM_API realm_object_id_t realm_sync_subscription_id(const realm_flx_sync_subscription_t* subscription) noexcept
 {
     REALM_ASSERT(subscription != nullptr);
-    return to_capi(subscription->id());
+    return to_capi(subscription->id);
 }
 
 RLM_API realm_string_t realm_sync_subscription_name(const realm_flx_sync_subscription_t* subscription) noexcept
 {
     REALM_ASSERT(subscription != nullptr);
-    return to_capi(subscription->name());
+    return to_capi(subscription->name);
 }
 
 RLM_API realm_string_t
 realm_sync_subscription_object_class_name(const realm_flx_sync_subscription_t* subscription) noexcept
 {
     REALM_ASSERT(subscription != nullptr);
-    return to_capi(subscription->object_class_name());
+    return to_capi(subscription->object_class_name);
 }
 
 RLM_API realm_string_t
 realm_sync_subscription_query_string(const realm_flx_sync_subscription_t* subscription) noexcept
 {
     REALM_ASSERT(subscription != nullptr);
-    return to_capi(subscription->query_string());
+    return to_capi(subscription->query_string);
 }
 
 RLM_API realm_timestamp_t
 realm_sync_subscription_created_at(const realm_flx_sync_subscription_t* subscription) noexcept
 {
     REALM_ASSERT(subscription != nullptr);
-    return to_capi(subscription->created_at());
+    return to_capi(subscription->created_at);
 }
 
 RLM_API realm_timestamp_t
 realm_sync_subscription_updated_at(const realm_flx_sync_subscription_t* subscription) noexcept
 {
     REALM_ASSERT(subscription != nullptr);
-    return to_capi(subscription->updated_at());
+    return to_capi(subscription->updated_at);
 }
 
 RLM_API void realm_sync_config_set_before_client_reset_handler(realm_sync_config_t* config,
@@ -654,10 +654,10 @@ realm_sync_find_subscription_by_name(const realm_flx_sync_subscription_set_t* su
                                      const char* name) noexcept
 {
     REALM_ASSERT(subscription_set != nullptr);
-    auto it = subscription_set->find(name);
-    if (it == subscription_set->end())
+    auto ptr = subscription_set->find(name);
+    if (!ptr)
         return nullptr;
-    return new realm_flx_sync_subscription_t(*it);
+    return new realm_flx_sync_subscription_t(*ptr);
 }
 
 RLM_API realm_flx_sync_subscription_t*
@@ -666,10 +666,10 @@ realm_sync_find_subscription_by_results(const realm_flx_sync_subscription_set_t*
 {
     REALM_ASSERT(subscription_set != nullptr);
     auto realm_query = add_ordering_to_realm_query(results->get_query(), results->get_ordering());
-    auto it = subscription_set->find(realm_query);
-    if (it == subscription_set->end())
+    auto ptr = subscription_set->find(realm_query);
+    if (!ptr)
         return nullptr;
-    return new realm_flx_sync_subscription_t{*it};
+    return new realm_flx_sync_subscription_t{*ptr};
 }
 
 RLM_API realm_flx_sync_subscription_t*
@@ -690,10 +690,10 @@ realm_sync_find_subscription_by_query(const realm_flx_sync_subscription_set_t* s
 {
     REALM_ASSERT(subscription_set != nullptr);
     auto realm_query = add_ordering_to_realm_query(query->get_query(), query->get_ordering());
-    auto it = subscription_set->find(realm_query);
-    if (it == subscription_set->end())
+    auto ptr = subscription_set->find(realm_query);
+    if (!ptr)
         return nullptr;
-    return new realm_flx_sync_subscription_t(*it);
+    return new realm_flx_sync_subscription_t(*ptr);
 }
 
 RLM_API bool realm_sync_subscription_set_refresh(realm_flx_sync_subscription_set_t* subscription_set)
@@ -762,7 +762,7 @@ RLM_API bool realm_sync_subscription_set_erase_by_id(realm_flx_sync_mutable_subs
     *erased = false;
     return wrap_err([&] {
         auto it = std::find_if(subscription_set->begin(), subscription_set->end(), [id](const Subscription& sub) {
-            return from_capi(*id) == sub.id();
+            return from_capi(*id) == sub.id;
         });
         if (it != subscription_set->end()) {
             subscription_set->erase(it);
@@ -778,10 +778,7 @@ RLM_API bool realm_sync_subscription_set_erase_by_name(realm_flx_sync_mutable_su
     REALM_ASSERT(subscription_set != nullptr && name != nullptr);
     *erased = false;
     return wrap_err([&]() {
-        if (auto it = subscription_set->find(name); it != subscription_set->end()) {
-            subscription_set->erase(it);
-            *erased = true;
-        }
+        *erased = subscription_set->erase(name);
         return true;
     });
 }
@@ -793,10 +790,7 @@ RLM_API bool realm_sync_subscription_set_erase_by_query(realm_flx_sync_mutable_s
     *erased = false;
     return wrap_err([&]() {
         auto realm_query = add_ordering_to_realm_query(query->get_query(), query->get_ordering());
-        if (auto it = subscription_set->find(realm_query); it != subscription_set->end()) {
-            subscription_set->erase(it);
-            *erased = true;
-        }
+        *erased = subscription_set->erase(realm_query);
         return true;
     });
 }
@@ -808,10 +802,7 @@ RLM_API bool realm_sync_subscription_set_erase_by_results(realm_flx_sync_mutable
     *erased = false;
     return wrap_err([&]() {
         auto realm_query = add_ordering_to_realm_query(results->get_query(), results->get_ordering());
-        if (auto it = subscription_set->find(realm_query); it != subscription_set->end()) {
-            subscription_set->erase(it);
-            *erased = true;
-        }
+        *erased = subscription_set->erase(realm_query);
         return true;
     });
 }

src/realm/sync/client.cpp

@@ -1036,7 +1036,7 @@ void SessionWrapper::on_flx_sync_error(int64_t version, std::string_view err_msg
 
     auto mut_subs = get_flx_subscription_store()->get_mutable_by_version(version);
     mut_subs.update_state(SubscriptionSet::State::Error, err_msg);
-    std::move(mut_subs).commit();
+    mut_subs.commit();
 }
 
 void SessionWrapper::on_flx_sync_version_complete(int64_t version)
@@ -1085,7 +1085,7 @@ void SessionWrapper::on_flx_sync_progress(int64_t new_version, DownloadBatchStat
 
     auto mut_subs = get_flx_subscription_store()->get_mutable_by_version(new_version);
     mut_subs.update_state(new_state);
-    std::move(mut_subs).commit();
+    mut_subs.commit();
 }
 
 SubscriptionStore* SessionWrapper::get_flx_subscription_store()
@@ -1484,7 +1484,7 @@ void SessionWrapper::on_download_completion()
                              m_flx_pending_mark_version);
         auto mutable_subs = m_flx_subscription_store->get_mutable_by_version(m_flx_pending_mark_version);
         mutable_subs.update_state(SubscriptionSet::State::Complete);
-        std::move(mutable_subs).commit();
+        mutable_subs.commit();
         m_flx_pending_mark_version = SubscriptionSet::EmptyVersion;
     }