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.

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

Allow collections of non-embedded objects in asymmetric objects #7003

Merged
merged 2 commits into from
Oct 2, 2023

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Sep 27, 2023

The schema validation checks forbidding this were removed in #6981, but Table
also had its own validation. Most of the changes here are just making the asymmetric sync tests verify that the expected documents are actually created on the server.

@tgoyne tgoyne self-assigned this Sep 27, 2023
@cla-bot cla-bot bot added the cla: yes label Sep 27, 2023
@coveralls-official
Copy link

coveralls-official bot commented Sep 27, 2023

Pull Request Test Coverage Report for Build github_pull_request_277462

  • 281 of 281 (100.0%) changed or added relevant lines in 7 files are covered.
  • 77 unchanged lines in 11 files lost coverage.
  • Overall coverage increased (+0.5%) to 91.655%

Files with Coverage Reduction New Missed Lines %
src/realm/array_backlink.cpp 2 95.9%
src/realm/object-store/impl/epoll/external_commit_helper.cpp 2 78.41%
src/realm/sync/changeset.hpp 2 73.49%
src/realm/sync/network/http.hpp 2 80.87%
src/realm/sync/noinst/client_impl_base.cpp 2 85.82%
src/realm/util/file.cpp 2 81.25%
src/realm/util/serializer.cpp 3 90.03%
src/realm/link_translator.cpp 4 80.33%
src/realm/sync/noinst/client_reset.cpp 4 92.99%
src/realm/bplustree.cpp 6 75.72%
Totals Coverage Status
Change from base Build 1721: 0.5%
Covered Lines: 230380
Relevant Lines: 251356

💛 - Coveralls

@tgoyne tgoyne force-pushed the tg/asymmetric-object-list branch 2 times, most recently from 4422de3 to fc51de8 Compare September 27, 2023 20:23
@@ -2916,17 +2956,26 @@ TEST_CASE("flx: data ingest", "[sync][flx][data ingest][baas]") {

auto table = realm->read_group().get_table("class_Asymmetric");
REQUIRE(table->size() == 0);

auto documents = get_documents(*realm->config().sync_config->user, "Asymmetric", 100);
for (int i = 0; i < 100; ++i) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: use the container size (i.e, obj_ids.size()) (+ in some other places below)

realm->commit_transaction();

wait_for_upload(*realm);
wait_for_download(*realm);

auto table = realm->read_group().get_table("class_Asymmetric");
REQUIRE(table->size() == 0);

auto documents = get_documents(*realm->config().sync_config->user, "Asymmetric", 2);
check_document(documents, foo_obj_id, {{"embedded obj", BsonDocument{{"value", "foo"}}}});
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we check the second doc was created as well?

@@ -2864,17 +2903,19 @@ TEST_CASE("flx: data ingest", "[sync][flx][data ingest][baas]") {
Object::create(c, realm, "Asymmetric", std::any(AnyDict{{"_id", foo_obj_id}, {"location", "foo"s}}));
Object::create(c, realm, "Asymmetric", std::any(AnyDict{{"_id", bar_obj_id}, {"location", "bar"s}}));
realm->commit_transaction();
wait_for_upload(*realm);

auto documents = get_documents(*realm->config().sync_config->user, "Asymmetric", 2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

you should wait for upload before getting the documents. This works too because sync is not stopped, but you are querying the server unnecessarily (should be added in some other places below as well).

Copy link
Member Author

Choose a reason for hiding this comment

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

Waiting for the upload to complete before polling doesn't really change how many times we poll. Upload completion happens about a quarter of the way through the time to when the documents are visible for asymmetric sync (and about 1% of the way for non-asymmetric).

test/object-store/util/test_file.cpp Show resolved Hide resolved
test/object-store/sync/flx_sync.cpp Outdated Show resolved Hide resolved
@tgoyne tgoyne force-pushed the tg/asymmetric-object-list branch 2 times, most recently from be58eae to b54fe1b Compare September 29, 2023 23:33
It's unclear if this actually caused any problems, but there were a number of
places where we were deleting Realm files while they were in use due to copying
a TestFile and then deleting the copy when we actually wanted to copy the
config.
The schema validation checks forbidding this were removed in #6981, but Table
also had its own validation.
@tgoyne tgoyne merged commit e76f494 into master Oct 2, 2023
29 of 30 checks passed
@tgoyne tgoyne deleted the tg/asymmetric-object-list branch October 2, 2023 18:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants