address commnets after rebase

db/active_replicator.go

@@ -211,8 +211,7 @@ func connect(arc *activeReplicatorCommon, idSuffix string) (blipSender *blip.Sen
 	// TODO: CBG-3661 ActiveReplicator subprotocol versions
 	// - make this configurable for testing mixed-version replications
 	// - if unspecified, default to v2 and v3 until VV is supported with ISGR, then also include v4
-	protocols := []string{CBMobileReplicationV3.SubprotocolString()}
-	//blipContext, err := NewSGBlipContext(arc.ctx, arc.config.ID+idSuffix, originPatterns)
+	protocols := []string{CBMobileReplicationV3.SubprotocolString(), CBMobileReplicationV2.SubprotocolString()} //blipContext, err := NewSGBlipContext(arc.ctx, arc.config.ID+idSuffix, originPatterns)
 	blipContext, err := NewSGBlipContextWithProtocols(arc.ctx, arc.config.ID+idSuffix, originPatterns, protocols)
 	if err != nil {
 		return nil, nil, err

db/blip_handler.go

@@ -483,17 +483,14 @@ func (bh *blipHandler) sendChanges(sender *blip.Sender, opts *sendChangesOptions
 					for _, item := range change.Changes {
 						changeRow := bh.buildChangesRow(change, item["rev"])
 						pendingChanges = append(pendingChanges, changeRow)
-						if err := sendPendingChangesAt(opts.batchSize); err != nil {
-							return err
-						}
 					}
 				} else {
-					// populate change row with current version
 					changeRow := bh.buildChangesRow(change, change.CurrentVersion.String())
 					pendingChanges = append(pendingChanges, changeRow)
-					if err := sendPendingChangesAt(opts.batchSize); err != nil {
-						return err
-					}
+				}
+
+				if err := sendPendingChangesAt(opts.batchSize); err != nil {
+					return err
 				}
 			}
 		}

db/hybrid_logical_vector.go

@@ -410,35 +410,32 @@ func (hlv *HybridLogicalVector) setPreviousVersion(source string, version uint64
 // toHistoryForHLV formats blip History property for V4 replication and above
 func (hlv *HybridLogicalVector) toHistoryForHLV() string {
 	// take pv and mv from hlv if defined and add to history
-	var pvString, mvString string
 	var s strings.Builder
+	var separatorNeeded bool
 	// Merge versions must be defined first if they exist
 	if hlv.MergeVersions != nil {
-		mvString = extractStringFromMap(hlv.MergeVersions)
-		if mvString != "" {
-			s.WriteString(mvString)
+		var pairList []string
+		for key, value := range hlv.MergeVersions {
+			vrs := Version{SourceID: key, Value: value}
+			pairList = append(pairList, vrs.String())
+		}
+		if len(pairList) > 0 {
+			separatorNeeded = true
+			s.WriteString(strings.Join(pairList, ","))
 		}
 	}
 	if hlv.PreviousVersions != nil {
-		pvString = extractStringFromMap(hlv.PreviousVersions)
-		if pvString != "" {
-			// if mv is defined we need separator
-			if mvString != "" {
-				s.WriteString(";")
-			}
-			s.WriteString(pvString)
+		if separatorNeeded && len(hlv.PreviousVersions) != 0 {
+			s.WriteString(";")
+		}
+		var pairList []string
+		for key, value := range hlv.PreviousVersions {
+			vrs := Version{SourceID: key, Value: value}
+			pairList = append(pairList, vrs.String())
+		}
+		if len(pairList) > 0 {
+			s.WriteString(strings.Join(pairList, ","))
 		}
 	}
 	return s.String()
 }
-
-// extractStringFromMap returns Couchbase Lite-compatible string representation of the map
-func extractStringFromMap(hlvMap map[string]uint64) string {
-	var pairList []string
-	for key, value := range hlvMap {
-		vrs := Version{SourceID: key, Value: value}
-		pairStr := vrs.String()
-		pairList = append(pairList, pairStr)
-	}
-	return strings.Join(pairList, ",")
-}

rest/blip_client_test.go

@@ -1045,8 +1045,9 @@ func (btc *BlipTesterCollectionClient) GetVersion(docID string, docVersion DocVe
 }
 
 func (btc *BlipTesterClient) AssertOnBlipHistory(t *testing.T, msg *blip.Message, docVersion DocVersion) {
-	if btc.SupportedBLIPProtocols[0] == db.CBMobileReplicationV4.SubprotocolString() {
-		// history could be empty a lot of the time in HLV messages as updates from the same source won't populate previous versions
+	subProtocol, err := db.ParseSubprotocolString(btc.SupportedBLIPProtocols[0])
+	require.NoError(t, err)
+	if subProtocol >= db.CBMobileReplicationV4 { // history could be empty a lot of the time in HLV messages as updates from the same source won't populate previous versions
 		if msg.Properties[db.RevMessageHistory] != "" {
 			assert.Equal(t, docVersion.CV.String(), msg.Properties[db.RevMessageHistory])
 		}