RCORE-2201 Add an implementation for the realm_app_config_get_sync_client_config() function in the Core CAPI

[michael-wb]

What, How & Why?

As part of the changes to separate the App from the Sync Client, a realm_app_config_get_sync_client_config() function was added to the C_API to retrieve a pointer to the sync client config stored in the AppConfig. Unfortunately, this function was never implemented and was only a function declaration.
There are two implementations for getting a realm_sync_client_config_t, depending on the value of the REALM_APP_SERVICES build flag:

• If it is set, realm_app_config_get_sync_client_config() is available to get a pointer to the sync_client_config member property of the realm_app_config_t structure. The value returned by this function is a reference and should not be freed using realm_release().
• If it is not set, realm_sync_client_config_new() is available to create a new realm_sync_client_config_t instance this is intended to be used with realm_sync_manager_create(), which is also only available if REALM_APP_SERVICES is not set. The value returned by this function is a new object and must be manually freed using realm_release()

The realm_sync_client_config_t returned by either realm_app_config_get_sync_client_config() or realm_sync_client_config_new() is compatible with the realm_sync_client_config_set_...() functions for setting the individual parameters of this structure.

The tests for realm_sync_client_config_t and realm_app_config_t were updated to verify these functions for retrieving an instance/reference and setting the SyncClientConfig values.

Fixes #7890

☑️ ToDos

• 📝 Changelog update
• 🚦 Tests (or not relevant)
• C-API, if public C++ API changed
• [ ] bindgen/spec.yml, if public C++ API changed

[coveralls-official]

Pull Request Test Coverage Report for Build michael.wilkersonbarker_1281

Details

• 52 of 52 (100.0%) changed or added relevant lines in 2 files are covered.
• 100 unchanged lines in 16 files lost coverage.
• Overall coverage decreased (-0.03%) to 90.987%

---

Files with Coverage Reduction	New Missed Lines	%
src/realm/sync/noinst/server/server.cpp	1	73.82%
test/fuzz_tester.hpp	1	57.73%
src/realm/cluster.cpp	2	75.85%
src/realm/query_expression.cpp	2	86.62%
src/realm/sync/network/http.hpp	2	82.27%
test/object-store/sync/client_reset.cpp	2	99.03%
test/test_lang_bind_helper.cpp	2	93.2%
src/realm/table.cpp	3	90.59%
src/realm/sync/noinst/server/server_history.cpp	4	63.51%
src/realm/util/assert.hpp	4	87.1%

Totals
Change from base Build 2495:	-0.03%
Covered Lines:	215500
Relevant Lines:	236846

---

💛 - Coveralls

[ironage]

I don't fully understand why this getter couldn't just be implemented. The issue I see is that with only a setter available, it becomes impossible to just set one property of the sync_client_config because realm_app_config_set_sync_client_config will overwrite everything. This means that this API will only allow a use like this:

1. make a new sync_client_config
2. set all the properties
3. use realm_app_config_set_sync_client_config to apply the changes
4. any further changes to the sync_client_config instance are not actually going to change the config that the app uses, and you'd have to do step 3 again

Whereas if we support a getter that returns a pointer to the already existing instance AppConfig::sync_client_config then users of this API can just get that pointer and modify it directly with the various setters, and never need to allocate memory on their side of things.

[tgoyne]

The problem is that in the non-appservices scenario users allocate and free realm_sync_client_config_t, which requires that it inherit from WrapC. Owning and non-owning pointers need to be different types with how we've structured the C API. We could add a second type, but a simpler variation on that would be to add getters and setters for the SyncConfig fields that just take an AppConfig.

[michael-wb]

I have been trying to add a second type that also uses the existing functions, but I don't think I've been able to get it just right without having to add separate functions for the second type. I tried creating a realm_sync_client_config that inherits from realm::SyncClientConfig and used by all the functions; and a realm_sync_client_config_wrapc that inherits from WrapC and realm_sync_client_config, which is returned by the sync_client_config_new() function....
I would be happy to just add setters for the SyncConfig fields (we usually don't have getters on most of the struct fields).

[ironage]

I see, that is an unfortunate conflict that muddies the API. Whatever seems easiest is fine.
If the SDKs use the realm_app_config_set_sync_client_config function as it is now, then I suppose they have created a new config instance that they manage the lifetime of anyways, which means that if they want to update a single property, they could set it on their instance, and then update the entire thing again using the setter. So not the end of the world.

[michael-wb]

I left the realm_app_config_set_sync_client_config() function in the code and also added individual C_API setter functions for the SyncClientConfig parameters in the app config to give the SDK a choice as to what they want to use. I could also easily add a realm_app_config_get_sync_client_config() function that returns a copy of the sync_client_config, but I thought that might be confusing/misleading...

[michael-wb]

@ironage - since the realm_sync_client_config_new() is not needed if REALM_APP_SERVICES is enabled, I just updated the realm_sync_client_config_t to be one of the two different types depending on the REALM_APP_SERVICES value:

• If it is set, realm_app_config_get_sync_client_config() is now available to get the sync client config from app config.
• If it is not set, realm_sync_client_config_new() is available to create the sync client config to use with the realm_sync_manager_create(), which is also only available if REALM_APP_SERVICES is not set.

[ironage]

LGTM

[rorbech]

I have verified this with realm/realm-kotlin#1779 and can confirm that we are now able to round trip things with our own platform web socket implementation.

[rorbech]

We should add a comment here that the returned pointer is owned by core and that it shouldn't be released and maybe some other remarks on lifecycle.

[michael-wb]

Sorry, I forgot to complete my comment....
It has been updated