Skip to content

Commit 993b299

Browse files
drkannandrkannan
committed
Addressed review comments and added 8 smoke test cases
1 parent 303e66e commit 993b299

File tree

3 files changed

+294
-9
lines changed

3 files changed

+294
-9
lines changed

‎src/ExtCmd.actor.cpp‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -406,15 +406,29 @@ struct DropCollectionCmd {
406406
};
407407
REGISTER_CMD(DropCollectionCmd, "drop");
408408

409-
ACTOR static Future<int> internal_doRenameCollectionIndexesActor(Reference<DocTransaction> tr,
410-
Namespace ns,
411-
Reference<MetadataManager> mm) {
409+
ACTOR static Future<Void> internal_doRenameCollectionIndexesActor(Reference<DocTransaction> tr,
410+
Namespace ns,
411+
Reference<MetadataManager> mm,
412+
std::string destinationCollection) {
413+
state Reference<UnboundCollectionContext> indexesCollection = wait(mm->indexesCollection(tr, ns.first));
414+
state Reference<Plan> indexesPlan = wait(getIndexesForCollectionPlan(ns, tr, mm));
415+
state std::vector<bson::BSONObj> indexes = wait(getIndexesTransactionally(indexesPlan, tr));
416+
state Reference<QueryContext> matchingIndex;
417+
418+
for (const auto& indexObj : indexes) {
419+
matchingIndex = indexesCollection->bindCollectionContext(tr)->cx->getSubContext(
420+
DataValue(indexObj.getField(DocLayerConstants::ID_FIELD)).encode_key_part());
421+
matchingIndex->set(DataValue(DocLayerConstants::NS_FIELD, DVTypeCode::STRING).encode_key_part(),
422+
DataValue(ns.first + "." + destinationCollection, DVTypeCode::STRING).encode_value());
423+
wait(matchingIndex->commitChanges());
424+
}
425+
412426
state Reference<UnboundCollectionContext> unbound = wait(mm->getUnboundCollectionContext(tr, ns));
413427
unbound->bindCollectionContext(tr)->bumpMetadataVersion();
414428
TraceEvent(SevInfo, "BumpMetadataVersion")
415429
.detail("reason", "renameCollection")
416430
.detail("ns", fullCollNameToString(ns));
417-
return true;
431+
return Void();
418432
}
419433

420434
ACTOR static Future<Void> Internal_doRenameCollection(Reference<DocTransaction> tr,
@@ -435,21 +449,25 @@ ACTOR static Future<Void> Internal_doRenameCollection(Reference<DocTransaction>
435449
if (exists_sourceCollectionF.get()) {
436450
if (exists_destinationCollectionF.get()) {
437451
if (dropTarget) {
452+
if (sourceCollection == destinationCollection) {
453+
throw old_and_new_collection_name_cannot_be_same();
454+
}
438455
ns.second = destinationCollection;
439456
state Reference<UnboundCollectionContext> unbound = wait(ec->mm->getUnboundCollectionContext(tr, ns));
440457
wait(success(internal_doDropIndexesActor(tr, ns, ec->mm)));
441458
wait(unbound->collectionDirectory->remove(tr->tr));
459+
} else {
460+
throw collection_name_already_exist();
442461
}
443462
}
444463
} else {
445-
throw directory_does_not_exist();
464+
throw collection_name_does_not_exist();
446465
}
447466

448-
Reference<DirectorySubspace> Collection =
449-
wait(ec->docLayer->rootDirectory->move(tr->tr, {StringRef(ns.first), StringRef(sourceCollection)},
450-
{StringRef(ns.first), StringRef(destinationCollection)}));
467+
wait(success(ec->docLayer->rootDirectory->move(tr->tr, {StringRef(ns.first), StringRef(sourceCollection)},
468+
{StringRef(ns.first), StringRef(destinationCollection)})));
451469
ns.second = sourceCollection;
452-
wait(success(internal_doRenameCollectionIndexesActor(tr, ns, ec->mm)));
470+
wait(success(internal_doRenameCollectionIndexesActor(tr, ns, ec->mm, destinationCollection)));
453471
return Void();
454472
}
455473

‎src/error_definitions.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ DOCLAYER_ERROR(wire_protocol_mismatch, 29966, "Wire protocol mismatch. Bad messa
8585
DOCLAYER_ERROR(no_index_name, 29967, "No index name specified");
8686
DOCLAYER_ERROR(unsupported_index_type, 29969, "Document Layer does not support this index type, yet.");
8787

88+
DOCLAYER_ERROR(collection_name_does_not_exist, 29976, "Collection name does not exist.");
89+
DOCLAYER_ERROR(collection_name_already_exist, 29977, "Collection name already exist.");
90+
DOCLAYER_ERROR(old_and_new_collection_name_cannot_be_same, 29978, "Old and New collection name cannot be same.");
8891
DOCLAYER_ERROR(collection_not_found, 29979, "Collection not found.");
8992
DOCLAYER_ERROR(no_transaction_in_progress, 29980, "No transaction in progress.");
9093
DOCLAYER_ERROR(no_symbol_type, 29981, "The Document Layer does not support the deprecated BSON `symbol` type.");

‎test/correctness/smoke/test_rename_collection.py‎

Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
1+
#!/usr/bin/python
2+
#
3+
# test_rename_collection.py
4+
#
5+
# This source file is part of the FoundationDB open source project
6+
#
7+
# Copyright 2013-2019 Apple Inc. and the FoundationDB project authors
8+
#
9+
# Licensed under the Apache License, Version 2.0 (the "License");
10+
# you may not use this file except in compliance with the License.
11+
# You may obtain a copy of the License at
12+
#
13+
# http://www.apache.org/licenses/LICENSE-2.0
14+
#
15+
# Unless required by applicable law or agreed to in writing, software
16+
# distributed under the License is distributed on an "AS IS" BASIS,
17+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
18+
# See the License for the specific language governing permissions and
19+
# limitations under the License.
20+
#
21+
# MongoDB is a registered trademark of MongoDB, Inc.
22+
#
23+
24+
from pymongo.errors import OperationFailure
25+
26+
# Rename Collection - New name - Without Index
27+
def test_renamecollection_1(fixture_db):
28+
db = fixture_db
29+
30+
# Make sure we first delete collections if it exists
31+
db.drop_collection("src_collection")
32+
db.drop_collection("dest_collection")
33+
34+
# Make sure we first delete record if it exists
35+
db.src_collection.delete_many({})
36+
37+
# Insert one document
38+
db.src_collection.insert_one({'A': 'Hello', 'B': 'World'})
39+
40+
# Query and list collection names
41+
found_collections = db.collection_names()
42+
total_collections = len(found_collections)
43+
assert total_collections == 2, "Expected:2, Found: {}".format(total_collections)
44+
assert found_collections[0] == u'src_collection', "Expected:1, Found: {}".format(found_collections)
45+
assert found_collections[1] == u'system.indexes', "Expected:1, Found: {}".format(found_collections)
46+
47+
# Rename src_collection to dest_collection
48+
db.src_collection.rename("dest_collection",dropTarget=False)
49+
50+
# Query and check collection names after rename
51+
found_collections = db.collection_names()
52+
total_collections = len(found_collections)
53+
assert total_collections == 2, "Expected:2, Found: {}".format(total_collections)
54+
assert found_collections[0] == u'dest_collection', "Expected:1, Found: {}".format(found_collection)
55+
assert found_collections[1] == u'system.indexes', "Expected:1, Found: {}".format(found_collections)
56+
57+
# Query and check results
58+
found = db.dest_collection.find().count()
59+
assert found == 1, "Expected:1, Found: {}".format(found)
60+
61+
# Drop collection
62+
db.drop_collection("dest_collection")
63+
64+
# Rename Collection - Existing name - force_rename=false - Without Index
65+
def test_renamecollection_2(fixture_db):
66+
db = fixture_db
67+
68+
db.src_collection.insert_one({'A': 'Hello', 'B': 'World'})
69+
db.dest_collection.insert_one({'A': 'Hello', 'B': 'California'})
70+
71+
# Query and check error
72+
try:
73+
db.src_collection.rename("dest_collection",dropTarget=False)
74+
except OperationFailure as e:
75+
serverErrObj = e.details
76+
assert serverErrObj['code'] != None
77+
# 29977 : Collection name already exist
78+
assert serverErrObj['code'] == 29977, "Expected:29977, Found: {}".format(serverErrObj)
79+
80+
db.drop_collection("src_collection")
81+
db.drop_collection("dest_collection")
82+
83+
# Rename Collection - Existing name - force_rename=true - Without Index
84+
def test_renamecollection_3(fixture_db):
85+
db = fixture_db
86+
87+
# Make sure we first delete record if it exists
88+
db.src_collection.delete_many({})
89+
db.src_collection.insert_one({'A': 'Hello', 'B': 'World'})
90+
91+
db.dest_collection.delete_many({})
92+
db.dest_collection.insert_one({'A': 'Hello', 'B': 'California'})
93+
94+
found_collections = db.collection_names()
95+
total_collections = len(found_collections)
96+
assert total_collections == 3, "Expected:3, Found: {}".format(total_collections)
97+
assert found_collections[0] == u'dest_collection', "Expected:1, Found: {}".format(found_collections)
98+
assert found_collections[1] == u'src_collection', "Expected:1, Found: {}".format(found_collections)
99+
assert found_collections[2] == u'system.indexes', "Expected:1, Found: {}".format(found_collections)
100+
101+
# Query and force rename
102+
db.src_collection.rename("dest_collection",dropTarget=True)
103+
104+
# Query and check collection names after rename
105+
found_collections = db.collection_names()
106+
total_collections = len(found_collections)
107+
assert total_collections == 2, "Expected:2, Found: {}".format(total_collections)
108+
assert found_collections[0] == u'dest_collection', "Expected:1, Found: {}".format(found_collections)
109+
assert found_collections[1] == u'system.indexes', "Expected:1, Found: {}".format(found_collections)
110+
111+
#Query and check dest_collection has one document
112+
found = db.dest_collection.find().count()
113+
assert found == 1, "Expected:1, Found: {}".format(found)
114+
115+
# Query and check src_collection document moved to dest_collection
116+
found = db.dest_collection.find({'B':'World'}).count()
117+
assert found == 1, "Expected:1, Found: {}".format(found)
118+
119+
db.drop_collection("dest_collection")
120+
121+
# Rename Collection - New name - With Index
122+
def test_renamecollection_4(fixture_db):
123+
db = fixture_db
124+
125+
# Make sure we first delete collections if it exists
126+
db.drop_collection("src_collection")
127+
db.drop_collection("dest_collection")
128+
129+
# Create an index for source collection
130+
db.src_collection.create_index('field_a')
131+
db.src_collection.insert_one({'A': 'Hello', 'B': 'California'})
132+
133+
# Query and check name space in collection index
134+
indexes = db.src_collection.index_information()
135+
src_ns = indexes['field_a_1']['ns']
136+
src_ns_collection = src_ns.split(".")
137+
assert src_ns_collection[1] == u'src_collection', "Expected:1, Found: {}".format(src_ns_collection)
138+
139+
db.src_collection.rename("dest_collection",dropTarget=False)
140+
141+
# Query and check collection indexes updated with dest_collection
142+
indexes = db.dest_collection.index_information()
143+
dest_ns = indexes['field_a_1']['ns']
144+
dest_ns_collection = dest_ns.split(".")
145+
assert dest_ns_collection[1] == u'dest_collection', "Expected:1, Found: {}".format(dest_ns_collection)
146+
147+
# Query and check source collection index
148+
indexes = db.src_collection.index_information()
149+
assert 'field_a_1' not in indexes, "Expected:0, Found: {}".format(indexes)
150+
151+
# Query and check dest_collection has src_collection document
152+
found = db.dest_collection.find({'B': 'California'}).count()
153+
assert found == 1, "Expected:1, Found: {}".format(found)
154+
155+
db.drop_collection("dest_collection")
156+
157+
# Rename Collection - Existing name - force_rename=false - With Index
158+
def test_renamecollection_5(fixture_db):
159+
db = fixture_db
160+
161+
# Create an index for source collection
162+
db.src_collection.create_index('field_a')
163+
db.src_collection.insert_one({'A': 'Hello', 'B': 'World'})
164+
165+
# Create an index for destination collection
166+
db.dest_collection.create_index('field_b')
167+
db.dest_collection.insert_one({'A': 'Hello', 'B': 'California'})
168+
169+
# Query and check error
170+
try:
171+
db.src_collection.rename("dest_collection",dropTarget=False)
172+
except OperationFailure as e:
173+
serverErrObj = e.details
174+
assert serverErrObj['code'] != None
175+
# 29977 : Collection name already exist
176+
assert serverErrObj['code'] == 29977, "Expected:29977, Found: {}".format(serverErrObj)
177+
178+
db.drop_collection("src_collection")
179+
db.drop_collection("dest_collection")
180+
181+
# Rename Collection - Existing name - force_rename=true - With Index
182+
def test_renamecollection_6(fixture_db):
183+
db = fixture_db
184+
185+
db.src_collection.delete_many({})
186+
187+
# Create an index for source collection
188+
db.src_collection.create_index('field_a')
189+
db.src_collection.insert_one({'A': 'Hello', 'B': 'World'})
190+
191+
# Query and get source collection id
192+
indexes = db.src_collection.index_information()
193+
source_object_id = indexes['field_a_1']['_id']
194+
195+
db.dest_collection.delete_many({})
196+
197+
# Create an index for destination collection
198+
db.dest_collection.create_index('field_b')
199+
db.dest_collection.insert_one({'A': 'Hello', 'B': 'California'})
200+
201+
# Query and check collection names
202+
found_collections = db.collection_names()
203+
total_collections = len(found_collections)
204+
assert total_collections == 3, "Expected:3, Found: {}".format(total_collections)
205+
assert found_collections[0] == u'dest_collection', "Expected:1, Found: {}".format(found_collections)
206+
assert found_collections[1] == u'src_collection', "Expected:1, Found: {}".format(found_collections)
207+
assert found_collections[2] == u'system.indexes', "Expected:1, Found: {}".format(found_collections)
208+
209+
# Query and force rename
210+
db.src_collection.rename("dest_collection",dropTarget=True)
211+
212+
# Query and check collection names after rename
213+
found_collections = db.collection_names()
214+
total_collections = len(found_collections)
215+
assert total_collections == 2, "Expected:2, Found: {}".format(total_collections)
216+
assert found_collections[0] == u'dest_collection', "Expected:1, Found: {}".format(found_collections)
217+
assert found_collections[1] == u'system.indexes', "Expected:1, Found: {}".format(found_collections)
218+
219+
# Query and get destination collection id
220+
indexes = db.dest_collection.index_information()
221+
destination_object_id = indexes['field_a_1']['_id']
222+
223+
# After rename, check dest_collection updated with src_collection id
224+
assert source_object_id == destination_object_id, "Expected:1, Found: {}".format(destination_object_id)
225+
226+
indexes = db.src_collection.index_information()
227+
assert 'field_a_1' not in indexes, "Expected:0, Found: {}".format(indexes)
228+
229+
found = db.dest_collection.find({'B': 'World'}).count()
230+
assert found == 1, "Expected:1, Found: {}".format(found)
231+
232+
db.drop_collection("dest_collection")
233+
234+
# Rename Collection - Check Error - force_rename=false - Without Index
235+
def test_renamecollection_7(fixture_db):
236+
db = fixture_db
237+
238+
# Rename and check error, when source collection doesn't exist
239+
try:
240+
db.src_collection.rename("dest_collection",dropTarget=False)
241+
except OperationFailure as e:
242+
serverErrObj = e.details
243+
assert serverErrObj['code'] != None
244+
# 29976 : Collection name does not exist
245+
assert serverErrObj['code'] == 29976, "Expected:29976, Found: {}".format(serverErrObj)
246+
247+
# Rename Collection - Existing name - Check Error - force_rename=True - With Index
248+
def test_renamecollection_8(fixture_db):
249+
db = fixture_db
250+
251+
# Create an index for source collection
252+
db.src_collection.create_index('field_a')
253+
db.src_collection.insert_one({'A': 'Hello', 'B': 'World'})
254+
255+
# Query and check error if src and dest collection names are same
256+
try:
257+
db.src_collection.rename("src_collection",dropTarget=True)
258+
except OperationFailure as e:
259+
serverErrObj = e.details
260+
assert serverErrObj['code'] != None
261+
# 29978 : Old and New collection name cannot be same
262+
assert serverErrObj['code'] == 29978, "Expected:29978, Found: {}".format(serverErrObj)
263+
264+
db.drop_collection("src_collection")

0 commit comments

Comments
 (0)