Composite Primary Keys broken on replicated collections

[peterkogo]

BUG REPLICATION

Describe the problem you have without this PR

When documents are added to a collection with a composite primary key through the pull handler of a replication, the resulting composite primary key is accessed before it is generated.

[peterkogo]

This is a work in progress - I wanted to check if the unit tests actually work. I hope it is ok that I already opened the pull request.

[peterkogo]

@pubkey the bug reproduction is finished.
I tried to look for a possible fix, however I lack a deeper grasp of the replication flow to be confident about my reasoning.

[pubkey]

Test looks ok, however this is intentional. RxDB expects the remote to only send documents that exactly match the schema, so the primary key cannot be missing.
I would merge a feature to fill up the primary key if possible, but I will not do the work by myself.
If we fill up missing keys, we also need a check to ensure that an error is thrown if the composite primary key is there but different than how RxDB would create it.

[peterkogo]

I find it odd that the behaviour is different for data coming through replication vs. data coming through insert(). What is the reasoning behind this?

I thought the logic behind supporting composite primary keys in general was to make it compatible with backend data models - in my case a postgres database - but the composite key behaviour of the backend is not directly compatible with rxdb (concatinating two strings with a separator).
I think it is bad design to expect the remote data/api to reflect an implementation that is configured and only relevant on the client.

Anyways, I'd be interested to add this if you can give me a little hint where the change would make sense.
Otherwise I'll make a pull request to add additional documentation on the current state of affairs. Right now it sounds like the key added for the composite primary is solely there for rxdb to fill it in.

[pubkey]

You are totally right, the current behavior is not correct. I just wanted to tell you that, so you do not have to look at error at your side.

Anyways, I'd be interested to add this if you can give me a little hint where the change would make sense.

If you want to fix that, it should be done in the replication plugin. There we already transform all data that comes from the server. Atm it only replaces the custom deleted field with _deleted. This is done with the function swapdeletedFieldToDefaultDeleted(). So you can check the parts of the code where this function is used, and there it should also check/fill-up the primary key. The solution will be just a few lines of code, but building the correct tests could be some work to do.

[peterkogo]

@pubkey I just wanted to start working on this and saw you closed this with a fix! Thank you!
Do you still need some help with additional tests?

[pubkey]

@peterkey
I just added your single test.
We could need an additional test (or enhance the current one) to ensure the composite key is filled when the document comes from the pull.stream$.