Skip to content

Commit 4c27c83

Browse files
committed
Implement review feedback
1 parent e0fa3d7 commit 4c27c83

File tree

5 files changed

+133
-111
lines changed

5 files changed

+133
-111
lines changed

Firestore/core/src/firebase/firestore/local/leveldb_key.cc

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ bool ReadDocumentKey(leveldb::Slice *contents, DocumentKey *result) {
321321
leveldb::Slice complete_segments = *contents;
322322

323323
std::string segment;
324-
std::vector<std::string> path_segments{};
324+
std::vector<std::string> path_segments;
325325
for (;;) {
326326
// Advance a temporary slice to avoid advancing contents into the next key
327327
// component which may not be a path segment.
@@ -343,7 +343,7 @@ bool ReadDocumentKey(leveldb::Slice *contents, DocumentKey *result) {
343343
ResourcePath path{std::move(path_segments)};
344344
if (path.size() > 0 && DocumentKey::IsDocumentKey(path)) {
345345
*contents = complete_segments;
346-
*result = DocumentKey(std::move(path));
346+
*result = DocumentKey{std::move(path)};
347347
return true;
348348
}
349349

@@ -370,37 +370,37 @@ inline bool ReadTableNameMatching(leveldb::Slice *contents,
370370
expected_table_name);
371371
}
372372

373-
inline void WriteBatchID(std::string *dest, model::BatchId batch_id) {
373+
inline void WriteBatchId(std::string *dest, model::BatchId batch_id) {
374374
WriteLabeledInt32(dest, ComponentLabel::BatchId, batch_id);
375375
}
376376

377-
inline bool ReadBatchID(leveldb::Slice *contents, model::BatchId *batch_id) {
377+
inline bool ReadBatchId(leveldb::Slice *contents, model::BatchId *batch_id) {
378378
return ReadLabeledInt32(contents, ComponentLabel::BatchId, batch_id);
379379
}
380380

381-
inline void WriteCanonicalID(std::string *dest,
381+
inline void WriteCanonicalId(std::string *dest,
382382
absl::string_view canonical_id) {
383383
WriteLabeledString(dest, ComponentLabel::CanonicalId, canonical_id);
384384
}
385385

386-
inline bool ReadCanonicalID(leveldb::Slice *contents,
386+
inline bool ReadCanonicalId(leveldb::Slice *contents,
387387
std::string *canonical_id) {
388388
return ReadLabeledString(contents, ComponentLabel::CanonicalId, canonical_id);
389389
}
390390

391-
inline void WriteTargetID(std::string *dest, model::TargetId target_id) {
391+
inline void WriteTargetId(std::string *dest, model::TargetId target_id) {
392392
WriteLabeledInt32(dest, ComponentLabel::TargetId, target_id);
393393
}
394394

395-
inline bool ReadTargetID(leveldb::Slice *contents, model::TargetId *target_id) {
395+
inline bool ReadTargetId(leveldb::Slice *contents, model::TargetId *target_id) {
396396
return ReadLabeledInt32(contents, ComponentLabel::TargetId, target_id);
397397
}
398398

399-
inline void WriteUserID(std::string *dest, absl::string_view user_id) {
399+
inline void WriteUserId(std::string *dest, absl::string_view user_id) {
400400
WriteLabeledString(dest, ComponentLabel::UserId, user_id);
401401
}
402402

403-
inline bool ReadUserID(leveldb::Slice *contents, std::string *user_id) {
403+
inline bool ReadUserId(leveldb::Slice *contents, std::string *user_id) {
404404
return ReadLabeledString(contents, ComponentLabel::UserId, user_id);
405405
}
406406

@@ -457,28 +457,28 @@ std::string Describe(leveldb::Slice key) {
457457

458458
} else if (label == ComponentLabel::BatchId) {
459459
model::BatchId batch_id;
460-
if (!ReadBatchID(&tmp, &batch_id)) {
460+
if (!ReadBatchId(&tmp, &batch_id)) {
461461
break;
462462
}
463463
absl::StrAppend(&description, " batch_id=", batch_id);
464464

465465
} else if (label == ComponentLabel::CanonicalId) {
466466
std::string canonical_id;
467-
if (!ReadCanonicalID(&tmp, &canonical_id)) {
467+
if (!ReadCanonicalId(&tmp, &canonical_id)) {
468468
break;
469469
}
470470
absl::StrAppend(&description, " canonical_id=", canonical_id);
471471

472472
} else if (label == ComponentLabel::TargetId) {
473473
model::TargetId target_id;
474-
if (!ReadTargetID(&tmp, &target_id)) {
474+
if (!ReadTargetId(&tmp, &target_id)) {
475475
break;
476476
}
477477
absl::StrAppend(&description, " target_id=", target_id);
478478

479479
} else if (label == ComponentLabel::UserId) {
480480
std::string user_id;
481-
if (!ReadUserID(&tmp, &user_id)) {
481+
if (!ReadUserId(&tmp, &user_id)) {
482482
break;
483483
}
484484
absl::StrAppend(&description, " user_id=", user_id);
@@ -518,25 +518,26 @@ std::string LevelDbMutationKey::KeyPrefix() {
518518
std::string LevelDbMutationKey::KeyPrefix(absl::string_view user_id) {
519519
std::string result;
520520
WriteTableName(&result, kMutationsTable);
521-
WriteUserID(&result, user_id);
521+
WriteUserId(&result, user_id);
522522
return result;
523523
}
524524

525525
std::string LevelDbMutationKey::Key(absl::string_view user_id,
526526
model::BatchId batch_id) {
527527
std::string result;
528528
WriteTableName(&result, kMutationsTable);
529-
WriteUserID(&result, user_id);
530-
WriteBatchID(&result, batch_id);
529+
WriteUserId(&result, user_id);
530+
WriteBatchId(&result, batch_id);
531531
WriteTerminator(&result);
532532
return result;
533533
}
534534

535535
bool LevelDbMutationKey::Decode(leveldb::Slice key) {
536536
user_id_.clear();
537+
batch_id_ = 0;
537538

538539
return ReadTableNameMatching(&key, kMutationsTable) &&
539-
ReadUserID(&key, &user_id_) && ReadBatchID(&key, &batch_id_) &&
540+
ReadUserId(&key, &user_id_) && ReadBatchId(&key, &batch_id_) &&
540541
ReadTerminator(&key);
541542
}
542543

@@ -549,15 +550,15 @@ std::string LevelDbDocumentMutationKey::KeyPrefix() {
549550
std::string LevelDbDocumentMutationKey::KeyPrefix(absl::string_view user_id) {
550551
std::string result;
551552
WriteTableName(&result, kDocumentMutationsTable);
552-
WriteUserID(&result, user_id);
553+
WriteUserId(&result, user_id);
553554
return result;
554555
}
555556

556557
std::string LevelDbDocumentMutationKey::KeyPrefix(
557558
absl::string_view user_id, const ResourcePath &resource_path) {
558559
std::string result;
559560
WriteTableName(&result, kDocumentMutationsTable);
560-
WriteUserID(&result, user_id);
561+
WriteUserId(&result, user_id);
561562
WriteResourcePath(&result, resource_path);
562563
return result;
563564
}
@@ -567,20 +568,21 @@ std::string LevelDbDocumentMutationKey::Key(absl::string_view user_id,
567568
model::BatchId batch_id) {
568569
std::string result;
569570
WriteTableName(&result, kDocumentMutationsTable);
570-
WriteUserID(&result, user_id);
571+
WriteUserId(&result, user_id);
571572
WriteResourcePath(&result, document_key.path());
572-
WriteBatchID(&result, batch_id);
573+
WriteBatchId(&result, batch_id);
573574
WriteTerminator(&result);
574575
return result;
575576
}
576577

577578
bool LevelDbDocumentMutationKey::Decode(leveldb::Slice key) {
578579
user_id_.clear();
579580
document_key_ = DocumentKey{};
581+
batch_id_ = 0;
580582

581583
return ReadTableNameMatching(&key, kDocumentMutationsTable) &&
582-
ReadUserID(&key, &user_id_) && ReadDocumentKey(&key, &document_key_) &&
583-
ReadBatchID(&key, &batch_id_) && ReadTerminator(&key);
584+
ReadUserId(&key, &user_id_) && ReadDocumentKey(&key, &document_key_) &&
585+
ReadBatchId(&key, &batch_id_) && ReadTerminator(&key);
584586
}
585587

586588
std::string LevelDbMutationQueueKey::KeyPrefix() {
@@ -592,7 +594,7 @@ std::string LevelDbMutationQueueKey::KeyPrefix() {
592594
std::string LevelDbMutationQueueKey::Key(absl::string_view user_id) {
593595
std::string result;
594596
WriteTableName(&result, kMutationQueuesTable);
595-
WriteUserID(&result, user_id);
597+
WriteUserId(&result, user_id);
596598
WriteTerminator(&result);
597599
return result;
598600
}
@@ -601,7 +603,7 @@ bool LevelDbMutationQueueKey::Decode(leveldb::Slice key) {
601603
user_id_.clear();
602604

603605
return ReadTableNameMatching(&key, kMutationQueuesTable) &&
604-
ReadUserID(&key, &user_id_) && ReadTerminator(&key);
606+
ReadUserId(&key, &user_id_) && ReadTerminator(&key);
605607
}
606608

607609
std::string LevelDbTargetGlobalKey::Key() {
@@ -625,14 +627,15 @@ std::string LevelDbTargetKey::KeyPrefix() {
625627
std::string LevelDbTargetKey::Key(model::TargetId target_id) {
626628
std::string result;
627629
WriteTableName(&result, kTargetsTable);
628-
WriteTargetID(&result, target_id);
630+
WriteTargetId(&result, target_id);
629631
WriteTerminator(&result);
630632
return result;
631633
}
632634

633635
bool LevelDbTargetKey::Decode(leveldb::Slice key) {
636+
target_id_ = 0;
634637
return ReadTableNameMatching(&key, kTargetsTable) &&
635-
ReadTargetID(&key, &target_id_) && ReadTerminator(&key);
638+
ReadTargetId(&key, &target_id_) && ReadTerminator(&key);
636639
}
637640

638641
std::string LevelDbQueryTargetKey::KeyPrefix() {
@@ -644,26 +647,27 @@ std::string LevelDbQueryTargetKey::KeyPrefix() {
644647
std::string LevelDbQueryTargetKey::KeyPrefix(absl::string_view canonical_id) {
645648
std::string result;
646649
WriteTableName(&result, kQueryTargetsTable);
647-
WriteCanonicalID(&result, canonical_id);
650+
WriteCanonicalId(&result, canonical_id);
648651
return result;
649652
}
650653

651654
std::string LevelDbQueryTargetKey::Key(absl::string_view canonical_id,
652655
model::TargetId target_id) {
653656
std::string result;
654657
WriteTableName(&result, kQueryTargetsTable);
655-
WriteCanonicalID(&result, canonical_id);
656-
WriteTargetID(&result, target_id);
658+
WriteCanonicalId(&result, canonical_id);
659+
WriteTargetId(&result, target_id);
657660
WriteTerminator(&result);
658661
return result;
659662
}
660663

661664
bool LevelDbQueryTargetKey::Decode(leveldb::Slice key) {
662665
canonical_id_.clear();
666+
target_id_ = 0;
663667

664668
return ReadTableNameMatching(&key, kQueryTargetsTable) &&
665-
ReadCanonicalID(&key, &canonical_id_) &&
666-
ReadTargetID(&key, &target_id_) && ReadTerminator(&key);
669+
ReadCanonicalId(&key, &canonical_id_) &&
670+
ReadTargetId(&key, &target_id_) && ReadTerminator(&key);
667671
}
668672

669673
std::string LevelDbTargetDocumentKey::KeyPrefix() {
@@ -675,25 +679,26 @@ std::string LevelDbTargetDocumentKey::KeyPrefix() {
675679
std::string LevelDbTargetDocumentKey::KeyPrefix(model::TargetId target_id) {
676680
std::string result;
677681
WriteTableName(&result, kTargetDocumentsTable);
678-
WriteTargetID(&result, target_id);
682+
WriteTargetId(&result, target_id);
679683
return result;
680684
}
681685

682686
std::string LevelDbTargetDocumentKey::Key(model::TargetId target_id,
683687
const DocumentKey &document_key) {
684688
std::string result;
685689
WriteTableName(&result, kTargetDocumentsTable);
686-
WriteTargetID(&result, target_id);
690+
WriteTargetId(&result, target_id);
687691
WriteResourcePath(&result, document_key.path());
688692
WriteTerminator(&result);
689693
return result;
690694
}
691695

692696
bool LevelDbTargetDocumentKey::Decode(leveldb::Slice key) {
697+
target_id_ = 0;
693698
document_key_ = DocumentKey{};
694699

695700
return ReadTableNameMatching(&key, kTargetDocumentsTable) &&
696-
ReadTargetID(&key, &target_id_) &&
701+
ReadTargetId(&key, &target_id_) &&
697702
ReadDocumentKey(&key, &document_key_) && ReadTerminator(&key);
698703
}
699704

@@ -716,17 +721,18 @@ std::string LevelDbDocumentTargetKey::Key(const DocumentKey &document_key,
716721
std::string result;
717722
WriteTableName(&result, kDocumentTargetsTable);
718723
WriteResourcePath(&result, document_key.path());
719-
WriteTargetID(&result, target_id);
724+
WriteTargetId(&result, target_id);
720725
WriteTerminator(&result);
721726
return result;
722727
}
723728

724729
bool LevelDbDocumentTargetKey::Decode(leveldb::Slice key) {
725730
document_key_ = DocumentKey{};
731+
target_id_ = 0;
726732

727733
return ReadTableNameMatching(&key, kDocumentTargetsTable) &&
728734
ReadDocumentKey(&key, &document_key_) &&
729-
ReadTargetID(&key, &target_id_) && ReadTerminator(&key);
735+
ReadTargetId(&key, &target_id_) && ReadTerminator(&key);
730736
}
731737

732738
std::string LevelDbRemoteDocumentKey::KeyPrefix() {

0 commit comments

Comments
 (0)