Skip to content

Commit 1971af4

Browse files
authored
Partially migrate stream tokens from NSData to ByteString (#3643)
* Convert stream tokens from NSData to ByteString Note that this persists in using protobuf's conflation of empty and null strings. * Rename remote_event.mm to .cc
1 parent 2f5e4fd commit 1971af4

21 files changed

+94
-73
lines changed

Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ - (void)testEncodesQueryData {
215215
expected.targetId = targetID;
216216
expected.lastListenSequenceNumber = 10;
217217
expected.snapshotVersion.nanos = 1039000;
218-
expected.resumeToken = MakeNSData(resumeToken);
218+
expected.resumeToken = MakeNullableNSData(resumeToken);
219219
expected.query.parent = queryTarget.parent;
220220
expected.query.structuredQuery = queryTarget.structuredQuery;
221221

Firestore/Example/Tests/Local/FSTMutationQueueTests.mm

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
using firebase::firestore::model::kBatchIdUnknown;
4545
using firebase::firestore::model::Mutation;
4646
using firebase::firestore::model::SetMutation;
47+
using firebase::firestore::nanopb::ByteString;
4748
using firebase::firestore::testutil::Key;
4849
using firebase::firestore::testutil::Query;
4950

@@ -103,11 +104,11 @@ - (void)testAcknowledgeBatchID {
103104

104105
XCTAssertEqual([self batchCount], 3);
105106

106-
self.mutationQueue->AcknowledgeBatch(batch1, nil);
107+
self.mutationQueue->AcknowledgeBatch(batch1, {});
107108
self.mutationQueue->RemoveMutationBatch(batch1);
108109
XCTAssertEqual([self batchCount], 2);
109110

110-
self.mutationQueue->AcknowledgeBatch(batch2, nil);
111+
self.mutationQueue->AcknowledgeBatch(batch2, {});
111112
XCTAssertEqual([self batchCount], 2);
112113

113114
self.mutationQueue->RemoveMutationBatch(batch2);
@@ -124,7 +125,7 @@ - (void)testAcknowledgeThenRemove {
124125
self.persistence.run("testAcknowledgeThenRemove", [&]() {
125126
FSTMutationBatch *batch1 = [self addMutationBatch];
126127

127-
self.mutationQueue->AcknowledgeBatch(batch1, nil);
128+
self.mutationQueue->AcknowledgeBatch(batch1, {});
128129
self.mutationQueue->RemoveMutationBatch(batch1);
129130

130131
XCTAssertEqual([self batchCount], 0);
@@ -377,19 +378,19 @@ - (void)testRemoveMutationBatches {
377378
- (void)testStreamToken {
378379
if ([self isTestBaseClass]) return;
379380

380-
NSData *streamToken1 = [@"token1" dataUsingEncoding:NSUTF8StringEncoding];
381-
NSData *streamToken2 = [@"token2" dataUsingEncoding:NSUTF8StringEncoding];
381+
ByteString streamToken1("token1");
382+
ByteString streamToken2("token2");
382383

383384
self.persistence.run("testStreamToken", [&]() {
384385
self.mutationQueue->SetLastStreamToken(streamToken1);
385386

386387
FSTMutationBatch *batch1 = [self addMutationBatch];
387388
[self addMutationBatch];
388389

389-
XCTAssertEqualObjects(self.mutationQueue->GetLastStreamToken(), streamToken1);
390+
XCTAssertEqual(self.mutationQueue->GetLastStreamToken(), streamToken1);
390391

391392
self.mutationQueue->AcknowledgeBatch(batch1, streamToken2);
392-
XCTAssertEqualObjects(self.mutationQueue->GetLastStreamToken(), streamToken2);
393+
XCTAssertEqual(self.mutationQueue->GetLastStreamToken(), streamToken2);
393394
});
394395
}
395396

Firestore/Source/Local/FSTLocalSerializer.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ - (FSTPBTarget *)encodedQueryData:(const QueryData &)queryData {
238238
proto.targetId = queryData.target_id();
239239
proto.lastListenSequenceNumber = queryData.sequence_number();
240240
proto.snapshotVersion = [remoteSerializer encodedVersion:queryData.snapshot_version()];
241-
proto.resumeToken = MakeNSData(queryData.resume_token());
241+
proto.resumeToken = MakeNullableNSData(queryData.resume_token());
242242

243243
const Query &query = queryData.query();
244244
if (query.IsDocumentQuery()) {

Firestore/Source/Local/FSTLocalStore.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "Firestore/core/src/firebase/firestore/model/mutation.h"
3131
#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h"
3232
#include "Firestore/core/src/firebase/firestore/model/types.h"
33+
#include "Firestore/core/src/firebase/firestore/nanopb/byte_string.h"
3334

3435
namespace firebase {
3536
namespace firestore {
@@ -51,6 +52,7 @@ namespace auth = firebase::firestore::auth;
5152
namespace core = firebase::firestore::core;
5253
namespace local = firebase::firestore::local;
5354
namespace model = firebase::firestore::model;
55+
namespace nanopb = firebase::firestore::nanopb;
5456
namespace remote = firebase::firestore::remote;
5557

5658
NS_ASSUME_NONNULL_BEGIN
@@ -139,14 +141,14 @@ NS_ASSUME_NONNULL_BEGIN
139141
- (model::MaybeDocumentMap)rejectBatchID:(model::BatchId)batchID;
140142

141143
/** Returns the last recorded stream token for the current user. */
142-
- (nullable NSData *)lastStreamToken;
144+
- (nanopb::ByteString)lastStreamToken;
143145

144146
/**
145147
* Sets the stream token for the current user without acknowledging any mutation batch. This is
146148
* usually only useful after a stream handshake or in response to an error that requires clearing
147149
* the stream token.
148150
*/
149-
- (void)setLastStreamToken:(nullable NSData *)streamToken;
151+
- (void)setLastStreamToken:(const nanopb::ByteString &)streamToken;
150152

151153
/**
152154
* Returns the last consistent snapshot processed (used by the RemoteStore to determine whether to

Firestore/Source/Local/FSTLocalStore.mm

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include "Firestore/core/src/firebase/firestore/model/document_map.h"
4444
#include "Firestore/core/src/firebase/firestore/model/patch_mutation.h"
4545
#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h"
46+
#include "Firestore/core/src/firebase/firestore/nanopb/nanopb_util.h"
4647
#include "Firestore/core/src/firebase/firestore/remote/remote_event.h"
4748
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
4849
#include "Firestore/core/src/firebase/firestore/util/log.h"
@@ -224,7 +225,7 @@ - (LocalWriteResult)locallyWriteMutations:(std::vector<Mutation> &&)mutations {
224225
- (MaybeDocumentMap)acknowledgeBatchWithResult:(FSTMutationBatchResult *)batchResult {
225226
return self.persistence.run("Acknowledge batch", [&] {
226227
FSTMutationBatch *batch = batchResult.batch;
227-
_mutationQueue->AcknowledgeBatch(batch, batchResult.streamToken);
228+
_mutationQueue->AcknowledgeBatch(batch, nanopb::MakeByteString(batchResult.streamToken));
228229
[self applyBatchResult:batchResult];
229230
_mutationQueue->PerformConsistencyCheck();
230231

@@ -244,11 +245,11 @@ - (MaybeDocumentMap)rejectBatchID:(BatchId)batchID {
244245
});
245246
}
246247

247-
- (nullable NSData *)lastStreamToken {
248+
- (ByteString)lastStreamToken {
248249
return _mutationQueue->GetLastStreamToken();
249250
}
250251

251-
- (void)setLastStreamToken:(nullable NSData *)streamToken {
252+
- (void)setLastStreamToken:(const ByteString &)streamToken {
252253
self.persistence.run("Set stream token",
253254
[&]() { _mutationQueue->SetLastStreamToken(streamToken); });
254255
}

Firestore/Source/Remote/FSTSerializerBeta.mm

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -788,9 +788,7 @@ - (GCFSTarget *)encodedTarget:(const QueryData &)queryData {
788788
}
789789

790790
result.targetId = queryData.target_id();
791-
if (!queryData.resume_token().empty()) {
792-
result.resumeToken = MakeNSData(queryData.resume_token());
793-
}
791+
result.resumeToken = MakeNullableNSData(queryData.resume_token());
794792

795793
return result;
796794
}

Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class LevelDbMutationQueue : public MutationQueue {
6565
bool IsEmpty() override;
6666

6767
void AcknowledgeBatch(FSTMutationBatch* batch,
68-
NSData* _Nullable stream_token) override;
68+
const nanopb::ByteString& stream_token) override;
6969

7070
FSTMutationBatch* AddMutationBatch(
7171
const Timestamp& local_write_time,
@@ -95,9 +95,9 @@ class LevelDbMutationQueue : public MutationQueue {
9595

9696
void PerformConsistencyCheck() override;
9797

98-
NSData* _Nullable GetLastStreamToken() override;
98+
nanopb::ByteString GetLastStreamToken() override;
9999

100-
void SetLastStreamToken(NSData* _Nullable stream_token) override;
100+
void SetLastStreamToken(const nanopb::ByteString& stream_token) override;
101101

102102
private:
103103
/**

Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.mm

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "Firestore/core/src/firebase/firestore/local/leveldb_util.h"
2828
#include "Firestore/core/src/firebase/firestore/model/mutation_batch.h"
2929
#include "Firestore/core/src/firebase/firestore/model/resource_path.h"
30+
#include "Firestore/core/src/firebase/firestore/nanopb/nanopb_util.h"
3031
#include "Firestore/core/src/firebase/firestore/util/string_util.h"
3132
#include "Firestore/core/src/firebase/firestore/util/to_string.h"
3233
#include "absl/strings/match.h"
@@ -48,6 +49,9 @@
4849
using model::kBatchIdUnknown;
4950
using model::Mutation;
5051
using model::ResourcePath;
52+
using nanopb::ByteString;
53+
using nanopb::MakeByteString;
54+
using nanopb::MakeNSData;
5155

5256
BatchId LoadNextBatchIdFromDb(DB* db) {
5357
// TODO(gsoltis): implement Prev() and SeekToLast() on
@@ -151,7 +155,7 @@ BatchId LoadNextBatchIdFromDb(DB* db) {
151155
}
152156

153157
void LevelDbMutationQueue::AcknowledgeBatch(FSTMutationBatch* batch,
154-
NSData* _Nullable stream_token) {
158+
const ByteString& stream_token) {
155159
SetLastStreamToken(stream_token);
156160
}
157161

@@ -423,12 +427,12 @@ BatchId LoadNextBatchIdFromDb(DB* db) {
423427
util::ToString(dangling_mutation_references));
424428
}
425429

426-
NSData* _Nullable LevelDbMutationQueue::GetLastStreamToken() {
427-
return metadata_.lastStreamToken;
430+
ByteString LevelDbMutationQueue::GetLastStreamToken() {
431+
return MakeByteString(metadata_.lastStreamToken);
428432
}
429433

430-
void LevelDbMutationQueue::SetLastStreamToken(NSData* _Nullable stream_token) {
431-
metadata_.lastStreamToken = stream_token;
434+
void LevelDbMutationQueue::SetLastStreamToken(const ByteString& stream_token) {
435+
metadata_.lastStreamToken = MakeNullableNSData(stream_token);
432436

433437
db_.currentTransaction->Put(mutation_queue_key(), metadata_);
434438
}

Firestore/core/src/firebase/firestore/local/memory_mutation_queue.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class MemoryMutationQueue : public MutationQueue {
5353
bool IsEmpty() override;
5454

5555
void AcknowledgeBatch(FSTMutationBatch* batch,
56-
NSData* _Nullable stream_token) override;
56+
const nanopb::ByteString& stream_token) override;
5757

5858
FSTMutationBatch* AddMutationBatch(
5959
const Timestamp& local_write_time,
@@ -89,8 +89,8 @@ class MemoryMutationQueue : public MutationQueue {
8989

9090
size_t CalculateByteSize(FSTLocalSerializer* serializer);
9191

92-
NSData* _Nullable GetLastStreamToken() override;
93-
void SetLastStreamToken(NSData* _Nullable token) override;
92+
nanopb::ByteString GetLastStreamToken() override;
93+
void SetLastStreamToken(const nanopb::ByteString& token) override;
9494

9595
private:
9696
using DocumentKeyReferenceSet =
@@ -143,7 +143,7 @@ class MemoryMutationQueue : public MutationQueue {
143143
* responses the client has processed. Stream tokens are opaque checkpoint
144144
* markers whose only real value is their inclusion in the next request.
145145
*/
146-
NSData* _Nullable last_stream_token_;
146+
nanopb::ByteString last_stream_token_;
147147

148148
/** An ordered mapping between documents and the mutation batch IDs. */
149149
DocumentKeyReferenceSet batches_by_document_key_;

Firestore/core/src/firebase/firestore/local/memory_mutation_queue.mm

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
using model::kBatchIdUnknown;
4242
using model::Mutation;
4343
using model::ResourcePath;
44+
using nanopb::ByteString;
4445

4546
MemoryMutationQueue::MemoryMutationQueue(FSTMemoryPersistence* persistence)
4647
: persistence_(persistence) {
@@ -53,7 +54,7 @@
5354
}
5455

5556
void MemoryMutationQueue::AcknowledgeBatch(FSTMutationBatch* batch,
56-
NSData* _Nullable stream_token) {
57+
const ByteString& stream_token) {
5758
HARD_ASSERT(!queue_.empty(), "Cannot acknowledge batch on an empty queue");
5859

5960
// Guaranteed to exist, due to above assert
@@ -262,11 +263,11 @@
262263
return count;
263264
}
264265

265-
NSData* _Nullable MemoryMutationQueue::GetLastStreamToken() {
266+
ByteString MemoryMutationQueue::GetLastStreamToken() {
266267
return last_stream_token_;
267268
}
268269

269-
void MemoryMutationQueue::SetLastStreamToken(NSData* _Nullable token) {
270+
void MemoryMutationQueue::SetLastStreamToken(const ByteString& token) {
270271
last_stream_token_ = token;
271272
}
272273

0 commit comments

Comments
 (0)