Skip to content

Commit e6296af

Browse files
jimwwalkerdaverigby
jimwwalker
authored and
daverigby
committed
MB-16181: Change setCollections to take a CollectionsManifest
Change from passing a string, to passing the more usable CollectionsManifest type. This allows future updates to setCollections to inspect the given manifest without string/json parsing. Change-Id: I79b131b14cc1dbf5996e4deddbae58de7fa39c22 Reviewed-on: http://review.couchbase.org/c/kv_engine/+/143057 Tested-by: Build Bot <[email protected]> Reviewed-by: Paolo Cocchi <[email protected]>
1 parent 91468bd commit e6296af

8 files changed

+153
-203
lines changed

‎engines/ep/tests/module_tests/collections/evp_store_collections_dcp_test.cc‎

Lines changed: 81 additions & 117 deletions
Large diffs are not rendered by default.

‎engines/ep/tests/module_tests/collections/evp_store_collections_test.cc‎

Lines changed: 39 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -55,46 +55,38 @@
5555

5656
TEST_P(CollectionsParameterizedTest, uid_increment) {
5757
CollectionsManifest cm{CollectionEntry::meat};
58-
EXPECT_EQ(setCollections(cookie, std::string{cm}),
59-
cb::engine_errc::success);
58+
EXPECT_EQ(setCollections(cookie, cm), cb::engine_errc::success);
6059
cm.add(CollectionEntry::vegetable);
61-
EXPECT_EQ(setCollections(cookie, std::string{cm}),
62-
cb::engine_errc::success);
60+
EXPECT_EQ(setCollections(cookie, cm), cb::engine_errc::success);
6361
}
6462

6563
TEST_P(CollectionsParameterizedTest, uid_decrement) {
6664
CollectionsManifest cm{CollectionEntry::meat};
67-
EXPECT_EQ(setCollections(cookie, std::string{cm}),
68-
cb::engine_errc::success);
65+
EXPECT_EQ(setCollections(cookie, cm), cb::engine_errc::success);
6966
CollectionsManifest newCm{};
70-
setCollections(cookie,
71-
std::string{newCm},
72-
cb::engine_errc::cannot_apply_collections_manifest);
67+
setCollections(
68+
cookie, newCm, cb::engine_errc::cannot_apply_collections_manifest);
7369
}
7470

7571
TEST_P(CollectionsParameterizedTest, uid_equal) {
7672
CollectionsManifest cm{CollectionEntry::meat};
77-
EXPECT_EQ(setCollections(cookie, std::string{cm}),
78-
cb::engine_errc::success);
73+
EXPECT_EQ(setCollections(cookie, cm), cb::engine_errc::success);
7974

8075
// An equal manifest is tolerated (and ignored)
81-
EXPECT_EQ(setCollections(cookie, std::string{cm}),
82-
cb::engine_errc::success);
76+
EXPECT_EQ(setCollections(cookie, cm), cb::engine_errc::success);
8377
}
8478

8579
TEST_P(CollectionsParameterizedTest, manifest_uid_equal_with_differences) {
8680
CollectionsManifest cm{CollectionEntry::meat};
87-
EXPECT_EQ(setCollections(cookie, std::string{cm}),
88-
cb::engine_errc::success);
81+
EXPECT_EQ(setCollections(cookie, cm), cb::engine_errc::success);
8982

9083
auto uid = cm.getUid();
9184
cm.add(CollectionEntry::fruit);
9285
// force the uid back
9386
cm.updateUid(uid);
9487
// manifest is equal, but contains an extra collection, unexpected diversion
95-
setCollections(cookie,
96-
std::string{cm},
97-
cb::engine_errc::cannot_apply_collections_manifest);
88+
setCollections(
89+
cookie, cm, cb::engine_errc::cannot_apply_collections_manifest);
9890
}
9991

10092
// This test stores a key which matches what collections internally uses, but
@@ -398,8 +390,7 @@ TEST_P(CollectionsParameterizedTest, get_collection_id) {
398390
cm.add(CollectionEntry::dairy);
399391
cm.add(ScopeEntry::shop2);
400392
cm.add(CollectionEntry::meat, ScopeEntry::shop2);
401-
std::string json = cm;
402-
setCollections(cookie, json);
393+
setCollections(cookie, cm);
403394
// Check bad 'paths'
404395
auto rv = store->getCollectionID("");
405396
EXPECT_EQ(cb::engine_errc::invalid_arguments, rv.result);
@@ -455,8 +446,7 @@ TEST_P(CollectionsParameterizedTest, get_collection_id) {
455446

456447
// Now we should fail getting _default
457448
cm.remove(CollectionEntry::defaultC);
458-
json = cm;
459-
setCollections(cookie, json);
449+
setCollections(cookie, cm);
460450
rv = store->getCollectionID(".");
461451
EXPECT_EQ(cb::engine_errc::unknown_collection, rv.result);
462452
rv = store->getCollectionID("._default");
@@ -469,8 +459,7 @@ TEST_P(CollectionsParameterizedTest, get_scope_id) {
469459
cm.add(CollectionEntry::dairy, ScopeEntry::shop1);
470460
cm.add(ScopeEntry::shop2);
471461
cm.add(CollectionEntry::meat, ScopeEntry::shop2);
472-
std::string json = cm;
473-
setCollections(cookie, json);
462+
setCollections(cookie, cm);
474463

475464
// Check bad 'paths', require 0 or 1 dot
476465
auto rv = store->getScopeID("..");
@@ -1278,7 +1267,7 @@ TEST_F(CollectionsWarmupTest, MB_38125) {
12781267

12791268
// cookie now notified and setCollections can go ahead
12801269
EXPECT_EQ(ENGINE_SUCCESS, cookie_to_mock_cookie(cookie)->status);
1281-
setCollections(cookie, std::string{cm});
1270+
setCollections(cookie, cm);
12821271

12831272
auto vb = store->getVBucket(vbid);
12841273

@@ -1316,7 +1305,7 @@ TEST_F(CollectionsWarmupTest, LockedVBStateDuringManifestUpdate) {
13161305

13171306
// cookie now notified and setCollections can go ahead
13181307
EXPECT_EQ(ENGINE_SUCCESS, cookie_to_mock_cookie(cookie)->status);
1319-
setCollections(cookie, std::string{cm});
1308+
setCollections(cookie, cm);
13201309
}
13211310

13221311
/**
@@ -1331,7 +1320,7 @@ TEST_P(CollectionsParameterizedTest, basic) {
13311320
}
13321321

13331322
CollectionsManifest cm(CollectionEntry::meat);
1334-
setCollections(cookie, std::string{cm});
1323+
setCollections(cookie, cm);
13351324

13361325
// Check all vbuckets got the collections
13371326
for (int vb = vbid.get(); vb <= (vbid.get() + extraVbuckets); vb++) {
@@ -1360,7 +1349,7 @@ TEST_P(CollectionsParameterizedTest, basic2) {
13601349
}
13611350

13621351
CollectionsManifest cm(CollectionEntry::meat);
1363-
setCollections(cookie, std::string{cm});
1352+
setCollections(cookie, cm);
13641353

13651354
// Check all vbuckets got the collections
13661355
for (int vb = vbid.get(); vb <= (vbid.get() + extraVbuckets); vb++) {
@@ -1703,8 +1692,7 @@ TEST_F(CollectionsTest, CollectionStatsIncludesScope) {
17031692
cm.add(ScopeEntry::shop2);
17041693
cm.add(CollectionEntry::meat, ScopeEntry::shop2);
17051694
cm.add(CollectionEntry::fruit, ScopeEntry::shop2);
1706-
std::string json = cm;
1707-
setCollections(cookie, json);
1695+
setCollections(cookie, cm);
17081696

17091697
KVBucketTest::flushVBucketToDiskIfPersistent(vbid, 5);
17101698

@@ -2180,7 +2168,7 @@ TEST_F(CollectionsTest, GetScopeIdForGivenKeyNoVbid) {
21802168
manifest.add(ScopeEntry::shop1)
21812169
.add(CollectionEntry::dairy, ScopeEntry::shop1);
21822170

2183-
setCollections(cookie, std::string{manifest});
2171+
setCollections(cookie, manifest);
21842172
flush_vbucket_to_disk(vbid, 2);
21852173

21862174
StoredDocKey keyDefault{"default", CollectionEntry::defaultC};
@@ -2271,7 +2259,7 @@ TEST_F(CollectionsTest, GetAllKeysNonCollectionConnection) {
22712259

22722260
// Add the meat collection
22732261
CollectionsManifest cm(CollectionEntry::meat);
2274-
setCollections(cookie, std::string{cm});
2262+
setCollections(cookie, cm);
22752263
flushVBucketToDiskIfPersistent(vbid, 1);
22762264

22772265
// Create and flush items for the default and meat collections
@@ -2294,7 +2282,7 @@ TEST_F(CollectionsTest, GetAllKeysNonCollectionConnectionMaxCountTen) {
22942282

22952283
// Add the meat collection
22962284
CollectionsManifest cm(CollectionEntry::meat);
2297-
setCollections(cookie, std::string{cm});
2285+
setCollections(cookie, cm);
22982286
flushVBucketToDiskIfPersistent(vbid, 1);
22992287

23002288
// Create and flush items for the default collection
@@ -2330,7 +2318,7 @@ TEST_F(CollectionsTest, GetAllKeysStartHalfWayForCollection) {
23302318

23312319
// Add the meat collection
23322320
CollectionsManifest cm(CollectionEntry::meat);
2333-
setCollections(cookie, std::string{cm});
2321+
setCollections(cookie, cm);
23342322
flushVBucketToDiskIfPersistent(vbid, 1);
23352323

23362324
store_items(
@@ -2358,7 +2346,7 @@ TEST_F(CollectionsTest, GetAllKeysForCollectionEmptyKey) {
23582346

23592347
// Add the meat collection
23602348
CollectionsManifest cm(CollectionEntry::meat);
2361-
setCollections(cookie, std::string{cm});
2349+
setCollections(cookie, cm);
23622350
flushVBucketToDiskIfPersistent(vbid, 1);
23632351

23642352
store_items(
@@ -2404,7 +2392,7 @@ TEST_F(CollectionsTest, GetAllKeysCollectionConnection) {
24042392

24052393
// Add the meat collection
24062394
CollectionsManifest cm(CollectionEntry::meat);
2407-
setCollections(cookie, std::string{cm});
2395+
setCollections(cookie, cm);
24082396
flushVBucketToDiskIfPersistent(vbid, 1);
24092397

24102398
// Create and flush items for the default and meat collections
@@ -2484,7 +2472,7 @@ TEST_F(CollectionsTest, TestGetKeyStatsBadVbids) {
24842472
TEST_F(CollectionsTest, TestGetKeyStats) {
24852473
// Add the meat collection
24862474
CollectionsManifest cm(CollectionEntry::meat);
2487-
setCollections(cookie, std::string{cm});
2475+
setCollections(cookie, cm);
24882476
flushVBucketToDiskIfPersistent(vbid, 1);
24892477

24902478
store_item(vbid,
@@ -2536,7 +2524,7 @@ TEST_F(CollectionsTest, TestGetKeyStats) {
25362524
TEST_F(CollectionsTest, TestGetVKeyStats) {
25372525
// Add the meat collection
25382526
CollectionsManifest cm(CollectionEntry::meat);
2539-
setCollections(cookie, std::string{cm});
2527+
setCollections(cookie, cm);
25402528
flushVBucketToDiskIfPersistent(vbid, 1);
25412529

25422530
store_item(vbid,
@@ -2651,7 +2639,7 @@ TEST_F(CollectionsRbacTest, GetAllKeysRbacCollectionConnection) {
26512639
// Add the meat and dairy collections
26522640
CollectionsManifest cm(CollectionEntry::meat);
26532641
cm.add(CollectionEntry::dairy);
2654-
setCollections(cookie, std::string{cm});
2642+
setCollections(cookie, cm);
26552643
flushVBucketToDiskIfPersistent(vbid, 2);
26562644

26572645
// Create and flush items for the default and meat collections
@@ -2701,7 +2689,7 @@ TEST_F(CollectionsRbacTest, TestKeyStats) {
27012689
mock_set_collections_support(cookie, true);
27022690
// Add the meat collection
27032691
CollectionsManifest cm(CollectionEntry::meat);
2704-
setCollections(cookie, std::string{cm});
2692+
setCollections(cookie, cm);
27052693
flushVBucketToDiskIfPersistent(vbid, 1);
27062694

27072695
store_item(vbid,
@@ -2748,7 +2736,7 @@ TEST_F(CollectionsRbacTest, TestVKeyStats) {
27482736
mock_set_collections_support(cookie, true);
27492737
// Add the meat collection
27502738
CollectionsManifest cm(CollectionEntry::meat);
2751-
setCollections(cookie, std::string{cm});
2739+
setCollections(cookie, cm);
27522740
flushVBucketToDiskIfPersistent(vbid, 1);
27532741

27542742
store_item(vbid,
@@ -2808,28 +2796,28 @@ TEST_P(CollectionsPersistentParameterizedTest, SystemEventsDoNotCount) {
28082796
CollectionsManifest cm;
28092797
cm.add(ScopeEntry::shop1);
28102798
cm.add(CollectionEntry::fruit).add(CollectionEntry::meat);
2811-
setCollections(cookie, std::string{cm});
2799+
setCollections(cookie, cm);
28122800
flushVBucketToDiskIfPersistent(vbid, 3);
28132801

28142802
// Now get the engine warmed up
28152803
resetEngineAndWarmup();
28162804
{ EXPECT_EQ(0, store->getVBucket(vbid)->getNumTotalItems()); }
28172805
cm.remove(CollectionEntry::meat);
2818-
setCollections(cookie, std::string{cm});
2806+
setCollections(cookie, cm);
28192807
flushVBucketToDiskIfPersistent(vbid, 1);
28202808

28212809
resetEngineAndWarmup();
28222810
{ EXPECT_EQ(0, store->getVBucket(vbid)->getNumTotalItems()); }
28232811

28242812
cm.remove(CollectionEntry::fruit);
2825-
setCollections(cookie, std::string{cm});
2813+
setCollections(cookie, cm);
28262814
flushVBucketToDiskIfPersistent(vbid, 1);
28272815

28282816
resetEngineAndWarmup();
28292817
{ EXPECT_EQ(0, store->getVBucket(vbid)->getNumTotalItems()); }
28302818

28312819
cm.remove(CollectionEntry::defaultC);
2832-
setCollections(cookie, std::string{cm});
2820+
setCollections(cookie, cm);
28332821
flushVBucketToDiskIfPersistent(vbid, 1);
28342822

28352823
resetEngineAndWarmup();
@@ -2840,7 +2828,7 @@ TEST_P(CollectionsParameterizedTest, ScopeIDIsValid) {
28402828
CollectionsManifest cm;
28412829
cm.add(CollectionEntry::fruit);
28422830
cm.add(ScopeEntry::shop1);
2843-
setCollections(cookie, std::string{cm});
2831+
setCollections(cookie, cm);
28442832
flushVBucketToDiskIfPersistent(vbid, 2);
28452833

28462834
auto& manager = getCollectionsManager();
@@ -2864,7 +2852,7 @@ TEST_P(CollectionsParameterizedTest, OneScopeStatsByIdParsing) {
28642852
CollectionsManifest cm;
28652853
cm.add(CollectionEntry::fruit);
28662854
cm.add(ScopeEntry::shop1);
2867-
setCollections(cookie, std::string{cm});
2855+
setCollections(cookie, cm);
28682856
flushVBucketToDiskIfPersistent(vbid, 2);
28692857

28702858
auto& manager = getCollectionsManager();
@@ -2894,16 +2882,16 @@ TEST_P(CollectionsParameterizedTest, OneScopeStatsByIdParsing) {
28942882
TEST_P(CollectionsPersistentParameterizedTest, FlushDropCreateDropCleansUp) {
28952883
CollectionsManifest cm;
28962884
cm.add(CollectionEntry::fruit); // seq:1
2897-
setCollections(cookie, std::string{cm});
2885+
setCollections(cookie, cm);
28982886
flushVBucketToDiskIfPersistent(vbid, 1);
28992887
cm.remove(CollectionEntry::fruit); // seq:2
2900-
setCollections(cookie, std::string{cm});
2888+
setCollections(cookie, cm);
29012889
flushVBucketToDiskIfPersistent(vbid, 1);
29022890
cm.add(CollectionEntry::fruit); // seq:3
2903-
setCollections(cookie, std::string{cm});
2891+
setCollections(cookie, cm);
29042892
flushVBucketToDiskIfPersistent(vbid, 1);
29052893
cm.remove(CollectionEntry::fruit); // seq:4
2906-
setCollections(cookie, std::string{cm});
2894+
setCollections(cookie, cm);
29072895
flushVBucketToDiskIfPersistent(vbid, 1);
29082896

29092897
VBucketPtr vb = store->getVBucket(vbid);

‎engines/ep/tests/module_tests/collections/evp_store_durability_collections_dcp_test.cc‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,8 @@ TEST_P(CollectionsSyncWriteParamTest,
7777
seqno_advanced_one_mutation_plus_pending) {
7878
VBucketPtr vb = store->getVBucket(vbid);
7979
CollectionsManifest cm{};
80-
setCollections(
81-
cookie,
82-
std::string{
83-
cm.add(CollectionEntry::meat).add(CollectionEntry::dairy)});
80+
setCollections(cookie,
81+
cm.add(CollectionEntry::meat).add(CollectionEntry::dairy));
8482
// filter only CollectionEntry::dairy
8583
createDcpObjects({{R"({"collections":["c"]})"}});
8684

@@ -116,7 +114,7 @@ TEST_P(CollectionsSyncWriteParamTest,
116114
TEST_P(CollectionsSyncWriteParamTest, drop_collection_with_pending_write) {
117115
VBucketPtr vb = store->getVBucket(vbid);
118116
CollectionsManifest cm{};
119-
setCollections(cookie, std::string{cm.add(CollectionEntry::dairy)});
117+
setCollections(cookie, cm.add(CollectionEntry::dairy));
120118

121119
store_item(vbid, StoredDocKey{"milk", CollectionEntry::defaultC}, "milk");
122120

@@ -148,7 +146,7 @@ TEST_P(CollectionsSyncWriteParamTest, drop_collection_with_pending_write) {
148146
flush_vbucket_to_disk(replicaVB, 4);
149147
}
150148

151-
setCollections(cookie, std::string{cm.remove(CollectionEntry::dairy)});
149+
setCollections(cookie, cm.remove(CollectionEntry::dairy));
152150
notifyAndStepToCheckpoint();
153151
EXPECT_EQ(ENGINE_SUCCESS,
154152
producer->stepAndExpect(*producers,
@@ -188,7 +186,7 @@ failover_entry_t CollectionsSyncWriteParamTest::
188186
testCompleteDifferentPrepareOnActiveBeforeReplicaDropSetUp() {
189187
VBucketPtr vb = store->getVBucket(vbid);
190188
CollectionsManifest cm{};
191-
setCollections(cookie, std::string{cm.add(CollectionEntry::dairy)});
189+
setCollections(cookie, cm.add(CollectionEntry::dairy));
192190

193191
auto item = makePendingItem(StoredDocKey{"cream", CollectionEntry::dairy},
194192
"value");
@@ -212,7 +210,7 @@ failover_entry_t CollectionsSyncWriteParamTest::
212210
cb::mcbp::ClientOpcode::DcpPrepare));
213211
flushVBucketToDiskIfPersistent(replicaVB, 3);
214212

215-
setCollections(cookie, std::string{cm.remove(CollectionEntry::dairy)});
213+
setCollections(cookie, cm.remove(CollectionEntry::dairy));
216214

217215
// Get DCP ready, but don't step the drop event yet
218216
notifyAndStepToCheckpoint();

0 commit comments

Comments
 (0)