CBL-6597: The proposeChange not include remote revision id for the up…

[jianminzhao]

…dated doc when using version vector

When makeing a copy of a VectorDocument, we need to make up the effect of flag kSync.

[cbl-bot]

Code Coverage Results:

Type	Percentage
branches	66.2
functions	78.57
instantiations	70.73
lines	77.6
regions	73.33

[snej]

This change duplicates what's already done in VectorRecord::readRecordExtra.
If for some reason that code isn't doing the job, we should figure out why and fix it.

[jianminzhao]

As I commented in the code, VectorRecord::readRecordExtra does try to get _revisions from extraDoc. However, extraDoc does not the revision. For document that has kSynced flagged, it gets the revision from current and clear the flag. When this VectorRecord (srcRecord) is the source for a new VectorRecord, we encounter the situation whereby

1. srcRecord's extraDoc does not have revisions
2. srcRecord's flag kSynced is clear.

Therefore, VectorRecord::readRecordExtra won't get the revisions. What the PR does is to recognize the possibility that the srcRecord is the result of flag kSynced.

[snej]

This isn't the fix I described in Slack yesterday. Let me be more detailed.

1. Add bool VectorRecord::_wasSynced {false};. You did that.
2. When detecting and clearing the kSynced flag, set _wasSynced to true.
3. In the originalRecord() method, if _wasSynced is true, set the kSynced flag of the Record being created.

That's it. There's no need to change the copy constructor or any method signature.

[jianminzhao]

I did (1) and (2). The tricky thing is how to parse _wasSync from other to this in the following,

VectorRecord::VectorRecord(const VectorRecord& other)
        : VectorRecord(other._store, other.originalRecord(), other._wasSynced) {}

without the new argument, wasSynced, the delegated constructor, VectorRecord(KeyStore, Record). The actual argument if to avoid adding a new flag in Record. That was actual my first implementation. Later I found the flag in Record is no so natural. When will that flag be cleared, for example. Maybe we can assume this is the only occasion for the flag and not worry to reset it. I can do it.

Manage of VectorRecord:_wasSynced seems clear: it is set when _kSynced is cleared, on unset whenever setRemoteRevision(1, ?) is called, because if it is called not due to kSynced, we shouldn't apply the logic when kSynced true and cleared afterwards. _wasSynced is transitive. It is only used in the copy constructor.

Is the only concern the new argument to the copy constructor?