Connected client and Async #1394
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Only getDoc() is implemented so far. Also implemented the corresponding `getRev` BLIP message in the replicator, so it can be used in tests.
Make it better behaved if a bg thread is dumping logs for an exception while the main thread finishes a test case.
This greatly speeds up log calls in debug builds, making it more likely to catch Heisenbugs with logging on.
If both sockets in a pair are closed at about the same time, it's possible one will handle the close() just after its peer socket has told it it's closed. That triggered an assertion failure. I don't think that's a valid failure.
The replicator now only responds to connected-client messages "getRev", "putRev" if this option is set. (Also renamed "putDoc" to "putRev" for consistency.)
* subChanges: Use "future:true" instead of "since:NOW" * getRev: Don't return tombstones, just respond with 404 error.
* Add update doc API * generate revision ID and pass it back to platform as well.
- getDoc() method can't return C4DocResponse because that struct points to alloc_slices without owning them. Created a DocResponse struct whose members are actual alloc_slices. - Since C4ConnectedClient is RefCounted, the C retain and release functions can just be c4base_retain and c4base_release. - c4client_free is superseded by c4client_release. - Added retain/release wrappers in c4CppUtils.hh
The way the WebSocket was created wasn't compatible with the built-in WebSocket implementation used by CBL-C and the cblite tool, so I changed the call from `new C4SocketImpl` to `CreateWebSocket`. However, this function required a C4Database pointer ... turns out that's only so the WebSocket can access cookies. I changed the code to allow a null pointer, in which case it just won't do any cookie stuff. This is a short-term fix; long-term we need some other way to persist cookies, I think.
# Conflicts: # C/tests/c4Test.cc # LiteCore/Support/Async.hh # Networking/BLIP/BLIPConnection.cc # REST/RESTListener+Handlers.cc
…se/couchbase-lite-core into feature/connected-client
* Support for sending full query strings, if the server allows it. * Added C4ConnectedClient::query() * Added replicator constant kC4ReplicatorOptionAllQueries * Updated the protocol documentation.
This allows Fleece values in message bodies to be retained and copied, and eliminates a use of ValueFromData.
In the BLIP protocol, query results are now a series of JSON objects separated by newlines. In the LiteCore API, rows are now given to the callback as Dict, not Array, and the JSON value is given too. The Fleece form is optional.
Method names shouldn't be uppercase...
The options dict given to the WebSocket needs to contain all the options _plus_ `kC4SocketOptionWSProtocols`, not just the latter. Otherwise the WebSocket doesn't see the auth settings. While I was at it I removed c4ConnectedClientImpl.cc since it's only got two methods in it, making c4ConnectedClientImpl fully header-only. (It's only included in one place.)
It's too annoying having hundreds of HTML files update due to small C API changes when you're switching between branches.
Also getResponseHeaders() and getPeerTLSCertificate().
# Conflicts: # C/c4.def # C/c4.gnu # C/c4_ee.def # C/c4_ee.gnu # C/include/c4Base.h # REST/RESTListener+Handlers.cc
|
@snej Is this ready to review? |
callumbirks
reviewed
Mar 4, 2024
|
|
||
| /// A message number. Each peer numbers messages it sends sequentially starting at 1. | ||
| /// Each peer's message numbers are independent. | ||
| enum class MessageNo : messageno_t { }; |
There was a problem hiding this comment.
This is something I've seen before but don't understand, what's the reasoning for declaring this an enum class?
There was a problem hiding this comment.
Additionally, what's the motivation in the first place to make MessageNo its own class rather than simply using u64?
callumbirks
reviewed
Mar 4, 2024
There was a problem hiding this comment.
I love this document. Very excited to get these changes into the codebase.
callumbirks
reviewed
Mar 4, 2024
callumbirks
reviewed
Mar 4, 2024
There was a problem hiding this comment.
LGTM, as far as I can tell from attempting to read through. I'm certain there's things I could have missed, but that's what tests are for, and it looks like we're pretty well covered with those.
I need to update this branch to the latest master, and prepare to merge it in. What else do we need before that happens? I imagine changes for VV support? Anything else?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Infrastructure:
Result<T>holds either aTor aC4Error. Similar to types found in Swift, Kotlin, Rust, ...Async<T>is a promise/future of aT. (Actually of aResult<T>, since it can fail.) Integrates with Actors.Connected Client:
ConnectedClientopens a WebSocket (just like Replicator) and uses the BLIP replicator API to make CRUD requests and observe changes. Results are returned asAsyncvalues.