Skip to content

Commit adcc137

Browse files
olim7tAlexandre Dutra
olim7t
authored and
Alexandre Dutra
committed
JAVA-884: Fix UDT mapper to process fields in the correct order.
1 parent 99de433 commit adcc137

5 files changed

Lines changed: 98 additions & 65 deletions

File tree

‎changelog/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
- [new feature] JAVA-606: Codec support for Java 8.
2020
- [new feature] JAVA-565: Codec support for Java arrays.
2121
- [new feature] JAVA-605: Codec support for Java enums.
22+
- [bug] JAVA-884: Fix UDT mapper to process fields in the correct order.
2223

2324
Merged from 2.1 branch:
2425

‎driver-mapping/src/main/java/com/datastax/driver/mapping/AnnotationParser.java‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,11 @@
1818
import java.lang.annotation.Annotation;
1919
import java.lang.reflect.Field;
2020
import java.lang.reflect.*;
21-
import java.util.ArrayList;
22-
import java.util.Collections;
23-
import java.util.Comparator;
24-
import java.util.List;
21+
import java.util.*;
2522
import java.util.concurrent.atomic.AtomicInteger;
2623

2724
import com.google.common.base.Strings;
25+
import com.google.common.collect.Maps;
2826
import com.google.common.reflect.TypeToken;
2927

3028
import com.datastax.driver.core.ConsistencyLevel;
@@ -106,12 +104,22 @@ public static <T> EntityMapper<T> parseEntity(Class<T> entityClass, EntityMapper
106104
validateOrder(pks, "@PartitionKey");
107105
validateOrder(ccs, "@ClusteringColumn");
108106

109-
mapper.addColumns(convert(pks, factory, mapper.entityClass, mappingManager, columnCounter),
110-
convert(ccs, factory, mapper.entityClass, mappingManager, columnCounter),
111-
convert(rgs, factory, mapper.entityClass, mappingManager, columnCounter));
107+
mapper.addColumns(createColumnMappers(pks, factory, mapper.entityClass, mappingManager, columnCounter),
108+
createColumnMappers(ccs, factory, mapper.entityClass, mappingManager, columnCounter),
109+
createColumnMappers(rgs, factory, mapper.entityClass, mappingManager, columnCounter));
112110
return mapper;
113111
}
114112

113+
private static <T> List<ColumnMapper<T>> createColumnMappers(List<Field> fields, EntityMapper.Factory factory, Class<T> klass, MappingManager mappingManager, AtomicInteger columnCounter) {
114+
List<ColumnMapper<T>> mappers = new ArrayList<ColumnMapper<T>>(fields.size());
115+
for (int i = 0; i < fields.size(); i++) {
116+
Field field = fields.get(i);
117+
int pos = position(field);
118+
mappers.add(factory.createColumnMapper(klass, field, pos < 0 ? i : pos, mappingManager, columnCounter));
119+
}
120+
return mappers;
121+
}
122+
115123
public static <T> MappedUDTCodec<T> parseUDT(Class<T> udtClass, EntityMapper.Factory factory, MappingManager mappingManager) {
116124
UDT udt = AnnotationChecks.getTypeAnnotation(UDT.class, udtClass);
117125

@@ -153,16 +161,17 @@ public static <T> MappedUDTCodec<T> parseUDT(Class<T> udtClass, EntityMapper.Fac
153161
break;
154162
}
155163
}
156-
List<ColumnMapper<T>> columnMappers = convert(columns, factory, udtClass, mappingManager, null);
164+
Map<String, ColumnMapper<T>> columnMappers = createFieldMappers(columns, factory, udtClass, mappingManager, null);
157165
return new MappedUDTCodec<T>(userType, udtClass, columnMappers, mappingManager);
158166
}
159167

160-
private static <T> List<ColumnMapper<T>> convert(List<Field> fields, EntityMapper.Factory factory, Class<T> klass, MappingManager mappingManager, AtomicInteger columnCounter) {
161-
List<ColumnMapper<T>> mappers = new ArrayList<ColumnMapper<T>>(fields.size());
168+
private static <T> Map<String, ColumnMapper<T>> createFieldMappers(List<Field> fields, EntityMapper.Factory factory, Class<T> klass, MappingManager mappingManager, AtomicInteger columnCounter) {
169+
Map<String, ColumnMapper<T>> mappers = Maps.newHashMapWithExpectedSize(fields.size());
162170
for (int i = 0; i < fields.size(); i++) {
163171
Field field = fields.get(i);
164172
int pos = position(field);
165-
mappers.add(factory.createColumnMapper(klass, field, pos < 0 ? i : pos, mappingManager, columnCounter));
173+
ColumnMapper<T> mapper = factory.createColumnMapper(klass, field, pos < 0 ? i : pos, mappingManager, columnCounter);
174+
mappers.put(mapper.getColumnName(), mapper);
166175
}
167176
return mappers;
168177
}

‎driver-mapping/src/main/java/com/datastax/driver/mapping/ColumnMapper.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@
2020

2121
import com.google.common.reflect.TypeToken;
2222

23+
import com.datastax.driver.core.Metadata;
2324
import com.datastax.driver.core.TypeCodec;
2425

25-
import static com.datastax.driver.core.querybuilder.QueryBuilder.quote;
26-
2726
abstract class ColumnMapper<T> {
2827

2928
public enum Kind {PARTITION_KEY, CLUSTERING_COLUMN, REGULAR, COMPUTED};
@@ -57,7 +56,7 @@ protected ColumnMapper(Field field, int position, AtomicInteger columnCounter) {
5756
public String getColumnName() {
5857
return kind == Kind.COMPUTED
5958
? columnName
60-
: quote(columnName);
59+
: Metadata.quote(columnName);
6160
}
6261

6362
public String getAlias() {

‎driver-mapping/src/main/java/com/datastax/driver/mapping/MappedUDTCodec.java‎

Lines changed: 38 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import java.nio.ByteBuffer;
1919
import java.util.List;
20+
import java.util.Map;
2021

2122
import com.google.common.collect.Lists;
2223

@@ -26,13 +27,13 @@
2627
/**
2728
* Serializes a class annotated with {@code @UDT} to the corresponding CQL user-defined type.
2829
*/
29-
class MappedUDTCodec<T> extends TypeCodec<T> {
30+
class MappedUDTCodec<T> extends TypeCodec.AbstractUDTCodec<T> {
3031
private final UserType cqlUserType;
3132
private final Class<T> udtClass;
32-
private final List<ColumnMapper<T>> columnMappers;
33+
private final Map<String, ColumnMapper<T>> columnMappers;
3334
private final CodecRegistry codecRegistry;
3435

35-
public MappedUDTCodec(UserType cqlUserType, Class<T> udtClass, List<ColumnMapper<T>> columnMappers, MappingManager mappingManager) {
36+
public MappedUDTCodec(UserType cqlUserType, Class<T> udtClass, Map<String, ColumnMapper<T>> columnMappers, MappingManager mappingManager) {
3637
super(cqlUserType, udtClass);
3738
this.cqlUserType = cqlUserType;
3839
this.udtClass = udtClass;
@@ -41,70 +42,58 @@ public MappedUDTCodec(UserType cqlUserType, Class<T> udtClass, List<ColumnMapper
4142
}
4243

4344
@Override
44-
public ByteBuffer serialize(T sourceObject, ProtocolVersion protocolVersion) throws InvalidTypeException {
45-
if (sourceObject == null)
46-
return null;
47-
48-
int size = 0;
49-
50-
List<ByteBuffer> serializedFields = Lists.newArrayList();
51-
for (ColumnMapper<T> cm : columnMappers) {
52-
Object value = cm.getValue(sourceObject);
53-
TypeCodec<Object> codec = cm.getCustomCodec();
54-
if (codec == null)
55-
codec = codecRegistry.codecFor(cqlUserType.getFieldType(cm.getColumnName()), cm.getJavaType());
56-
ByteBuffer serializedField = codec.serialize(value, protocolVersion);
57-
size += 4 + (serializedField == null ? 0 : serializedField.remaining());
58-
serializedFields.add(serializedField);
59-
}
60-
61-
ByteBuffer result = ByteBuffer.allocate(size);
62-
for (ByteBuffer bb : serializedFields) {
63-
if (bb == null) {
64-
result.putInt(-1);
65-
} else {
66-
result.putInt(bb.remaining());
67-
result.put(bb.duplicate());
68-
}
45+
protected T newInstance() {
46+
try {
47+
return udtClass.newInstance();
48+
} catch (Exception e) {
49+
throw new IllegalArgumentException("Error creating instance of @UDT-annotated class " + udtClass, e);
6950
}
70-
return (ByteBuffer)result.flip();
7151
}
7252

7353
@Override
74-
public T deserialize(ByteBuffer bytes, ProtocolVersion protocolVersion) throws InvalidTypeException {
75-
if (bytes == null || bytes.remaining() == 0)
54+
protected ByteBuffer serializeField(T source, String fieldName, ProtocolVersion protocolVersion) {
55+
// The parent class passes lowercase names unquoted, but in our internal map of mappers they are always quoted
56+
if (!fieldName.startsWith("\""))
57+
fieldName = Metadata.quote(fieldName);
58+
59+
ColumnMapper<T> columnMapper = columnMappers.get(fieldName);
60+
61+
if (columnMapper == null)
7662
return null;
77-
ByteBuffer input = bytes.duplicate();
7863

79-
T targetObject = newInstance();
64+
Object value = columnMapper.getValue(source);
8065

81-
for (ColumnMapper<T> cm : columnMappers) {
82-
int size = input.getInt();
83-
ByteBuffer serialized = (size < 0) ? null : CodecUtils.readBytes(input, size);
84-
TypeCodec<Object> codec = cm.getCustomCodec();
85-
if (codec == null)
86-
codec = codecRegistry.codecFor(cqlUserType.getFieldType(cm.getColumnName()), cm.getJavaType());
87-
cm.setValue(targetObject, codec.deserialize(serialized, protocolVersion));
88-
}
66+
TypeCodec<Object> codec = columnMapper.getCustomCodec();
67+
if (codec == null)
68+
codec = codecRegistry.codecFor(cqlUserType.getFieldType(columnMapper.getColumnName()), columnMapper.getJavaType());
8969

90-
return targetObject;
70+
return codec.serialize(value, protocolVersion);
9171
}
9272

93-
private T newInstance() {
94-
try {
95-
return udtClass.newInstance();
96-
} catch (Exception e) {
97-
throw new IllegalArgumentException("Error creating instance of @UDT-annotated class " + udtClass, e);
73+
@Override
74+
protected T deserializeAndSetField(ByteBuffer input, T target, String fieldName, ProtocolVersion protocolVersion) {
75+
if (!fieldName.startsWith("\""))
76+
fieldName = Metadata.quote(fieldName);
77+
78+
ColumnMapper<T> columnMapper = columnMappers.get(fieldName);
79+
if (columnMapper != null) {
80+
TypeCodec<Object> codec = columnMapper.getCustomCodec();
81+
if (codec == null)
82+
codec = codecRegistry.codecFor(cqlUserType.getFieldType(columnMapper.getColumnName()), columnMapper.getJavaType());
83+
columnMapper.setValue(target, codec.deserialize(input, protocolVersion));
9884
}
85+
return target;
9986
}
10087

10188
@Override
102-
public T parse(String value) throws InvalidTypeException {
89+
protected String formatField(T source, String fieldName) {
90+
// This codec implementation is internal-use only, and its format method is never used
10391
throw new UnsupportedOperationException();
10492
}
10593

10694
@Override
107-
public String format(T value) throws InvalidTypeException {
95+
protected T parseAndSetField(String input, T target, String fieldName) {
96+
// This codec implementation is internal-use only, and its parse method is never used
10897
throw new UnsupportedOperationException();
10998
}
11099
}

‎driver-mapping/src/test/java/com/datastax/driver/mapping/MapperUDTTest.java‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ public void addOtherAddress(String name, Address address) {
113113

114114
@Override
115115
public boolean equals(Object other) {
116+
if (this == other)
117+
return true;
116118
if (other instanceof User) {
117119
User that = (User) other;
118120
return Objects.equal(this.userId, that.userId) &&
@@ -122,6 +124,21 @@ public boolean equals(Object other) {
122124
}
123125
return false;
124126
}
127+
128+
@Override
129+
public int hashCode() {
130+
return Objects.hashCode(this.userId, this.name, this.mainAddress, this.otherAddresses);
131+
}
132+
133+
@Override
134+
public String toString() {
135+
return Objects.toStringHelper(User.class)
136+
.add("userId", userId)
137+
.add("name", name)
138+
.add("mainAddress", mainAddress)
139+
.add("otherAddresses", otherAddresses)
140+
.toString();
141+
}
125142
}
126143

127144
@UDT(name = "address")
@@ -130,11 +147,12 @@ public static class Address {
130147
// Dummy constant to test that static fields are properly ignored
131148
public static final int FOO = 1;
132149

133-
private String street;
134-
135150
@Field // not strictly required, but we want to check that the annotation works without a name
136151
private String city;
137152

153+
// Declared out of order compared to the UDT definition, to make sure that we serialize fields in the correct order (JAVA-884)
154+
private String street;
155+
138156
@Field(name = "ZIP_code", caseSensitive = true)
139157
private int zipCode;
140158

@@ -187,6 +205,8 @@ public void setPhones(Set<String> phones) {
187205

188206
@Override
189207
public boolean equals(Object other) {
208+
if (this == other)
209+
return true;
190210
if (other instanceof Address) {
191211
Address that = (Address) other;
192212
return Objects.equal(this.street, that.street) &&
@@ -196,6 +216,21 @@ public boolean equals(Object other) {
196216
}
197217
return false;
198218
}
219+
220+
@Override
221+
public int hashCode() {
222+
return Objects.hashCode(this.street, this.city, this.zipCode, this.phones);
223+
}
224+
225+
@Override
226+
public String toString() {
227+
return Objects.toStringHelper(Address.class)
228+
.add("street", street)
229+
.add("city", city)
230+
.add("zip", zipCode)
231+
.add("phones", phones)
232+
.toString();
233+
}
199234
}
200235

201236
@Accessor

0 commit comments

Comments
 (0)