CBL-7977: Allow to access Correlation ID from C4Replicator by jianminzhao · Pull Request #2463 · couchbase/couchbase-lite-core

jianminzhao · 2026-04-03T23:30:09Z

No description provided.

cbl-bot · 2026-04-04T00:38:28Z

Code Coverage Results:

Type	Percentage
branches	64.82
functions	77.39
instantiations	70.97
lines	76.03
regions	72.35

Copilot

Pull request overview

Adds a public API to retrieve the server-provided “X-Correlation-Id” for a replication connection, exposing it via C4Replicator and validating it in replicator API tests.

Changes:

Introduces c4repl_getCorrelationID in the C API and wires it through the C++ C4Replicator interface to the underlying Replicator.
Captures and stores the correlation ID from the WebSocket HTTP response header during connect, and includes it in replicator logging key/value pairs.
Adds an API test that asserts the replicator correlation ID matches the value in the response headers.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
Replicator/tests/ReplicatorAPITest.hh	Adds a test context pointer used by the new correlation-id test callback.
Replicator/tests/ReplicatorAPITest.cc	Adds a new test that reads correlation ID via the C API and compares it to the response header.
Replicator/Replicator.hh	Adds correlation ID getter/setter and related state for thread-safe access.
Replicator/Replicator.cc	Stores correlation ID from response headers and uses it in logging key/value pairs.
Replicator/c4ReplicatorImpl.hh	Implements `C4Replicator::correlationID()` by delegating to `Replicator`.
Replicator/c4Replicator_CAPI.cc	Adds `c4repl_getCorrelationID` C API entry point.
C/scripts/c4.txt	Registers `c4repl_getCorrelationID` for C API script generation.
C/include/c4Replicator.h	Declares the new public C API function with documentation.
C/Cpp_include/c4Replicator.hh	Extends the C++ API `C4Replicator` interface with `correlationID()`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-06T16:14:37Z

Replicator/tests/ReplicatorAPITest.cc

    CHECK(c4coll_getDocumentCount(collRose2) == expectDocCountInDB2);
 }

+TEST_CASE_METHOD(ReplicatorAPITest, "API getCollectionID", "[C][Sync]") {


The new test case name appears incorrect: it validates Correlation ID behavior (c4repl_getCorrelationID / X-Correlation-Id header), but the test is named "API getCollectionID". Renaming it to reflect Correlation ID would avoid confusion when triaging failures and searching test output.

Suggested change

TEST_CASE_METHOD(ReplicatorAPITest, "API getCollectionID", "[C][Sync]") {

TEST_CASE_METHOD(ReplicatorAPITest, "API getCorrelationID", "[C][Sync]") {

Good catch. Changed

Copilot · 2026-04-06T16:14:37Z

Replicator/tests/ReplicatorAPITest.cc

+    alloc_slice xid;
+    _onDocsEnded = [](C4Replicator* repl, bool pushing, size_t numDocs, const C4DocumentEnded* docs[], void* context) {
+        CHECK(numDocs == 1);
+        *((alloc_slice*)((ReplicatorAPITest*)context)->_testContext) = c4repl_getCorrelationID(repl);
+    };
+    _enableDocProgressNotifications = true;
+    _testContext                    = &xid;
+    replicate(kC4OneShot, kC4Disabled);


Using a generic void* (_testContext) to smuggle an alloc_slice* into the documents-ended callback is not type-safe and makes it easy to introduce lifetime bugs (e.g., pointing at a stack variable) in future edits. Prefer storing the correlation ID in a typed member on ReplicatorAPITest (e.g., alloc_slice _correlationIDForTest) or making _testContext a correctly-typed pointer (alloc_slice*) to keep the intent and ownership clear.

This member field, _testContext, is supposed to be used per test.

Copilot · 2026-04-06T16:14:37Z

Replicator/c4ReplicatorImpl.hh

+        alloc_slice correlationID() const noexcept override {
+            if ( Retained<Replicator> retained = _replicator; retained ) return retained->getCorrelationID();
+            else
+                return {};
+        }


Adding the pure-virtual C4Replicator::correlationID() changes the C++ replicator interface and is an incompatible change for out-of-tree subclasses (notably c4LocalReplicator in the EE repo per the comment above). Please bump C4ReplicatorImpl::API_VERSION accordingly so mismatched builds fail fast instead of producing undefined behavior.

@pasin should we bump the API_VERSION? It should break SDK. What he points out, the EE repo is actually a internal part of LIteCore.

I don't know the answer of this question. The platform doesn't use API_VERSION for anything. From the comment // This number has no absolute meaning but is bumped whenever the LiteCore public API changes., seem like it's a practice to bump. You can check with Jens to be sure.

I did not see any reference to LITECORE_API_VERSION 352 but I do see C4ReplicatorImpl::API_VERSION in working. Bumped it from 5 to 6.

Copilot · 2026-04-06T16:14:38Z

Replicator/Replicator.hh

+        alloc_slice getCorrelationID() const {
+            if ( _isCorrIDAvailable.load(std::memory_order_acquire) ) return _correlationID;
+            else
+                return {};
+        }
+
      private:
+        // The setter is supposedly to be called once.
+        void setCorrelationID(slice corrID) {
+            DebugAssert(!_isCorrIDAvailable);
+            _correlationID = corrID;
+            _isCorrIDAvailable.store(true, std::memory_order_release);
+        }


c4repl_getCorrelationID is declared as thread-safe in the C header; Replicator::getCorrelationID currently isn’t marked noexcept even though it’s used from a noexcept virtual (C4Replicator::correlationID). Consider marking getCorrelationID (and setCorrelationID) noexcept to align with the API contract and avoid unexpected terminate() if an exception ever propagates.

Our convention is to let c4 functions to catch the exceptions.

Replicator/tests/ReplicatorAPITest.hh

pasin · 2026-04-06T17:26:38Z

Replicator/c4ReplicatorImpl.hh

+        alloc_slice correlationID() const noexcept override {
+            if ( Retained<Replicator> retained = _replicator; retained ) return retained->getCorrelationID();
+            else
+                return {};
+        }


I don't know the answer of this question. The platform doesn't use API_VERSION for anything. From the comment // This number has no absolute meaning but is bumped whenever the LiteCore public API changes., seem like it's a practice to bump. You can check with Jens to be sure.

Replicator/Replicator.hh

1. changing isCorrIDAvailable to hasCorrelationID 2. add a test show CorrelationID after C4Replcatore is stopped and restarted.

CBL-7977: Allow to access Correlation ID from C4Replicator

23815b8

jianminzhao force-pushed the cbl-7977 branch from 27c62b8 to 23815b8 Compare April 4, 2026 00:05

jianminzhao requested a review from pasin April 6, 2026 15:32

pasin requested a review from Copilot April 6, 2026 16:09

Copilot started reviewing on behalf of pasin April 6, 2026 16:10 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

Merge branch 'master' into cbl-7977

22ba331

pasin reviewed Apr 6, 2026

View reviewed changes

jianminzhao added 2 commits April 6, 2026 15:53

addressing review comment:

2debc7d

1. changing isCorrIDAvailable to hasCorrelationID 2. add a test show CorrelationID after C4Replcatore is stopped and restarted.

bump C4ReplicatorImpl::API_VERSION to 6

b4ec279

jianminzhao requested a review from pasin April 7, 2026 15:42

pasin approved these changes Apr 7, 2026

View reviewed changes

	TEST_CASE_METHOD(ReplicatorAPITest, "API getCollectionID", "[C][Sync]") {
	TEST_CASE_METHOD(ReplicatorAPITest, "API getCorrelationID", "[C][Sync]") {

Conversation

jianminzhao commented Apr 3, 2026

Uh oh!

cbl-bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jianminzhao Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jianminzhao Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jianminzhao Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

pasin Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jianminzhao Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jianminzhao Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pasin Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cbl-bot commented Apr 4, 2026 •

edited

Loading