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

Do not allow asymmetric tables in pbs #5853

Merged
merged 6 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* `Realm::refresh()` did not actually advance to the latest version in some cases. If there was a version newer than the current version which did not require blocking it would advance to that instead, contrary to the documented behavior.
* Fixed `realm_query_parse_for_results` ignoring query for `query_result_t` passed as parameter ([#5841](https://github.com/realm/realm-core/pull/5841)).
* Fixed `realm_query_parse_for_list` ignoring existing query ([#5850](https://github.com/realm/realm-core/pull/5850)).
* Fixed not allowing asymmetric tables in partition based sync ([#5691](https://github.com/realm/realm-core/issues/5691)).

### Breaking changes
* None.
Expand Down
5 changes: 3 additions & 2 deletions src/realm.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,9 @@ typedef enum realm_value_type {

typedef enum realm_schema_validation_mode {
RLM_SCHEMA_VALIDATION_BASIC = 0,
RLM_SCHEMA_VALIDATION_SYNC = 1,
RLM_SCHEMA_VALIDATION_REJECT_EMBEDDED_ORPHANS = 2
RLM_SCHEMA_VALIDATION_SYNC_PBS = 1,
RLM_SCHEMA_VALIDATION_REJECT_EMBEDDED_ORPHANS = 2,
RLM_SCHEMA_VALIDATION_SYNC_FLX = 4
} realm_schema_validation_mode_e;

/**
Expand Down
2 changes: 1 addition & 1 deletion src/realm/object-store/c_api/schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ RLM_API uint64_t realm_get_schema_version(const realm_t* realm)
RLM_API bool realm_schema_validate(const realm_schema_t* schema, uint64_t validation_mode)
{
return wrap_err([&]() {
schema->ptr->validate(validation_mode);
schema->ptr->validate(static_cast<SchemaValidationMode>(validation_mode));
return true;
});
}
Expand Down
10 changes: 9 additions & 1 deletion src/realm/object-store/object_schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ static void validate_property(Schema const& schema, ObjectSchema const& parent_o
}

void ObjectSchema::validate(Schema const& schema, std::vector<ObjectSchemaValidationException>& exceptions,
bool for_sync) const
SchemaValidationMode validation_mode) const
{
std::vector<StringData> public_property_names;
std::vector<StringData> internal_property_names;
Expand Down Expand Up @@ -416,6 +416,8 @@ void ObjectSchema::validate(Schema const& schema, std::vector<ObjectSchemaValida
exceptions.emplace_back("Specified primary key '%1.%2' does not exist.", name, primary_key);
}

auto for_sync =
(validation_mode & SchemaValidationMode::SyncPBS) || (validation_mode & SchemaValidationMode::SyncFLX);
if (for_sync && table_type != ObjectSchema::ObjectType::Embedded) {
if (primary_key.empty()) {
exceptions.emplace_back(util::format("There must be a primary key property named '_id' on a synchronized "
Expand All @@ -432,6 +434,12 @@ void ObjectSchema::validate(Schema const& schema, std::vector<ObjectSchemaValida
if (!for_sync && table_type == ObjectSchema::ObjectType::TopLevelAsymmetric) {
exceptions.emplace_back(util::format("Asymmetric table '%1' not allowed in a local Realm", name));
}

auto pbs_sync =
(validation_mode & SchemaValidationMode::SyncPBS) && !(validation_mode & SchemaValidationMode::SyncFLX);
if (pbs_sync && table_type == ObjectSchema::ObjectType::TopLevelAsymmetric) {
exceptions.emplace_back(util::format("Asymmetric table '%1' not allowed in partition based sync", name));
}
}

namespace realm {
Expand Down
3 changes: 2 additions & 1 deletion src/realm/object-store/object_schema.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Table;
enum class PropertyType : unsigned short;
struct ObjectSchemaValidationException;
struct Property;
enum SchemaValidationMode : uint64_t;

class ObjectSchema {
public:
Expand Down Expand Up @@ -80,7 +81,7 @@ class ObjectSchema {
bool property_is_computed(Property const& property) const noexcept;

void validate(Schema const& schema, std::vector<ObjectSchemaValidationException>& exceptions,
bool for_sync) const;
SchemaValidationMode validation_mode) const;

friend bool operator==(ObjectSchema const& a, ObjectSchema const& b) noexcept;

Expand Down
5 changes: 2 additions & 3 deletions src/realm/object-store/schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ std::unordered_set<std::string> get_embedded_object_orphans(const Schema& schema

} // end anonymous namespace

void Schema::validate(uint64_t validation_mode) const
void Schema::validate(SchemaValidationMode validation_mode) const
{
std::vector<ObjectSchemaValidationException> exceptions;

Expand All @@ -206,9 +206,8 @@ void Schema::validate(uint64_t validation_mode) const
ObjectSchemaValidationException("Type '%1' appears more than once in the schema.", it->name));
}

const bool for_sync = validation_mode & SchemaValidationMode::Sync;
for (auto const& object : *this) {
object.validate(*this, exceptions, for_sync);
object.validate(*this, exceptions, validation_mode);
}

// TODO: remove this client side check once the server supports it
Expand Down
4 changes: 2 additions & 2 deletions src/realm/object-store/schema.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class StringData;
struct TableKey;
struct Property;

enum SchemaValidationMode : uint64_t { Basic = 0, Sync = 1, RejectEmbeddedOrphans = 2 };
enum SchemaValidationMode : uint64_t { Basic = 0, SyncPBS = 1, RejectEmbeddedOrphans = 2, SyncFLX = 4 };

// How to handle update_schema() being called on a file which has
// already been initialized with a different schema
Expand Down Expand Up @@ -147,7 +147,7 @@ class Schema : private std::vector<ObjectSchema> {

// Verify that this schema is internally consistent (i.e. all properties are
// valid, links link to types that actually exist, etc.)
void validate(uint64_t validation_mode = SchemaValidationMode::Basic) const;
void validate(SchemaValidationMode validation_mode = SchemaValidationMode::Basic) const;

// Get the changes which must be applied to this schema to produce the passed-in schema
std::vector<SchemaChange> compare(Schema const&, SchemaMode = SchemaMode::Automatic,
Expand Down
9 changes: 6 additions & 3 deletions src/realm/object-store/shared_realm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,14 +385,17 @@ void Realm::update_schema(Schema schema, uint64_t version, MigrationFunction mig
DataInitializationFunction initialization_function, bool in_transaction)
{
uint64_t validation_mode = SchemaValidationMode::Basic;
if (m_config.sync_config) {
validation_mode |= SchemaValidationMode::Sync;
#if REALM_ENABLE_SYNC
if (auto sync_config = m_config.sync_config) {
validation_mode |=
sync_config->flx_sync_requested ? SchemaValidationMode::SyncFLX : SchemaValidationMode::SyncPBS;
}
#endif
if (m_config.schema_mode == SchemaMode::AdditiveExplicit) {
validation_mode |= SchemaValidationMode::RejectEmbeddedOrphans;
}

schema.validate(validation_mode);
schema.validate(static_cast<SchemaValidationMode>(validation_mode));

bool was_in_read_transaction = is_in_read_transaction();
Schema actual_schema = get_full_schema();
Expand Down
9 changes: 5 additions & 4 deletions test/object-store/schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,8 @@ TEST_CASE("Schema") {
ObjectSchema::ObjectType::TopLevelAsymmetric,
{{"_id", PropertyType::Int, Property::IsPrimary{true}}, {"street", PropertyType::String}}},
};
REQUIRE_NOTHROW(schema.validate(SchemaValidationMode::Sync));
REQUIRE_NOTHROW(schema.validate(SchemaValidationMode::SyncFLX));
REQUIRE_THROWS(schema.validate(SchemaValidationMode::SyncPBS));
}

SECTION("asymmetric tables not allowed in local realm") {
Expand All @@ -392,7 +393,7 @@ TEST_CASE("Schema") {
{{"_id", PropertyType::Int, Property::IsPrimary{true}}}},
};
REQUIRE_THROWS_CONTAINING(
schema.validate(SchemaValidationMode::Sync),
schema.validate(SchemaValidationMode::SyncFLX),
"Property 'object.link' of type 'object' cannot be a link to an asymmetric object.");
}

Expand All @@ -403,7 +404,7 @@ TEST_CASE("Schema") {
{{"link", PropertyType::Object | PropertyType::Nullable, "link target"}}},
{"link target", {{"value", PropertyType::Int}}},
};
REQUIRE_THROWS_CONTAINING(schema.validate(SchemaValidationMode::Sync),
REQUIRE_THROWS_CONTAINING(schema.validate(SchemaValidationMode::SyncFLX),
"Asymmetric table with property 'object.link' of type 'object' cannot have a "
"non-embedded object type.");
}
Expand All @@ -416,7 +417,7 @@ TEST_CASE("Schema") {
{"link", PropertyType::Object | PropertyType::Nullable, "link target"}}},
{"link target", ObjectSchema::ObjectType::Embedded, {{"value", PropertyType::Int}}},
};
schema.validate(SchemaValidationMode::Sync);
schema.validate(SchemaValidationMode::SyncFLX);
}

SECTION("rejects array properties with no target object") {
Expand Down
15 changes: 15 additions & 0 deletions test/object-store/sync/flx_sync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1950,6 +1950,21 @@ TEST_CASE("flx: asymmetric sync", "[sync][flx][app]") {
});
}

SECTION("asymmetric table not allowed in PBS") {
Schema schema{
{"Asymmetric2",
ObjectSchema::ObjectType::TopLevelAsymmetric,
{
{"_id", PropertyType::Int, Property::IsPrimary{true}},
{"location", PropertyType::Int},
{"reading", PropertyType::Int},
}},
};

SyncTestFile config(harness->app(), bson::Bson{}, schema);
REQUIRE_THROWS(Realm::get_shared_realm(config));
}

// Add any new test sections above this point

SECTION("teardown") {
Expand Down