Skip to content

Commit 22296c6

Browse files
adutraolim7t
adutra
authored and
olim7t
committed
JAVA-2698: TupleCodec and UdtCodec give wrong error message when parsing fails
1 parent d49132b commit 22296c6

File tree

6 files changed

+324
-40
lines changed

6 files changed

+324
-40
lines changed

‎changelog/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### 4.6.0 (in progress)
66

7+
- [bug] JAVA-2698: TupleCodec and UdtCodec give wrong error message when parsing fails
78
- [improvement] JAVA-2435: Add automatic-module-names to the manifests
89
- [new feature] JAVA-2054: Add now_in_seconds to protocol v5 query messages
910
- [bug] JAVA-2711: Fix handling of UDT keys in the mapper

‎core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/ParseUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public class ParseUtils {
2525
* @return the index of the first character in toParse from idx that is not a "space.
2626
*/
2727
public static int skipSpaces(String toParse, int idx) {
28-
while (isBlank(toParse.charAt(idx)) && idx < toParse.length()) ++idx;
28+
while (idx < toParse.length() && isBlank(toParse.charAt(idx))) ++idx;
2929
return idx;
3030
}
3131

‎core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/TupleCodec.java

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -161,59 +161,85 @@ public TupleValue parse(@Nullable String value) {
161161
}
162162

163163
TupleValue tuple = cqlType.newValue();
164+
int length = value.length();
164165

165166
int position = ParseUtils.skipSpaces(value, 0);
166-
if (value.charAt(position++) != '(') {
167+
if (value.charAt(position) != '(') {
167168
throw new IllegalArgumentException(
168169
String.format(
169170
"Cannot parse tuple value from \"%s\", at character %d expecting '(' but got '%c'",
170171
value, position, value.charAt(position)));
171172
}
172173

174+
position++;
173175
position = ParseUtils.skipSpaces(value, position);
174176

175-
if (value.charAt(position) == ')') {
176-
return tuple;
177-
}
178-
179177
CodecRegistry registry = cqlType.getAttachmentPoint().getCodecRegistry();
180178

181-
int i = 0;
182-
while (position < value.length()) {
179+
int field = 0;
180+
while (position < length) {
181+
if (value.charAt(position) == ')') {
182+
position = ParseUtils.skipSpaces(value, position + 1);
183+
if (position == length) {
184+
return tuple;
185+
}
186+
throw new IllegalArgumentException(
187+
String.format(
188+
"Cannot parse tuple value from \"%s\", at character %d expecting EOF or blank, but got \"%s\"",
189+
value, position, value.substring(position)));
190+
}
183191
int n;
184192
try {
185193
n = ParseUtils.skipCQLValue(value, position);
186194
} catch (IllegalArgumentException e) {
187195
throw new IllegalArgumentException(
188196
String.format(
189-
"Cannot parse tuple value from \"%s\", invalid CQL value at character %d",
190-
value, position),
197+
"Cannot parse tuple value from \"%s\", invalid CQL value at field %d (character %d)",
198+
value, field, position),
191199
e);
192200
}
193201

194202
String fieldValue = value.substring(position, n);
195-
DataType elementType = cqlType.getComponentTypes().get(i);
203+
DataType elementType = cqlType.getComponentTypes().get(field);
196204
TypeCodec<Object> codec = registry.codecFor(elementType);
197-
tuple = tuple.set(i, codec.parse(fieldValue), codec);
205+
Object parsed;
206+
try {
207+
parsed = codec.parse(fieldValue);
208+
} catch (Exception e) {
209+
throw new IllegalArgumentException(
210+
String.format(
211+
"Cannot parse tuple value from \"%s\", invalid CQL value at field %d (character %d): %s",
212+
value, field, position, e.getMessage()),
213+
e);
214+
}
215+
tuple = tuple.set(field, parsed, codec);
198216

199217
position = n;
200-
i += 1;
201218

202219
position = ParseUtils.skipSpaces(value, position);
220+
if (position == length) {
221+
throw new IllegalArgumentException(
222+
String.format(
223+
"Cannot parse tuple value from \"%s\", at field %d (character %d) expecting ',' or ')', but got EOF",
224+
value, field, position));
225+
}
203226
if (value.charAt(position) == ')') {
204-
return tuple;
227+
continue;
205228
}
206229
if (value.charAt(position) != ',') {
207230
throw new IllegalArgumentException(
208231
String.format(
209-
"Cannot parse tuple value from \"%s\", at character %d expecting ',' but got '%c'",
210-
value, position, value.charAt(position)));
232+
"Cannot parse tuple value from \"%s\", at field %d (character %d) expecting ',' but got '%c'",
233+
value, field, position, value.charAt(position)));
211234
}
212235
++position; // skip ','
213236

214237
position = ParseUtils.skipSpaces(value, position);
238+
field += 1;
215239
}
216240
throw new IllegalArgumentException(
217-
String.format("Malformed tuple value \"%s\", missing closing ')'", value));
241+
String.format(
242+
"Cannot parse tuple value from \"%s\", at field %d (character %d) expecting CQL value or ')', got EOF",
243+
value, field, position));
218244
}
219245
}

‎core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/UdtCodec.java

Lines changed: 60 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -165,24 +165,40 @@ public UdtValue parse(@Nullable String value) {
165165
}
166166

167167
UdtValue udt = cqlType.newValue();
168+
int length = value.length();
168169

169170
int position = ParseUtils.skipSpaces(value, 0);
170-
if (value.charAt(position++) != '{') {
171+
if (value.charAt(position) != '{') {
171172
throw new IllegalArgumentException(
172173
String.format(
173-
"Cannot parse UDT value from \"%s\", at character %d expecting '{' but got '%c'",
174+
"Cannot parse UDT value from \"%s\" at character %d: expecting '{' but got '%c'",
174175
value, position, value.charAt(position)));
175176
}
176177

178+
position++;
177179
position = ParseUtils.skipSpaces(value, position);
178180

179-
if (value.charAt(position) == '}') {
180-
return udt;
181+
if (position == length) {
182+
throw new IllegalArgumentException(
183+
String.format(
184+
"Cannot parse UDT value from \"%s\" at character %d: expecting CQL identifier or '}', got EOF",
185+
value, position));
181186
}
182187

183188
CodecRegistry registry = cqlType.getAttachmentPoint().getCodecRegistry();
184189

185-
while (position < value.length()) {
190+
CqlIdentifier id = null;
191+
while (position < length) {
192+
if (value.charAt(position) == '}') {
193+
position = ParseUtils.skipSpaces(value, position + 1);
194+
if (position == length) {
195+
return udt;
196+
}
197+
throw new IllegalArgumentException(
198+
String.format(
199+
"Cannot parse UDT value from \"%s\", at character %d expecting EOF or blank, but got \"%s\"",
200+
value, position, value.substring(position)));
201+
}
186202
int n;
187203
try {
188204
n = ParseUtils.skipCQLId(value, position);
@@ -193,55 +209,82 @@ public UdtValue parse(@Nullable String value) {
193209
value, position),
194210
e);
195211
}
196-
CqlIdentifier id = CqlIdentifier.fromInternal(value.substring(position, n));
212+
id = CqlIdentifier.fromInternal(value.substring(position, n));
197213
position = n;
198214

199215
if (!cqlType.contains(id)) {
200216
throw new IllegalArgumentException(
201-
String.format("Unknown field %s in value \"%s\"", id, value));
217+
String.format(
218+
"Cannot parse UDT value from \"%s\", unknown CQL identifier at character %d: \"%s\"",
219+
value, position, id));
202220
}
203221

204222
position = ParseUtils.skipSpaces(value, position);
205-
if (value.charAt(position++) != ':') {
223+
if (position == length) {
224+
throw new IllegalArgumentException(
225+
String.format(
226+
"Cannot parse UDT value from \"%s\", at field %s (character %d) expecting ':', but got EOF",
227+
value, id, position));
228+
}
229+
if (value.charAt(position) != ':') {
206230
throw new IllegalArgumentException(
207231
String.format(
208-
"Cannot parse UDT value from \"%s\", at character %d expecting ':' but got '%c'",
209-
value, position, value.charAt(position)));
232+
"Cannot parse UDT value from \"%s\", at field %s (character %d) expecting ':', but got '%c'",
233+
value, id, position, value.charAt(position)));
210234
}
235+
position++;
211236
position = ParseUtils.skipSpaces(value, position);
212237

213238
try {
214239
n = ParseUtils.skipCQLValue(value, position);
215240
} catch (IllegalArgumentException e) {
216241
throw new IllegalArgumentException(
217242
String.format(
218-
"Cannot parse UDT value from \"%s\", invalid CQL value at character %d",
219-
value, position),
243+
"Cannot parse UDT value from \"%s\", invalid CQL value at field %s (character %d)",
244+
value, id, position),
220245
e);
221246
}
222247

223248
String fieldValue = value.substring(position, n);
224249
// This works because ids occur at most once in UDTs
225250
DataType fieldType = cqlType.getFieldTypes().get(cqlType.firstIndexOf(id));
226251
TypeCodec<Object> codec = registry.codecFor(fieldType);
227-
udt = udt.set(id, codec.parse(fieldValue), codec);
252+
Object parsed;
253+
try {
254+
parsed = codec.parse(fieldValue);
255+
} catch (Exception e) {
256+
throw new IllegalArgumentException(
257+
String.format(
258+
"Cannot parse UDT value from \"%s\", invalid CQL value at field %s (character %d): %s",
259+
value, id, position, e.getMessage()),
260+
e);
261+
}
262+
udt = udt.set(id, parsed, codec);
228263
position = n;
229264

230265
position = ParseUtils.skipSpaces(value, position);
266+
if (position == length) {
267+
throw new IllegalArgumentException(
268+
String.format(
269+
"Cannot parse UDT value from \"%s\", at field %s (character %d) expecting ',' or '}', but got EOF",
270+
value, id, position));
271+
}
231272
if (value.charAt(position) == '}') {
232-
return udt;
273+
continue;
233274
}
234275
if (value.charAt(position) != ',') {
235276
throw new IllegalArgumentException(
236277
String.format(
237-
"Cannot parse UDT value from \"%s\", at character %d expecting ',' but got '%c'",
238-
value, position, value.charAt(position)));
278+
"Cannot parse UDT value from \"%s\", at field %s (character %d) expecting ',' but got '%c'",
279+
value, id, position, value.charAt(position)));
239280
}
240281
++position; // skip ','
241282

242283
position = ParseUtils.skipSpaces(value, position);
243284
}
244285
throw new IllegalArgumentException(
245-
String.format("Malformed UDT value \"%s\", missing closing '}'", value));
286+
String.format(
287+
"Cannot parse UDT value from \"%s\" at field %s (character %d): expecting CQL identifier or '}', got EOF",
288+
value, id, position));
246289
}
247290
}

‎core/src/test/java/com/datastax/oss/driver/internal/core/type/codec/TupleCodecTest.java

Lines changed: 102 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616
package com.datastax.oss.driver.internal.core.type.codec;
1717

1818
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1920
import static org.mockito.Mockito.spy;
2021
import static org.mockito.Mockito.verify;
22+
import static org.mockito.Mockito.verifyNoMoreInteractions;
2123
import static org.mockito.Mockito.verifyZeroInteractions;
2224
import static org.mockito.Mockito.when;
2325

@@ -169,7 +171,33 @@ public void should_parse_null_tuple() {
169171
}
170172

171173
@Test
172-
public void should_parse_tuple() {
174+
public void should_parse_empty_tuple() {
175+
TupleValue tuple = parse("()");
176+
177+
assertThat(tuple.isNull(0)).isTrue();
178+
assertThat(tuple.isNull(1)).isTrue();
179+
assertThat(tuple.isNull(2)).isTrue();
180+
181+
verifyNoMoreInteractions(intCodec);
182+
verifyNoMoreInteractions(doubleCodec);
183+
verifyNoMoreInteractions(textCodec);
184+
}
185+
186+
@Test
187+
public void should_parse_partial_tuple() {
188+
TupleValue tuple = parse("(1,NULL)");
189+
190+
assertThat(tuple.getInt(0)).isEqualTo(1);
191+
assertThat(tuple.isNull(1)).isTrue();
192+
assertThat(tuple.isNull(2)).isTrue();
193+
194+
verify(intCodec).parse("1");
195+
verify(doubleCodec).parse("NULL");
196+
verifyNoMoreInteractions(textCodec);
197+
}
198+
199+
@Test
200+
public void should_parse_full_tuple() {
173201
TupleValue tuple = parse("(1,NULL,'a')");
174202

175203
assertThat(tuple.getInt(0)).isEqualTo(1);
@@ -181,9 +209,80 @@ public void should_parse_tuple() {
181209
verify(textCodec).parse("'a'");
182210
}
183211

184-
@Test(expected = IllegalArgumentException.class)
212+
@Test
213+
public void should_parse_tuple_with_extra_whitespace() {
214+
TupleValue tuple = parse(" ( 1 , NULL , 'a' ) ");
215+
216+
assertThat(tuple.getInt(0)).isEqualTo(1);
217+
assertThat(tuple.isNull(1)).isTrue();
218+
assertThat(tuple.getString(2)).isEqualTo("a");
219+
220+
verify(intCodec).parse("1");
221+
verify(doubleCodec).parse("NULL");
222+
verify(textCodec).parse("'a'");
223+
}
224+
225+
@Test
185226
public void should_fail_to_parse_invalid_input() {
186-
parse("not a tuple");
227+
// general tuple structure invalid
228+
assertThatThrownBy(() -> parse("not a tuple"))
229+
.isInstanceOf(IllegalArgumentException.class)
230+
.hasMessage(
231+
"Cannot parse tuple value from \"not a tuple\", at character 0 expecting '(' but got 'n'");
232+
assertThatThrownBy(() -> parse(" ( "))
233+
.isInstanceOf(IllegalArgumentException.class)
234+
.hasMessage(
235+
"Cannot parse tuple value from \" ( \", at field 0 (character 3) expecting CQL value or ')', got EOF");
236+
assertThatThrownBy(() -> parse("( ["))
237+
.isInstanceOf(IllegalArgumentException.class)
238+
.hasMessage(
239+
"Cannot parse tuple value from \"( [\", invalid CQL value at field 0 (character 2)");
240+
assertThatThrownBy(() -> parse("( 12 , "))
241+
.isInstanceOf(IllegalArgumentException.class)
242+
.hasMessage(
243+
"Cannot parse tuple value from \"( 12 , \", at field 1 (character 7) expecting CQL value or ')', got EOF");
244+
assertThatThrownBy(() -> parse("( 12 12.34 "))
245+
.isInstanceOf(IllegalArgumentException.class)
246+
.hasMessage(
247+
"Cannot parse tuple value from \"( 12 12.34 \", at field 0 (character 5) expecting ',' but got '1'");
248+
assertThatThrownBy(() -> parse("(1234,12.34,'text'"))
249+
.isInstanceOf(IllegalArgumentException.class)
250+
.hasMessage(
251+
"Cannot parse tuple value from \"(1234,12.34,'text'\", at field 2 (character 18) expecting ',' or ')', but got EOF");
252+
assertThatThrownBy(() -> parse("(1234,12.34,'text'))"))
253+
.isInstanceOf(IllegalArgumentException.class)
254+
.hasMessage(
255+
"Cannot parse tuple value from \"(1234,12.34,'text'))\", at character 19 expecting EOF or blank, but got \")\"");
256+
assertThatThrownBy(() -> parse("())"))
257+
.isInstanceOf(IllegalArgumentException.class)
258+
.hasMessage(
259+
"Cannot parse tuple value from \"())\", at character 2 expecting EOF or blank, but got \")\"");
260+
assertThatThrownBy(() -> parse("(1234,12.34,'text') extra"))
261+
.isInstanceOf(IllegalArgumentException.class)
262+
.hasMessage(
263+
"Cannot parse tuple value from \"(1234,12.34,'text') extra\", at character 20 expecting EOF or blank, but got \"extra\"");
264+
// element syntax invalid
265+
assertThatThrownBy(() -> parse("(not a valid int,12.34,'text')"))
266+
.isInstanceOf(IllegalArgumentException.class)
267+
.hasMessage(
268+
"Cannot parse tuple value from \"(not a valid int,12.34,'text')\", "
269+
+ "invalid CQL value at field 0 (character 1): "
270+
+ "Cannot parse 32-bits int value from \"not\"")
271+
.hasRootCauseInstanceOf(IllegalArgumentException.class);
272+
assertThatThrownBy(() -> parse("(1234,not a valid double,'text')"))
273+
.isInstanceOf(IllegalArgumentException.class)
274+
.hasMessage(
275+
"Cannot parse tuple value from \"(1234,not a valid double,'text')\", "
276+
+ "invalid CQL value at field 1 (character 6): "
277+
+ "Cannot parse 64-bits double value from \"not\"")
278+
.hasRootCauseInstanceOf(IllegalArgumentException.class);
279+
assertThatThrownBy(() -> parse("(1234,12.34,not a valid text)"))
280+
.isInstanceOf(IllegalArgumentException.class)
281+
.hasMessage(
282+
"Cannot parse tuple value from \"(1234,12.34,not a valid text)\", "
283+
+ "invalid CQL value at field 2 (character 12): "
284+
+ "text or varchar values must be enclosed by single quotes")
285+
.hasRootCauseInstanceOf(IllegalArgumentException.class);
187286
}
188287

189288
@Test

0 commit comments

Comments
 (0)