Skip to content
This repository was archived by the owner on Feb 24, 2026. It is now read-only.

Commit e43df34

Browse files
yirutangyirutang
authored
fix: Cleanup JsonWriter bytes conversion code and add some test coverage (#984)
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/java-bigquerystorage/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) Fixes #<issue_number_goes_here> ☕️
1 parent 17bfbd8 commit e43df34

File tree

7 files changed

+101
-43
lines changed

7 files changed

+101
-43
lines changed

‎google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public class BQTableSchemaToProtoDescriptor {
5454
.put(Table.TableFieldSchema.Type.DOUBLE, FieldDescriptorProto.Type.TYPE_DOUBLE)
5555
.put(Table.TableFieldSchema.Type.GEOGRAPHY, FieldDescriptorProto.Type.TYPE_STRING)
5656
.put(Table.TableFieldSchema.Type.INT64, FieldDescriptorProto.Type.TYPE_INT64)
57-
.put(Table.TableFieldSchema.Type.NUMERIC, FieldDescriptorProto.Type.TYPE_STRING)
57+
.put(Table.TableFieldSchema.Type.NUMERIC, FieldDescriptorProto.Type.TYPE_BYTES)
5858
.put(Table.TableFieldSchema.Type.STRING, FieldDescriptorProto.Type.TYPE_STRING)
5959
.put(Table.TableFieldSchema.Type.STRUCT, FieldDescriptorProto.Type.TYPE_MESSAGE)
6060
.put(Table.TableFieldSchema.Type.TIME, FieldDescriptorProto.Type.TYPE_STRING)

‎google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessage.java‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.google.protobuf.DynamicMessage;
2424
import com.google.protobuf.Message;
2525
import com.google.protobuf.UninitializedMessageException;
26+
import java.util.logging.Logger;
2627
import org.json.JSONArray;
2728
import org.json.JSONException;
2829
import org.json.JSONObject;
@@ -32,10 +33,11 @@
3233
* descriptor must have all fields lowercased.
3334
*/
3435
public class JsonToProtoMessage {
36+
private static final Logger LOG = Logger.getLogger(JsonToProtoMessage.class.getName());
3537
private static ImmutableMap<FieldDescriptor.Type, String> FieldTypeToDebugMessage =
3638
new ImmutableMap.Builder<FieldDescriptor.Type, String>()
3739
.put(FieldDescriptor.Type.BOOL, "boolean")
38-
.put(FieldDescriptor.Type.BYTES, "string")
40+
.put(FieldDescriptor.Type.BYTES, "bytes")
3941
.put(FieldDescriptor.Type.INT32, "int32")
4042
.put(FieldDescriptor.Type.DOUBLE, "double")
4143
.put(FieldDescriptor.Type.INT64, "int64")
@@ -142,9 +144,6 @@ private static void fillField(
142144
if (val instanceof ByteString) {
143145
protoMsg.setField(fieldDescriptor, ((ByteString) val).toByteArray());
144146
return;
145-
} else if (val instanceof String) {
146-
protoMsg.setField(fieldDescriptor, ((String) val).getBytes());
147-
return;
148147
}
149148
break;
150149
case INT64:
@@ -237,18 +236,23 @@ private static void fillRepeatedField(
237236
}
238237
break;
239238
case BYTES:
240-
if (val instanceof String) {
241-
// TODO(jstocklass): If string, decode it and pass in the byte array. Will need to
242-
// update tests to ensure that strings passed in are properly encoded as well.
243-
protoMsg.addRepeatedField(fieldDescriptor, ((String) val).getBytes());
244-
} else if (val instanceof JSONArray) {
239+
if (val instanceof JSONArray) {
245240
try {
246241
byte[] bytes = new byte[((JSONArray) val).length()];
247242
for (int j = 0; j < ((JSONArray) val).length(); j++) {
248-
bytes[j] = (byte) ((byte) (((JSONArray) val).get(j)) & 0xFF);
243+
bytes[j] = (byte) ((JSONArray) val).getInt(j);
244+
if (bytes[j] != ((JSONArray) val).getInt(j)) {
245+
throw new IllegalArgumentException(
246+
String.format(
247+
"Error: "
248+
+ currentScope
249+
+ "["
250+
+ index
251+
+ "] could not be converted to byte[]."));
252+
}
249253
}
250254
protoMsg.addRepeatedField(fieldDescriptor, bytes);
251-
} catch (ClassCastException e) {
255+
} catch (JSONException e) {
252256
throw new IllegalArgumentException(
253257
String.format(
254258
"Error: "

‎google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public class BQTableSchemaToProtoDescriptorTest {
4444
.put(Table.TableFieldSchema.Type.DOUBLE, DoubleType.getDescriptor())
4545
.put(Table.TableFieldSchema.Type.GEOGRAPHY, StringType.getDescriptor())
4646
.put(Table.TableFieldSchema.Type.INT64, Int64Type.getDescriptor())
47-
.put(Table.TableFieldSchema.Type.NUMERIC, StringType.getDescriptor())
47+
.put(Table.TableFieldSchema.Type.NUMERIC, BytesType.getDescriptor())
4848
.put(Table.TableFieldSchema.Type.STRING, StringType.getDescriptor())
4949
.put(Table.TableFieldSchema.Type.TIME, StringType.getDescriptor())
5050
.put(Table.TableFieldSchema.Type.TIMESTAMP, Int64Type.getDescriptor())
@@ -206,6 +206,12 @@ public void testStructComplex() throws Exception {
206206
.setMode(Table.TableFieldSchema.Mode.NULLABLE)
207207
.setName("test_time")
208208
.build();
209+
final Table.TableFieldSchema TEST_NUMERIC_REPEATED =
210+
Table.TableFieldSchema.newBuilder()
211+
.setType(TableFieldSchema.Type.NUMERIC)
212+
.setMode(Table.TableFieldSchema.Mode.REPEATED)
213+
.setName("test_numeric_repeated")
214+
.build();
209215
final Table.TableSchema tableSchema =
210216
Table.TableSchema.newBuilder()
211217
.addFields(0, test_int)
@@ -220,6 +226,7 @@ public void testStructComplex() throws Exception {
220226
.addFields(9, TEST_GEO)
221227
.addFields(10, TEST_TIMESTAMP)
222228
.addFields(11, TEST_TIME)
229+
.addFields(12, TEST_NUMERIC_REPEATED)
223230
.build();
224231
final Descriptor descriptor =
225232
BQTableSchemaToProtoDescriptor.convertBQTableSchemaToProtoDescriptor(tableSchema);

‎google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/BQTableSchemaToProtoDescriptorTest.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,12 @@ public void testStructComplex() throws Exception {
203203
.setMode(TableFieldSchema.Mode.NULLABLE)
204204
.setName("test_time")
205205
.build();
206+
final TableFieldSchema TEST_NUMERIC_REPEATED =
207+
TableFieldSchema.newBuilder()
208+
.setType(TableFieldSchema.Type.NUMERIC)
209+
.setMode(TableFieldSchema.Mode.REPEATED)
210+
.setName("test_numeric_repeated")
211+
.build();
206212
final TableSchema tableSchema =
207213
TableSchema.newBuilder()
208214
.addFields(0, test_int)
@@ -217,6 +223,7 @@ public void testStructComplex() throws Exception {
217223
.addFields(9, TEST_GEO)
218224
.addFields(10, TEST_TIMESTAMP)
219225
.addFields(11, TEST_TIME)
226+
.addFields(12, TEST_NUMERIC_REPEATED)
220227
.build();
221228
final Descriptor descriptor =
222229
BQTableSchemaToProtoDescriptor.convertBQTableSchemaToProtoDescriptor(tableSchema);

‎google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriterTest.java‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@
3737
import com.google.protobuf.Timestamp;
3838
import java.io.IOException;
3939
import java.math.BigDecimal;
40-
import java.util.*;
40+
import java.util.Arrays;
41+
import java.util.Collection;
42+
import java.util.Collections;
43+
import java.util.HashSet;
44+
import java.util.UUID;
4145
import java.util.concurrent.ExecutionException;
4246
import java.util.logging.Logger;
4347
import org.json.JSONArray;
@@ -145,6 +149,12 @@ public class JsonStreamWriterTest {
145149
.setMode(TableFieldSchema.Mode.NULLABLE)
146150
.setName("test_numeric")
147151
.build();
152+
private final TableFieldSchema TEST_NUMERIC_REPEATED =
153+
TableFieldSchema.newBuilder()
154+
.setType(TableFieldSchema.Type.NUMERIC)
155+
.setMode(TableFieldSchema.Mode.REPEATED)
156+
.setName("test_numeric_repeated")
157+
.build();
148158
private final TableFieldSchema TEST_GEO =
149159
TableFieldSchema.newBuilder()
150160
.setType(TableFieldSchema.Type.GEOGRAPHY)
@@ -177,6 +187,7 @@ public class JsonStreamWriterTest {
177187
.addFields(9, TEST_GEO)
178188
.addFields(10, TEST_TIMESTAMP)
179189
.addFields(11, TEST_TIME)
190+
.addFields(12, TEST_NUMERIC_REPEATED)
180191
.build();
181192

182193
@Before
@@ -414,6 +425,14 @@ public void testSingleAppendComplexJson() throws Exception {
414425
.setTestGeo("POINT(1,1)")
415426
.setTestTimestamp(12345678)
416427
.setTestTime(CivilTimeEncoder.encodePacked64TimeMicros(LocalTime.of(1, 0, 1)))
428+
.addTestNumericRepeated(
429+
BigDecimalByteStringEncoder.encodeToNumericByteString(new BigDecimal("0")))
430+
.addTestNumericRepeated(
431+
BigDecimalByteStringEncoder.encodeToNumericByteString(
432+
new BigDecimal("99999999999999999999999999999.999999999")))
433+
.addTestNumericRepeated(
434+
BigDecimalByteStringEncoder.encodeToNumericByteString(
435+
new BigDecimal("-99999999999999999999999999999.999999999")))
417436
.build();
418437
JSONObject complex_lvl2 = new JSONObject();
419438
complex_lvl2.put("test_int", 3);
@@ -425,7 +444,7 @@ public void testSingleAppendComplexJson() throws Exception {
425444
JSONObject json = new JSONObject();
426445
json.put("test_int", 1);
427446
json.put("test_string", new JSONArray(new String[] {"a", "b", "c"}));
428-
json.put("test_bytes", "hello");
447+
json.put("test_bytes", ByteString.copyFrom("hello".getBytes()));
429448
json.put("test_bool", true);
430449
json.put("test_DOUBLe", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4}));
431450
json.put("test_date", 1);
@@ -434,6 +453,19 @@ public void testSingleAppendComplexJson() throws Exception {
434453
json.put(
435454
"test_numeric",
436455
BigDecimalByteStringEncoder.encodeToNumericByteString(new BigDecimal("1.23456")));
456+
json.put(
457+
"test_numeric_repeated",
458+
new JSONArray(
459+
new byte[][] {
460+
BigDecimalByteStringEncoder.encodeToNumericByteString(new BigDecimal("0"))
461+
.toByteArray(),
462+
BigDecimalByteStringEncoder.encodeToNumericByteString(
463+
new BigDecimal("99999999999999999999999999999.999999999"))
464+
.toByteArray(),
465+
BigDecimalByteStringEncoder.encodeToNumericByteString(
466+
new BigDecimal("-99999999999999999999999999999.999999999"))
467+
.toByteArray(),
468+
}));
437469
json.put("test_geo", "POINT(1,1)");
438470
json.put("test_timestamp", 12345678);
439471
json.put("test_time", CivilTimeEncoder.encodePacked64TimeMicros(LocalTime.of(1, 0, 1)));

‎google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessageTest.java‎

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public class JsonToProtoMessageTest {
4242
private static ImmutableMap<Descriptor, String> AllTypesToDebugMessageTest =
4343
new ImmutableMap.Builder<Descriptor, String>()
4444
.put(BoolType.getDescriptor(), "boolean")
45-
.put(BytesType.getDescriptor(), "string")
45+
.put(BytesType.getDescriptor(), "bytes")
4646
.put(Int64Type.getDescriptor(), "int64")
4747
.put(Int32Type.getDescriptor(), "int32")
4848
.put(DoubleType.getDescriptor(), "double")
@@ -59,9 +59,7 @@ public class JsonToProtoMessageTest {
5959
.put(
6060
BytesType.getDescriptor(),
6161
new Message[] {
62-
BytesType.newBuilder()
63-
.setTestFieldType(ByteString.copyFrom("test".getBytes()))
64-
.build()
62+
BytesType.newBuilder().setTestFieldType(ByteString.copyFromUtf8("test")).build()
6563
})
6664
.put(
6765
Int64Type.getDescriptor(),
@@ -101,15 +99,15 @@ public class JsonToProtoMessageTest {
10199
})
102100
.build();
103101

104-
private static ImmutableMap<Descriptor, String[]> AllRepeatedTypesToDebugMessageTest =
105-
new ImmutableMap.Builder<Descriptor, String[]>()
106-
.put(RepeatedBool.getDescriptor(), new String[] {"boolean"})
107-
.put(RepeatedBytes.getDescriptor(), new String[] {"string", "bytes"})
108-
.put(RepeatedInt64.getDescriptor(), new String[] {"int64"})
109-
.put(RepeatedInt32.getDescriptor(), new String[] {"int32"})
110-
.put(RepeatedDouble.getDescriptor(), new String[] {"double"})
111-
.put(RepeatedString.getDescriptor(), new String[] {"string"})
112-
.put(RepeatedObject.getDescriptor(), new String[] {"object"})
102+
private static ImmutableMap<Descriptor, String> AllRepeatedTypesToDebugMessageTest =
103+
new ImmutableMap.Builder<Descriptor, String>()
104+
.put(RepeatedBool.getDescriptor(), "boolean")
105+
.put(RepeatedBytes.getDescriptor(), "bytes")
106+
.put(RepeatedInt64.getDescriptor(), "int64")
107+
.put(RepeatedInt32.getDescriptor(), "int32")
108+
.put(RepeatedDouble.getDescriptor(), "double")
109+
.put(RepeatedString.getDescriptor(), "string")
110+
.put(RepeatedObject.getDescriptor(), "object")
113111
.build();
114112

115113
private static ImmutableMap<Descriptor, Message[]> AllRepeatedTypesToCorrectProto =
@@ -123,8 +121,8 @@ public class JsonToProtoMessageTest {
123121
RepeatedBytes.getDescriptor(),
124122
new Message[] {
125123
RepeatedBytes.newBuilder()
126-
.addTestRepeated(ByteString.copyFrom("hello".getBytes()))
127-
.addTestRepeated(ByteString.copyFrom("test".getBytes()))
124+
.addTestRepeated(ByteString.copyFrom(new byte[] {0}))
125+
.addTestRepeated(ByteString.copyFrom(new byte[] {0, -116, -122, 71}))
128126
.build(),
129127
RepeatedBytes.newBuilder()
130128
.addTestRepeated(
@@ -206,9 +204,10 @@ public class JsonToProtoMessageTest {
206204
new JSONObject().put("test_field_type", Integer.MAX_VALUE),
207205
new JSONObject().put("test_field_type", 1.23),
208206
new JSONObject().put("test_field_type", true),
209-
new JSONObject().put("test_field_type", "test"),
207+
new JSONObject().put("test_field_type", ByteString.copyFromUtf8("test")),
210208
new JSONObject().put("test_field_type", new JSONArray("[1, 2, 3]")),
211-
new JSONObject().put("test_field_type", new JSONObject().put("test_int", 1))
209+
new JSONObject().put("test_field_type", new JSONObject().put("test_int", 1)),
210+
new JSONObject().put("test_field_type", "test")
212211
};
213212

214213
private static JSONObject[] simpleJSONArrays = {
@@ -264,6 +263,7 @@ public class JsonToProtoMessageTest {
264263
BigDecimalByteStringEncoder.encodeToNumericByteString(new BigDecimal("1.2"))
265264
.toByteArray()
266265
})),
266+
new JSONObject().put("test_repeated", new JSONArray(new int[][] {{11111, 22222}})),
267267
new JSONObject().put("test_repeated", new JSONArray(new char[][] {{'a', 'b'}, {'c'}})),
268268
new JSONObject().put("test_repeated", new JSONArray(new String[][] {{"hello"}, {"test"}})),
269269
new JSONObject()
@@ -350,8 +350,10 @@ public void testAllTypes() throws Exception {
350350
int success = 0;
351351
for (JSONObject json : simpleJSONObjects) {
352352
try {
353+
LOG.info("Testing " + json + " over " + entry.getKey().getFullName());
353354
DynamicMessage protoMsg =
354355
JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json);
356+
LOG.info("Convert Success!");
355357
assertEquals(protoMsg, AllTypesToCorrectProto.get(entry.getKey())[success]);
356358
success += 1;
357359
} catch (IllegalArgumentException e) {
@@ -361,40 +363,45 @@ public void testAllTypes() throws Exception {
361363
}
362364
}
363365
if (entry.getKey() == Int64Type.getDescriptor()) {
364-
assertEquals(2, success);
366+
assertEquals(entry.getKey().getFullName(), 2, success);
365367
} else {
366-
assertEquals(1, success);
368+
assertEquals(entry.getKey().getFullName(), 1, success);
367369
}
368370
}
369371
}
370372

371373
@Test
372374
public void testAllRepeatedTypesWithLimits() throws Exception {
373-
for (Map.Entry<Descriptor, String[]> entry : AllRepeatedTypesToDebugMessageTest.entrySet()) {
375+
for (Map.Entry<Descriptor, String> entry : AllRepeatedTypesToDebugMessageTest.entrySet()) {
374376
int success = 0;
375377
for (JSONObject json : simpleJSONArrays) {
376378
try {
379+
LOG.info("Testing " + json + " over " + entry.getKey().getFullName());
377380
DynamicMessage protoMsg =
378381
JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json);
379-
assertEquals(protoMsg, AllRepeatedTypesToCorrectProto.get(entry.getKey())[success]);
382+
LOG.info("Convert Success!");
383+
assertEquals(
384+
protoMsg.toString(),
385+
protoMsg,
386+
AllRepeatedTypesToCorrectProto.get(entry.getKey())[success]);
380387
success += 1;
381388
} catch (IllegalArgumentException e) {
389+
LOG.info(e.getMessage());
382390
assertTrue(
383391
e.getMessage()
384392
.equals(
385393
"JSONObject does not have a "
386-
+ entry.getValue()[0]
394+
+ entry.getValue()
387395
+ " field at root.test_repeated[0].")
388396
|| e.getMessage()
389397
.equals("Error: root.test_repeated[0] could not be converted to byte[]."));
390398
}
391399
}
392400
if (entry.getKey() == RepeatedInt64.getDescriptor()
393-
|| entry.getKey() == RepeatedDouble.getDescriptor()
394-
|| entry.getKey() == RepeatedBytes.getDescriptor()) {
395-
assertEquals(2, success);
401+
|| entry.getKey() == RepeatedDouble.getDescriptor()) {
402+
assertEquals(entry.getKey().getFullName(), 2, success);
396403
} else {
397-
assertEquals(1, success);
404+
assertEquals(entry.getKey().getFullName(), 1, success);
398405
}
399406
}
400407
}
@@ -501,7 +508,7 @@ public void testStructComplex() throws Exception {
501508
JSONObject json = new JSONObject();
502509
json.put("test_int", 1);
503510
json.put("test_string", new JSONArray(new String[] {"a", "b", "c"}));
504-
json.put("test_bytes", "hello");
511+
json.put("test_bytes", ByteString.copyFromUtf8("hello"));
505512
json.put("test_bool", true);
506513
json.put("test_DOUBLe", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4}));
507514
json.put("test_date", 1);
@@ -525,7 +532,7 @@ public void testStructComplexFail() throws Exception {
525532
JSONObject json = new JSONObject();
526533
json.put("test_int", 1);
527534
json.put("test_string", new JSONArray(new String[] {"a", "b", "c"}));
528-
json.put("test_bytes", "hello");
535+
json.put("test_bytes", ByteString.copyFromUtf8("hello"));
529536
json.put("test_bool", true);
530537
json.put("test_double", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4}));
531538
json.put("test_date", 1);

‎google-cloud-bigquerystorage/src/test/proto/jsonTest.proto‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ message ComplexRoot {
1515
optional string test_geo = 10;
1616
optional int64 test_timestamp = 11;
1717
optional int64 test_time = 12;
18+
repeated bytes test_numeric_repeated = 13;
1819
}
1920

2021
message CasingComplex {

0 commit comments

Comments
 (0)