Replication of operations on nested collections #6658
Conversation
| else if (type == type_List) { | ||
| // throw IllegalOperation("Cannot sync nested list"); | ||
| return Instruction::Payload(Instruction::Payload::List()); | ||
| } |
There was a problem hiding this comment.
Did you intend to keep the commented out throw statements?
There was a problem hiding this comment.
We need to disallow syncing of nested collections until this is supported by the server. So at some point we should throw here,
There was a problem hiding this comment.
Shouldn't we throw now and then update it to return the payload when we have server support? Edit: I guess we want it this way so we can test with the legacy sync server right? When will we want to throw then?
|
Some tests in the |
|
@danieltabacaru fixed error. |
Nice. There also seems to be a linting error. |
| @@ -345,6 +370,7 @@ struct Payload { | |||
| switch (lhs.type) { | |||
| case Type::Erased: | |||
| return true; | |||
| case Type::List: | |||
There was a problem hiding this comment.
nit: [[fallthrough]];
There was a problem hiding this comment.
We don't use [[fallthrough]]; when the labels point to the same code. I collapsed even more cases.
| @@ -185,6 +199,9 @@ struct Payload { | |||
| /// Note: For Mixed columns (including typed links), no separate value is required, because the | |||
| /// instruction set encodes the type of each value in the instruction. | |||
| enum class Type : int8_t { | |||
| // Special value indicating that a list should be created at the position. | |||
| List = -5, | |||
There was a problem hiding this comment.
Please update the comment at line 194. Do we also need an ErasedList option? Or if Erased should be used in both cases, then consider updation the comment as well.
There was a problem hiding this comment.
Comment updated. We just use the normal Instruction::ArrayErase instruction.
jedelbo
force-pushed
the
je/nested-collection-path
branch
from
6491145
to
d2a07a9
Compare
jedelbo
force-pushed
the
je/nested-collection-replication
branch
from
ac41eaf
to
ec740cf
Compare
src/realm/sync/instructions.hpp
Outdated
| struct ObjectValue { | ||
| }; | ||
| struct ObjectValue {}; | ||
| /// Create an empty list in-place (does not clear an existing lsit). |
test/test_sync.cpp
Outdated
| { | ||
| ReadTransaction read_1{db_1}; | ||
| ReadTransaction read_2{db_2}; | ||
| // tr.get_group().to_json(std::cout); |
There was a problem hiding this comment.
you should remove this. same below
|
|
||
| { | ||
| ReadTransaction read_1{db_1}; | ||
| ReadTransaction read_2{db_2}; |
There was a problem hiding this comment.
we could check both realms have the same data, i.e,:
CHECK(compare_groups(rt_1, rt_2));
jedelbo
force-pushed
the
je/nested-collection-replication
branch
from
be93b1a
to
2223b6d
Compare
What, How & Why?
Make sure nested collections can be replicated through our legacy test framework.
☑️ ToDos