Skip to content

Commit 766a600

Browse files
rozzarozza
committed
TypeData no longer validates data.
With the introduction of PropertyCodecProviders the TypeData class no longer should validate the data it sees for Collections or Maps. This changes behavior for "invalid Maps" to a possible runtime CodecConfigurationException. However, it allows users to register their own PropertyCodecProviders to handle these types if desired. The side effect of this change is @BsonIgnore and conventions can now be used to ignore properties regardless of validity. JAVA-2620
1 parent 8b35118 commit 766a600

14 files changed

+405
-178
lines changed

‎bson/src/main/org/bson/codecs/pojo/InvalidMapImplementation.java

Lines changed: 0 additions & 84 deletions
This file was deleted.

‎bson/src/main/org/bson/codecs/pojo/MapPropertyCodecProvider.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,18 @@
2727
import java.util.Map;
2828
import java.util.Map.Entry;
2929

30+
import static java.lang.String.format;
31+
3032
final class MapPropertyCodecProvider implements PropertyCodecProvider {
33+
3134
@SuppressWarnings({"rawtypes", "unchecked"})
3235
@Override
3336
public <T> Codec<T> get(final TypeWithTypeParameters<T> type, final PropertyCodecRegistry registry) {
3437
if (Map.class.isAssignableFrom(type.getType()) && type.getTypeParameters().size() == 2) {
38+
Class<?> keyType = type.getTypeParameters().get(0).getType();
39+
if (!keyType.equals(String.class)) {
40+
throw new CodecConfigurationException(format("Invalid Map type. Maps MUST have string keys, found %s instead.", keyType));
41+
}
3542
return new MapCodec(type.getType(), registry.get(type.getTypeParameters().get(1)));
3643
} else {
3744
return null;

‎bson/src/main/org/bson/codecs/pojo/TypeData.java

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import java.lang.reflect.Type;
2323
import java.lang.reflect.TypeVariable;
2424
import java.util.ArrayList;
25-
import java.util.Collection;
2625
import java.util.Collections;
2726
import java.util.HashMap;
2827
import java.util.List;
@@ -146,54 +145,10 @@ public Builder<T> addTypeParameters(final List<TypeData<?>> typeParameters) {
146145
* @return the class type data
147146
*/
148147
public TypeData<T> build() {
149-
validate();
150148
return new TypeData<T>(type, Collections.unmodifiableList(typeParameters));
151149
}
152-
153-
private void validate() {
154-
if (Collection.class.isAssignableFrom(type)) {
155-
if (typeParameters.size() == 0) {
156-
for (Type interfaceType : type.getGenericInterfaces()) {
157-
if (interfaceType instanceof ParameterizedType) {
158-
ParameterizedType pType = (ParameterizedType) interfaceType;
159-
if (Collection.class.equals((Class<?>) pType.getRawType())) {
160-
Type rawListType = pType.getActualTypeArguments()[0];
161-
if (!(rawListType instanceof Class<?>)) {
162-
throw new IllegalStateException("Invalid Collection type. Collections must have a defined type.");
163-
}
164-
}
165-
}
166-
}
167-
} else if (typeParameters.size() > 1) {
168-
throw new IllegalStateException("Invalid Collection type. Collections must have a single type parameter defined.");
169-
}
170-
} else if (Map.class.isAssignableFrom(type)) {
171-
Class<?> keyType = Object.class;
172-
if (typeParameters.size() != 2) {
173-
for (Type interfaceType : type.getGenericInterfaces()) {
174-
if (interfaceType instanceof ParameterizedType) {
175-
ParameterizedType pType = (ParameterizedType) interfaceType;
176-
if (Map.class.equals((Class<?>) pType.getRawType())) {
177-
Type rawKeyType = pType.getActualTypeArguments()[0];
178-
if (!(rawKeyType instanceof Class<?>)) {
179-
throw new IllegalStateException("Invalid Map type. Maps MUST have string keys");
180-
} else {
181-
keyType = (Class<?>) rawKeyType;
182-
}
183-
}
184-
}
185-
}
186-
} else {
187-
keyType = typeParameters.get(0).getType();
188-
}
189-
if (!keyType.equals(String.class)) {
190-
throw new IllegalStateException(format("Invalid Map type. Maps MUST have string keys, found %s instead.", keyType));
191-
}
192-
}
193-
}
194150
}
195151

196-
197152
@Override
198153
public String toString() {
199154
String typeParams = typeParameters.isEmpty() ? ""

‎bson/src/test/unit/org/bson/codecs/pojo/ClassModelBuilderTest.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import org.bson.codecs.pojo.annotations.BsonProperty;
2121
import org.bson.codecs.pojo.entities.ConcreteCollectionsModel;
2222
import org.bson.codecs.pojo.entities.GenericHolderModel;
23-
import org.bson.codecs.pojo.entities.InvalidMapModel;
2423
import org.bson.codecs.pojo.entities.NestedGenericHolderModel;
2524
import org.bson.codecs.pojo.entities.SimpleGenericsModel;
2625
import org.bson.codecs.pojo.entities.UpperBoundsModel;
@@ -159,11 +158,6 @@ public void testValidationDuplicateDocumentFieldName() {
159158
builder.build();
160159
}
161160

162-
@Test(expected = IllegalStateException.class)
163-
public void testIllegalMapKey() {
164-
ClassModel.builder(InvalidMapModel.class).build();
165-
}
166-
167161
private static final List<Annotation> TEST_ANNOTATIONS = Collections.<Annotation>singletonList(
168162
new BsonProperty() {
169163
@Override

‎bson/src/test/unit/org/bson/codecs/pojo/PojoCustomTest.java

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@
2828
import org.bson.codecs.pojo.entities.ConverterModel;
2929
import org.bson.codecs.pojo.entities.CustomPropertyCodecOptionalModel;
3030
import org.bson.codecs.pojo.entities.GenericTreeModel;
31+
import org.bson.codecs.pojo.entities.InvalidCollectionModel;
3132
import org.bson.codecs.pojo.entities.InvalidGetterAndSetterModel;
33+
import org.bson.codecs.pojo.entities.InvalidMapModel;
34+
import org.bson.codecs.pojo.entities.InvalidMapPropertyCodecProvider;
3235
import org.bson.codecs.pojo.entities.InvalidSetterArgsModel;
3336
import org.bson.codecs.pojo.entities.Optional;
3437
import org.bson.codecs.pojo.entities.OptionalPropertyCodecProviderForTest;
@@ -51,12 +54,14 @@
5154
import java.util.Map;
5255

5356
import static java.lang.String.format;
57+
import static java.util.Arrays.asList;
5458
import static org.bson.codecs.configuration.CodecRegistries.fromCodecs;
5559
import static org.bson.codecs.configuration.CodecRegistries.fromProviders;
5660
import static org.bson.codecs.configuration.CodecRegistries.fromRegistries;
5761
import static org.bson.codecs.pojo.Conventions.NO_CONVENTIONS;
5862

5963
public final class PojoCustomTest extends PojoTestCase {
64+
6065
@Test
6166
public void testRegisterClassModelPreferredOverClass() {
6267
ClassModel<SimpleModel> classModel = ClassModel.builder(SimpleModel.class).enableDiscriminator(true).build();
@@ -267,21 +272,42 @@ public void testCanHandleTopLevelGenericIfHasCodec() {
267272
@Test
268273
public void testCustomRegisteredPropertyCodecWithValue() {
269274
CustomPropertyCodecOptionalModel model = new CustomPropertyCodecOptionalModel(Optional.of("foo"));
270-
ClassModelBuilder<CustomPropertyCodecOptionalModel> classModel =
271-
ClassModel.builder(CustomPropertyCodecOptionalModel.class);
272-
273-
roundTrip(getPojoCodecProviderBuilder(classModel).register(new OptionalPropertyCodecProviderForTest()), model,
274-
"{'optionalField': 'foo'}");
275+
roundTrip(getPojoCodecProviderBuilder(CustomPropertyCodecOptionalModel.class).register(new OptionalPropertyCodecProviderForTest()),
276+
model, "{'optionalField': 'foo'}");
275277
}
276278

277279
@Test
278280
public void testCustomRegisteredPropertyCodecOmittedValue() {
279281
CustomPropertyCodecOptionalModel model = new CustomPropertyCodecOptionalModel(Optional.<String>empty());
280-
ClassModelBuilder<CustomPropertyCodecOptionalModel> classModel =
281-
ClassModel.builder(CustomPropertyCodecOptionalModel.class);
282+
roundTrip(getPojoCodecProviderBuilder(CustomPropertyCodecOptionalModel.class).register(new OptionalPropertyCodecProviderForTest()),
283+
model, "{'optionalField': null}");
284+
}
285+
286+
@Test(expected = CodecConfigurationException.class)
287+
public void testEncodingInvalidMapModel() {
288+
encodesTo(getPojoCodecProviderBuilder(InvalidMapModel.class), getInvalidMapModel(), "{'invalidMap': {'1': 1, '2': 2}}");
289+
}
290+
291+
@Test(expected = CodecConfigurationException.class)
292+
public void testDecodingInvalidMapModel() {
293+
decodingShouldFail(getCodec(InvalidMapModel.class), "{'invalidMap': {'1': 1, '2': 2}}");
294+
}
295+
296+
@Test(expected = CodecConfigurationException.class)
297+
public void testEncodingInvalidCollectionModel() {
298+
encodesTo(getPojoCodecProviderBuilder(InvalidCollectionModel.class), new InvalidCollectionModel(asList(1, 2, 3)),
299+
"{collectionField: [1, 2, 3]}");
300+
}
282301

283-
roundTrip(getPojoCodecProviderBuilder(classModel).register(new OptionalPropertyCodecProviderForTest()), model,
284-
"{'optionalField': null}");
302+
@Test(expected = CodecConfigurationException.class)
303+
public void testDecodingInvalidCollectionModel() {
304+
decodingShouldFail(getCodec(InvalidCollectionModel.class), "{collectionField: [1, 2, 3]}");
305+
}
306+
307+
@Test
308+
public void testInvalidMapModelWithCustomPropertyCodecProvider() {
309+
encodesTo(getPojoCodecProviderBuilder(InvalidMapModel.class).register(new InvalidMapPropertyCodecProvider()), getInvalidMapModel(),
310+
"{'invalidMap': {'1': 1, '2': 2}}");
285311
}
286312

287313
@Test(expected = CodecConfigurationException.class)

‎bson/src/test/unit/org/bson/codecs/pojo/PojoRoundTripTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import org.bson.codecs.pojo.entities.SimpleModel;
6060
import org.bson.codecs.pojo.entities.SimpleNestedPojoModel;
6161
import org.bson.codecs.pojo.entities.UpperBoundsConcreteModel;
62+
import org.bson.codecs.pojo.entities.conventions.BsonIgnoreInvalidMapModel;
6263
import org.bson.codecs.pojo.entities.conventions.CollectionDiscriminatorModel;
6364
import org.bson.codecs.pojo.entities.conventions.CreatorAllFinalFieldsModel;
6465
import org.bson.codecs.pojo.entities.conventions.CreatorConstructorIdModel;
@@ -120,6 +121,10 @@ private static List<TestData> testCases() {
120121
+ "'child': {'_id': 'child', 'myFinalField': 10, 'myIntField': 10,"
121122
+ "'model': {'integerField': 42, 'stringField': 'myString'}}}"));
122123

124+
data.add(new TestData("BsonIgnore invalid map", new BsonIgnoreInvalidMapModel("myString"),
125+
getPojoCodecProviderBuilder(BsonIgnoreInvalidMapModel.class),
126+
"{stringField: 'myString'}"));
127+
123128
data.add(new TestData("Interfaced based model", new InterfaceModelImpl("a", "b"),
124129
getPojoCodecProviderBuilder(InterfaceModelImpl.class),
125130
"{'propertyA': 'a', 'propertyB': 'b'}"));

‎bson/src/test/unit/org/bson/codecs/pojo/PojoTestCase.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.bson.codecs.pojo.entities.ConventionModel;
3535
import org.bson.codecs.pojo.entities.GenericHolderModel;
3636
import org.bson.codecs.pojo.entities.GenericTreeModel;
37+
import org.bson.codecs.pojo.entities.InvalidMapModel;
3738
import org.bson.codecs.pojo.entities.MultipleBoundsModel;
3839
import org.bson.codecs.pojo.entities.NestedGenericHolderFieldWithMultipleTypeParamsModel;
3940
import org.bson.codecs.pojo.entities.NestedGenericHolderMapModel;
@@ -339,6 +340,13 @@ static GenericTreeModel<String, String> getGenericTreeModelStrings() {
339340
new GenericTreeModel<String, String>("left", "5", null, null), null));
340341
}
341342

343+
static InvalidMapModel getInvalidMapModel() {
344+
Map<Integer, Integer> map = new HashMap<Integer, Integer>();
345+
map.put(1, 1);
346+
map.put(2, 2);
347+
return new InvalidMapModel(map);
348+
}
349+
342350
static final String SIMPLE_MODEL_JSON = "{'integerField': 42, 'stringField': 'myString'}";
343351

344352
class StringToObjectIdCodec implements Codec<String> {

‎bson/src/test/unit/org/bson/codecs/pojo/TypeDataTest.java

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -77,37 +77,4 @@ public void testRecursiveTypeData() {
7777

7878
typeData.toString();
7979
}
80-
81-
@Test(expected = IllegalStateException.class)
82-
public void testListNoParamsValidation() {
83-
TypeData.builder(List.class).build();
84-
}
85-
86-
@Test(expected = IllegalStateException.class)
87-
public void testListToManyParamsValidation() {
88-
TypeData<String> stringTypeData = TypeData.builder(String.class).build();
89-
TypeData.builder(List.class).addTypeParameter(stringTypeData).addTypeParameter(stringTypeData).build();
90-
}
91-
92-
@Test(expected = IllegalStateException.class)
93-
public void testMapNoParamsValidation() {
94-
TypeData.builder(Map.class).build();
95-
}
96-
97-
@Test(expected = IllegalStateException.class)
98-
public void testMapKeyValidation() {
99-
TypeData.builder(Map.class).addTypeParameter(TypeData.builder(Integer.class).build()).build();
100-
}
101-
102-
@Test(expected = IllegalStateException.class)
103-
public void testInvalidMapImplementationValidation() {
104-
TypeData.builder(InvalidMapImplementation.class).build();
105-
}
106-
107-
@Test(expected = IllegalStateException.class)
108-
public void testMapToManyParamsValidation() {
109-
TypeData<String> stringTypeData = TypeData.builder(String.class).build();
110-
TypeData.builder(Map.class).addTypeParameter(stringTypeData).addTypeParameter(stringTypeData)
111-
.addTypeParameter(stringTypeData).build();
112-
}
11380
}

‎bson/src/test/unit/org/bson/codecs/pojo/entities/ConcreteInterfaceGenericModel.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ public void setPropertyA(final String property) {
3838

3939
@Override
4040
public boolean equals(final Object o) {
41-
if (this == o) return true;
41+
if (this == o) {
42+
return true;
43+
}
4244
if (o == null || getClass() != o.getClass()) {
4345
return false;
4446
}

0 commit comments

Comments
 (0)