Skip to content

Commit 07a5291

Browse files
rozzarozza
authored
Ensure use of generateIdIfAbsentFromDocument assigns the result
 There is no guarantee that CollectibleCodec<TDocument> will be able to mutate the TDocument instance. JAVA-2272
1 parent ad3612a commit 07a5291

File tree

6 files changed

+317
-2
lines changed

6 files changed

+317
-2
lines changed

‎driver-async/src/main/com/mongodb/async/client/MongoCollectionImpl.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ public void bulkWrite(final List<? extends WriteModel<? extends TDocument>> requ
270270
} else if (writeModel instanceof InsertOneModel) {
271271
TDocument document = ((InsertOneModel<TDocument>) writeModel).getDocument();
272272
if (getCodec() instanceof CollectibleCodec) {
273-
((CollectibleCodec<TDocument>) getCodec()).generateIdIfAbsentFromDocument(document);
273+
document = ((CollectibleCodec<TDocument>) getCodec()).generateIdIfAbsentFromDocument(document);
274274
}
275275
writeRequest = new InsertRequest(documentToBsonDocument(document));
276276
} else if (writeModel instanceof ReplaceOneModel) {
@@ -322,7 +322,7 @@ public void insertOne(final TDocument document, final SingleResultCallback<Void>
322322
public void insertOne(final TDocument document, final InsertOneOptions options, final SingleResultCallback<Void> callback) {
323323
TDocument insertDocument = document;
324324
if (getCodec() instanceof CollectibleCodec) {
325-
((CollectibleCodec<TDocument>) getCodec()).generateIdIfAbsentFromDocument(insertDocument);
325+
insertDocument = ((CollectibleCodec<TDocument>) getCodec()).generateIdIfAbsentFromDocument(insertDocument);
326326
}
327327
executeSingleWriteRequest(new InsertRequest(documentToBsonDocument(insertDocument)), options.getBypassDocumentValidation(),
328328
new SingleResultCallback<BulkWriteResult>() {

‎driver-async/src/test/unit/com/mongodb/async/client/MongoCollectionSpecification.groovy

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ import com.mongodb.bulk.IndexRequest
3838
import com.mongodb.bulk.InsertRequest
3939
import com.mongodb.bulk.UpdateRequest
4040
import com.mongodb.bulk.WriteConcernError
41+
import com.mongodb.client.ImmutableDocument
42+
import com.mongodb.client.ImmutableDocumentCodecProvider
4143
import com.mongodb.client.model.BulkWriteOptions
4244
import com.mongodb.client.model.Collation
4345
import com.mongodb.client.model.CountOptions
@@ -96,6 +98,7 @@ import static com.mongodb.bulk.WriteRequest.Type.REPLACE
9698
import static com.mongodb.bulk.WriteRequest.Type.UPDATE
9799
import static java.util.concurrent.TimeUnit.MILLISECONDS
98100
import static org.bson.codecs.configuration.CodecRegistries.fromProviders
101+
import static org.bson.codecs.configuration.CodecRegistries.fromRegistries
99102
import static spock.util.matcher.HamcrestSupport.expect
100103

101104
@SuppressWarnings('ClassSize')
@@ -1204,4 +1207,52 @@ class MongoCollectionSpecification extends Specification {
12041207
expect operation, isTheSameAs(expectedOperation)
12051208
}
12061209

1210+
def 'should not expect to mutate the document when inserting'() {
1211+
given:
1212+
def executor = new TestOperationExecutor([null])
1213+
def customCodecRegistry = fromRegistries(codecRegistry, fromProviders(new ImmutableDocumentCodecProvider()))
1214+
def collection = new MongoCollectionImpl(namespace, ImmutableDocument, customCodecRegistry, readPreference, writeConcern,
1215+
readConcern, executor)
1216+
def document = new ImmutableDocument(['a': 1])
1217+
def futureResultCallback = new FutureResultCallback<Void>()
1218+
1219+
when:
1220+
collection.insertOne(document, futureResultCallback)
1221+
futureResultCallback.get()
1222+
1223+
then:
1224+
!document.containsKey('_id')
1225+
1226+
when:
1227+
def operation = executor.getWriteOperation() as MixedBulkWriteOperation
1228+
def request = operation.writeRequests.get(0) as InsertRequest
1229+
1230+
then:
1231+
request.getDocument().containsKey('_id')
1232+
}
1233+
1234+
def 'should not expect to mutate the document when bulk writing'() {
1235+
given:
1236+
def executor = new TestOperationExecutor([null])
1237+
def customCodecRegistry = fromRegistries(codecRegistry, fromProviders(new ImmutableDocumentCodecProvider()))
1238+
def collection = new MongoCollectionImpl(namespace, ImmutableDocument, customCodecRegistry, readPreference, writeConcern,
1239+
readConcern, executor)
1240+
def document = new ImmutableDocument(['a': 1])
1241+
def futureResultCallback = new FutureResultCallback<BulkWriteResult>()
1242+
1243+
when:
1244+
collection.bulkWrite([new InsertOneModel<ImmutableDocument>(document)], futureResultCallback)
1245+
futureResultCallback.get()
1246+
1247+
then:
1248+
!document.containsKey('_id')
1249+
1250+
when:
1251+
def operation = executor.getWriteOperation() as MixedBulkWriteOperation
1252+
def request = operation.writeRequests.get(0) as InsertRequest
1253+
1254+
then:
1255+
request.getDocument().containsKey('_id')
1256+
}
1257+
12071258
}

‎driver-core/src/test/unit/com/mongodb/client/ImmutableDocument.java

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Copyright 2016 MongoDB, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.mongodb.client;
18+
19+
import org.bson.BsonDocument;
20+
import org.bson.BsonDocumentWrapper;
21+
import org.bson.codecs.configuration.CodecRegistry;
22+
import org.bson.conversions.Bson;
23+
24+
import java.io.Serializable;
25+
import java.util.Collection;
26+
import java.util.Collections;
27+
import java.util.LinkedHashMap;
28+
import java.util.Map;
29+
import java.util.Set;
30+
31+
public final class ImmutableDocument implements Map<String, Object>, Serializable, Bson {
32+
private final Map<String, Object> immutableDocument;
33+
34+
/**
35+
* Creates a Document instance initialized with the given map.
36+
*
37+
* @param map initial map
38+
*/
39+
public ImmutableDocument(final Map<String, Object> map) {
40+
immutableDocument = Collections.unmodifiableMap(new LinkedHashMap<String, Object>(map));
41+
}
42+
43+
44+
@Override
45+
public int size() {
46+
return immutableDocument.size();
47+
}
48+
49+
@Override
50+
public boolean isEmpty() {
51+
return immutableDocument.isEmpty();
52+
}
53+
54+
@Override
55+
public boolean containsKey(final Object key) {
56+
return immutableDocument.containsKey(key);
57+
}
58+
59+
@Override
60+
public boolean containsValue(final Object value) {
61+
return immutableDocument.containsValue(value);
62+
}
63+
64+
@Override
65+
public Object get(final Object key) {
66+
return immutableDocument.get(key);
67+
}
68+
69+
@Override
70+
public Object put(final String key, final Object value) {
71+
throw new UnsupportedOperationException();
72+
}
73+
74+
@Override
75+
public Object remove(final Object key) {
76+
throw new UnsupportedOperationException();
77+
}
78+
79+
@Override
80+
public void putAll(final Map<? extends String, ?> m) {
81+
throw new UnsupportedOperationException();
82+
}
83+
84+
@Override
85+
public void clear() {
86+
throw new UnsupportedOperationException();
87+
}
88+
89+
@Override
90+
public Set<String> keySet() {
91+
return immutableDocument.keySet();
92+
}
93+
94+
@Override
95+
public Collection<Object> values() {
96+
return immutableDocument.values();
97+
}
98+
99+
@Override
100+
public Set<Entry<String, Object>> entrySet() {
101+
return immutableDocument.entrySet();
102+
}
103+
104+
@Override
105+
public <TDocument> BsonDocument toBsonDocument(final Class<TDocument> tDocumentClass, final CodecRegistry codecRegistry) {
106+
return new BsonDocumentWrapper<ImmutableDocument>(this, codecRegistry.get(ImmutableDocument.class));
107+
}
108+
}

‎driver-core/src/test/unit/com/mongodb/client/ImmutableDocumentCodec.java

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Copyright 2016 MongoDB, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.mongodb.client;
18+
19+
import org.bson.BsonReader;
20+
import org.bson.BsonValue;
21+
import org.bson.BsonWriter;
22+
import org.bson.Document;
23+
import org.bson.codecs.CollectibleCodec;
24+
import org.bson.codecs.DecoderContext;
25+
import org.bson.codecs.EncoderContext;
26+
import org.bson.codecs.configuration.CodecRegistry;
27+
import org.bson.types.ObjectId;
28+
29+
import java.util.LinkedHashMap;
30+
31+
import static java.lang.String.format;
32+
33+
public final class ImmutableDocumentCodec implements CollectibleCodec<ImmutableDocument> {
34+
private final CodecRegistry codecRegistry;
35+
private static final String ID_FIELD_NAME = "_id";
36+
37+
public ImmutableDocumentCodec(final CodecRegistry codecRegistry) {
38+
this.codecRegistry = codecRegistry;
39+
}
40+
41+
@Override
42+
public ImmutableDocument generateIdIfAbsentFromDocument(final ImmutableDocument document) {
43+
LinkedHashMap<String, Object> mutable = new LinkedHashMap<String, Object>(document);
44+
mutable.put(ID_FIELD_NAME, new ObjectId());
45+
return new ImmutableDocument(mutable);
46+
}
47+
48+
@Override
49+
public boolean documentHasId(final ImmutableDocument document) {
50+
return document.containsKey(ID_FIELD_NAME);
51+
}
52+
53+
@Override
54+
public BsonValue getDocumentId(final ImmutableDocument document) {
55+
if (!documentHasId(document)) {
56+
throw new IllegalStateException(format("The document does not contain an %s", ID_FIELD_NAME));
57+
}
58+
return document.toBsonDocument(ImmutableDocument.class, codecRegistry).get(ID_FIELD_NAME);
59+
}
60+
61+
@Override
62+
public void encode(final BsonWriter writer, final ImmutableDocument value, final EncoderContext encoderContext) {
63+
codecRegistry.get(Document.class).encode(writer, new Document(value), encoderContext);
64+
}
65+
66+
@Override
67+
public Class<ImmutableDocument> getEncoderClass() {
68+
return ImmutableDocument.class;
69+
}
70+
71+
@Override
72+
public ImmutableDocument decode(final BsonReader reader, final DecoderContext decoderContext) {
73+
Document document = codecRegistry.get(Document.class).decode(reader, decoderContext);
74+
return new ImmutableDocument(document);
75+
}
76+
}

‎driver-core/src/test/unit/com/mongodb/client/ImmutableDocumentCodecProvider.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright 2016 MongoDB, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.mongodb.client;
18+
19+
import org.bson.codecs.Codec;
20+
import org.bson.codecs.configuration.CodecProvider;
21+
import org.bson.codecs.configuration.CodecRegistry;
22+
23+
public final class ImmutableDocumentCodecProvider implements CodecProvider {
24+
@Override
25+
@SuppressWarnings("unchecked")
26+
public <T> Codec<T> get(final Class<T> clazz, final CodecRegistry registry) {
27+
if (clazz.equals(ImmutableDocument.class)) {
28+
return (Codec<T>) new ImmutableDocumentCodec(registry);
29+
}
30+
return null;
31+
}
32+
}

‎driver/src/test/unit/com/mongodb/MongoCollectionSpecification.groovy

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import com.mongodb.bulk.DeleteRequest
2020
import com.mongodb.bulk.IndexRequest
2121
import com.mongodb.bulk.InsertRequest
2222
import com.mongodb.bulk.UpdateRequest
23+
import com.mongodb.client.ImmutableDocument
24+
import com.mongodb.client.ImmutableDocumentCodecProvider
2325
import com.mongodb.client.model.BulkWriteOptions
2426
import com.mongodb.client.model.Collation
2527
import com.mongodb.client.model.CountOptions
@@ -78,8 +80,10 @@ import static com.mongodb.bulk.WriteRequest.Type.REPLACE
7880
import static com.mongodb.bulk.WriteRequest.Type.UPDATE
7981
import static java.util.concurrent.TimeUnit.MILLISECONDS
8082
import static org.bson.codecs.configuration.CodecRegistries.fromProviders
83+
import static org.bson.codecs.configuration.CodecRegistries.fromRegistries
8184
import static spock.util.matcher.HamcrestSupport.expect
8285

86+
@SuppressWarnings('ClassSize')
8387
class MongoCollectionSpecification extends Specification {
8488

8589
def namespace = new MongoNamespace('databaseName', 'collectionName')
@@ -1056,4 +1060,48 @@ class MongoCollectionSpecification extends Specification {
10561060
expect operation, isTheSameAs(expectedOperation)
10571061
}
10581062

1063+
def 'should not expect to mutate the document when inserting'() {
1064+
given:
1065+
def executor = new TestOperationExecutor([null])
1066+
def customCodecRegistry = fromRegistries(codecRegistry, fromProviders(new ImmutableDocumentCodecProvider()))
1067+
def collection = new MongoCollectionImpl(namespace, ImmutableDocument, customCodecRegistry, readPreference, writeConcern,
1068+
readConcern, executor)
1069+
def document = new ImmutableDocument(['a': 1])
1070+
1071+
when:
1072+
collection.insertOne(document)
1073+
1074+
then:
1075+
!document.containsKey('_id')
1076+
1077+
when:
1078+
def operation = executor.getWriteOperation() as MixedBulkWriteOperation
1079+
def request = operation.writeRequests.get(0) as InsertRequest
1080+
1081+
then:
1082+
request.getDocument().containsKey('_id')
1083+
}
1084+
1085+
def 'should not expect to mutate the document when bulk writing'() {
1086+
given:
1087+
def executor = new TestOperationExecutor([null])
1088+
def customCodecRegistry = fromRegistries(codecRegistry, fromProviders(new ImmutableDocumentCodecProvider()))
1089+
def collection = new MongoCollectionImpl(namespace, ImmutableDocument, customCodecRegistry, readPreference, writeConcern,
1090+
readConcern, executor)
1091+
def document = new ImmutableDocument(['a': 1])
1092+
1093+
when:
1094+
collection.bulkWrite([new InsertOneModel<ImmutableDocument>(document)])
1095+
1096+
then:
1097+
!document.containsKey('_id')
1098+
1099+
when:
1100+
def operation = executor.getWriteOperation() as MixedBulkWriteOperation
1101+
def request = operation.writeRequests.get(0) as InsertRequest
1102+
1103+
then:
1104+
request.getDocument().containsKey('_id')
1105+
}
1106+
10591107
}

0 commit comments

Comments
 (0)