From 69e4d3c1dbddf4b76d70c35be76d9a231bba0b5f Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 7 Jul 2020 17:24:09 -0500 Subject: [PATCH 01/21] feat: Finished JsonToProtoMessage, adding in tests --- google-cloud-bigquerystorage/pom.xml | 6 + .../storage/v1alpha2/JsonToProtoMessage.java | 346 ++++++++++ .../v1alpha2/JsonToProtoMessageTest.java | 625 ++++++++++++++++++ 3 files changed, 977 insertions(+) create mode 100644 google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java create mode 100644 google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java diff --git a/google-cloud-bigquerystorage/pom.xml b/google-cloud-bigquerystorage/pom.xml index 9ba98ec314..d8bddfabd3 100644 --- a/google-cloud-bigquerystorage/pom.xml +++ b/google-cloud-bigquerystorage/pom.xml @@ -108,6 +108,12 @@ org.apache.commons commons-lang3 + + org.json + json + 20200518 + + diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java new file mode 100644 index 0000000000..2257ad667e --- /dev/null +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -0,0 +1,346 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigquery.storage.v1alpha2; + +import com.google.protobuf.Descriptors.Descriptor; +import com.google.protobuf.Descriptors.FieldDescriptor; +import com.google.protobuf.DynamicMessage; +import com.google.protobuf.Message; +import java.util.List; +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; + +/** Converts Json data to protocol buffer messages given the protocol buffer descriptor. */ +public class JsonToProtoMessage { + + /** + * Converts Json data to protocol buffer messages given the protocol buffer descriptor. + * + * @param protoSchema + * @param json + * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. + */ + public static DynamicMessage convertJsonToProtoMessage(Descriptor protoSchema, JSONObject json) + throws IllegalArgumentException { + return convertJsonToProtoMessage(protoSchema, json, false); + } + + /** + * Converts Json data to protocol buffer messages given the protocol buffer descriptor. + * + * @param protoSchema + * @param json + * @param allowUnknownFields Ignores unknown JSON fields. + * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. + */ + public static DynamicMessage convertJsonToProtoMessage( + Descriptor protoSchema, JSONObject json, boolean allowUnknownFields) + throws IllegalArgumentException { + return convertJsonToProtoMessageImpl(protoSchema, json, "root", true, allowUnknownFields); + } + + /** + * Converts Json data to protocol buffer messages given the protocol buffer descriptor. + * + * @param protoSchema + * @param json + * @param jsonScope Debugging purposes + * @param topLevel If root level has no matching fields, throws exception. + * @param allowUnknownFields Ignores unknown JSON fields. + * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. + */ + private static DynamicMessage convertJsonToProtoMessageImpl( + Descriptor protoSchema, + JSONObject json, + String jsonScope, + boolean topLevel, + boolean allowUnknownFields) + throws IllegalArgumentException { + DynamicMessage.Builder protoMsg = DynamicMessage.newBuilder(protoSchema); + int matchedFields = 0; + List protoFields = protoSchema.getFields(); + + if (JSONObject.getNames(json).length > protoFields.size()) { + if (!allowUnknownFields) { + throw new IllegalArgumentException( + "JSONObject has unknown fields. Set allowUnknownFields to True to ignore unknown fields."); + } + } + + for (FieldDescriptor field : protoFields) { + String fieldName = field.getName(); + String currentScope = jsonScope + "." + fieldName; + + if (!json.has(fieldName)) { + if (field.isRequired()) { + throw new IllegalArgumentException( + "JSONObject does not have the required field " + currentScope + "."); + } else { + continue; + } + } + matchedFields++; + if (!field.isRepeated()) { + fillField(protoMsg, field, json, currentScope, allowUnknownFields); + } else { + fillRepeatedField(protoMsg, field, json, currentScope, allowUnknownFields); + } + } + if (matchedFields == 0 && topLevel) { + throw new IllegalArgumentException( + "There are no matching fields found for the JSONObject and the protocol buffer descriptor."); + } + return protoMsg.build(); + } + + /** + * Fills a non-repetaed protoField with the json data. + * + * @param protoMsg + * @param field + * @param json + * @param currentScope Debugging purposes + * @param allowUnknownFields Ignores unknown JSON fields. + * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. + */ + private static void fillField( + DynamicMessage.Builder protoMsg, + FieldDescriptor field, + JSONObject json, + String currentScope, + boolean allowUnknownFields) + throws IllegalArgumentException { + + String fieldName = field.getName(); + switch (field.getType()) { + case BOOL: + try { + protoMsg.setField(field, new Boolean(json.getBoolean(fieldName))); + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a boolean field at " + currentScope + "."); + } + break; + case BYTES: + try { + protoMsg.setField(field, json.getString(fieldName).getBytes()); + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a string field at " + currentScope + "."); + } + break; + case INT64: + try { + java.lang.Object val = json.get(fieldName); + if (val instanceof Byte) { + protoMsg.setField(field, new Long((Byte) val)); + } else if (val instanceof Short) { + protoMsg.setField(field, new Long((Short) val)); + } else if (val instanceof Integer) { + protoMsg.setField(field, new Long((Integer) val)); + } else if (val instanceof Long) { + protoMsg.setField(field, new Long((Long) val)); + } else { + throw new IllegalArgumentException( + "JSONObject does not have a int64 field at " + currentScope + "."); + } + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a int64 field at " + currentScope + "."); + } + break; + case STRING: + try { + protoMsg.setField(field, json.getString(fieldName)); + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a string field at " + currentScope + "."); + } + break; + case DOUBLE: + try { + java.lang.Object val = json.get(fieldName); + if (val instanceof Double) { + protoMsg.setField(field, new Double((double) val)); + } else if (val instanceof Float) { + protoMsg.setField(field, new Double((float) val)); + } else { + throw new IllegalArgumentException( + "JSONObject does not have a double field at " + currentScope + "."); + } + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a double field at " + currentScope + "."); + } + break; + case MESSAGE: + Message.Builder message = protoMsg.newBuilderForField(field); + try { + protoMsg.setField( + field, + convertJsonToProtoMessageImpl( + field.getMessageType(), + json.getJSONObject(fieldName), + currentScope, + false, + allowUnknownFields)); + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have an object field at " + currentScope + "."); + } + break; + } + } + + /** + * Fills a repeated protoField with the json data. + * + * @param protoMsg + * @param field + * @param json If root level has no matching fields, throws exception. + * @param currentScope Debugging purposes + * @param allowUnknownFields Ignores unknown JSON fields. + * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. + */ + private static void fillRepeatedField( + DynamicMessage.Builder protoMsg, + FieldDescriptor field, + JSONObject json, + String currentScope, + boolean allowUnknownFields) + throws IllegalArgumentException { + String fieldName = field.getName(); + JSONArray jsonArray; + try { + jsonArray = json.getJSONArray(fieldName); + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have an array field at " + currentScope + "."); + } + + switch (field.getType()) { + case BOOL: + for (int i = 0; i < jsonArray.length(); i++) { + try { + protoMsg.addRepeatedField(field, new Boolean(jsonArray.getBoolean(i))); + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a boolean field at " + + currentScope + + "[" + + i + + "]" + + "."); + } + } + break; + case BYTES: + for (int i = 0; i < jsonArray.length(); i++) { + try { + protoMsg.addRepeatedField(field, jsonArray.getString(i).getBytes()); + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a string field at " + currentScope + "[" + i + "]" + "."); + } + } + break; + case INT64: + for (int i = 0; i < jsonArray.length(); i++) { + try { + java.lang.Object val = jsonArray.get(i); + if (val instanceof Byte) { + protoMsg.addRepeatedField(field, new Long((Byte) val)); + } else if (val instanceof Short) { + protoMsg.addRepeatedField(field, new Long((Short) val)); + } else if (val instanceof Integer) { + protoMsg.addRepeatedField(field, new Long((Integer) val)); + } else if (val instanceof Long) { + protoMsg.addRepeatedField(field, new Long((Long) val)); + } else { + throw new IllegalArgumentException( + "JSONObject does not have a int64 field at " + + currentScope + + "[" + + i + + "]" + + "."); + } + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a int64 field at " + currentScope + "[" + i + "]" + "."); + } + } + break; + case STRING: + for (int i = 0; i < jsonArray.length(); i++) { + try { + protoMsg.addRepeatedField(field, jsonArray.getString(i)); + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a string field at " + currentScope + "[" + i + "]" + "."); + } + } + break; + case DOUBLE: + for (int i = 0; i < jsonArray.length(); i++) { + try { + java.lang.Object val = jsonArray.get(i); + if (val instanceof Double) { + protoMsg.addRepeatedField(field, new Double((double) val)); + } else if (val instanceof Float) { + protoMsg.addRepeatedField(field, new Double((float) val)); + } else { + throw new IllegalArgumentException( + "JSONObject does not have a double field at " + + currentScope + + "[" + + i + + "]" + + "."); + } + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a double field at " + currentScope + "[" + i + "]" + "."); + } + } + break; + case MESSAGE: + for (int i = 0; i < jsonArray.length(); i++) { + try { + Message.Builder message = protoMsg.newBuilderForField(field); + protoMsg.addRepeatedField( + field, + convertJsonToProtoMessageImpl( + field.getMessageType(), + jsonArray.getJSONObject(i), + currentScope, + false, + allowUnknownFields)); + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have an object field at " + + currentScope + + "[" + + i + + "]" + + "."); + } + } + break; + } + } +} diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java new file mode 100644 index 0000000000..104baeb69a --- /dev/null +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -0,0 +1,625 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigquery.storage.v1alpha2; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +import com.google.cloud.bigquery.storage.test.SchemaTest.*; +import com.google.common.collect.ImmutableMap; +import com.google.protobuf.Descriptors.Descriptor; +import com.google.protobuf.Descriptors.FieldDescriptor; +import com.google.protobuf.DynamicMessage; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import org.json.JSONArray; +import org.json.JSONObject; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class JsonToProtoMessageTest { + private static ImmutableMap SimpleDescriptorsToDebugMessageTest = + new ImmutableMap.Builder() + .put(BoolType.getDescriptor(), "boolean") + .put(BytesType.getDescriptor(), "string") + .put(Int64Type.getDescriptor(), "int64") + .put(DoubleType.getDescriptor(), "double") + .put(StringType.getDescriptor(), "string") + .build(); + + private static JSONObject[] simpleJSONObjects = { + new JSONObject().put("test_field_type", 123), + new JSONObject().put("test_field_type", 1.23), + new JSONObject().put("test_field_type", true), + new JSONObject().put("test_field_type", "test") + }; + + // private static JSONObject[] simpleJSONArrays = { + // new JSONObject() + // .put( + // "test_field_type", + // new JSONArray(new Long[] {Long.MAX_VALUE, Long.MIN_VALUE, Integer.MAX_VALUE, + // Integer.MIN_VALUE, Short.MAX_VALUE, Short.MIN_VALUE, Byte.MAX_VALUE, Byte.MIN_VALUE, 0})), + // new JSONObject().put("test_field_type", new JSONArray(new Double[]{Double.MAX_VALUE, + // Double.MIN_VALUE})), + // new JSONObject().put("test_field_type", new JSONArray("[true, false]")), + // new JSONObject().put("test_field_type", new JSONArray("[hello, test]")) + // }; + + private void isProtoJsonEqual(DynamicMessage proto, JSONObject json) { + for (Map.Entry entry : proto.getAllFields().entrySet()) { + FieldDescriptor key = entry.getKey(); + java.lang.Object value = entry.getValue(); + if (key.isRepeated()) { + isProtoArrayJsonArrayEqual(key, value, json); + } else { + isProtoFieldJsonFieldEqual(key, value, json); + } + } + } + + private void isProtoFieldJsonFieldEqual( + FieldDescriptor key, java.lang.Object value, JSONObject json) { + String fieldName = key.getName(); + switch (key.getType()) { + case BOOL: + assertTrue((Boolean) value == json.getBoolean(fieldName)); + break; + case BYTES: + assertTrue(Arrays.equals((byte[]) value, json.getString(fieldName).getBytes())); + break; + case INT64: + assertTrue((long) value == json.getLong(fieldName)); + break; + case STRING: + assertTrue(((String) value).equals(json.getString(fieldName))); + break; + case DOUBLE: + assertTrue((double) value == json.getDouble(fieldName)); + break; + case MESSAGE: + isProtoJsonEqual((DynamicMessage) value, json.getJSONObject(fieldName)); + break; + } + } + + private void isProtoArrayJsonArrayEqual( + FieldDescriptor key, java.lang.Object value, JSONObject json) { + String fieldName = key.getName(); + JSONArray jsonArray = json.getJSONArray(fieldName); + switch (key.getType()) { + case BOOL: + List boolArr = (List) value; + for (int i = 0; i < jsonArray.length(); i++) { + assertTrue((boolArr.get(i) == jsonArray.getBoolean(i))); + } + break; + case BYTES: + List byteArr = (List) value; + for (int i = 0; i < jsonArray.length(); i++) { + assertTrue(Arrays.equals(byteArr.get(i), jsonArray.getString(i).getBytes())); + } + break; + case INT64: + List longArr = (List) value; + for (int i = 0; i < jsonArray.length(); i++) { + assertTrue((longArr.get(i) == jsonArray.getLong(i))); + } + break; + case STRING: + List stringArr = (List) value; + for (int i = 0; i < jsonArray.length(); i++) { + assertTrue(stringArr.get(i).equals(jsonArray.getString(i))); + } + break; + case DOUBLE: + List doubleArr = (List) value; + for (int i = 0; i < jsonArray.length(); i++) { + assertTrue((doubleArr.get(i) == jsonArray.getDouble(i))); + } + break; + case MESSAGE: + List messageArr = (List) value; + for (int i = 0; i < jsonArray.length(); i++) { + isProtoJsonEqual(messageArr.get(i), jsonArray.getJSONObject(i)); + } + break; + } + } + + @Test + public void testSimpleTypes() throws Exception { + for (Map.Entry entry : SimpleDescriptorsToDebugMessageTest.entrySet()) { + int success = 0; + for (JSONObject json : simpleJSONObjects) { + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json); + isProtoJsonEqual(protoMsg, json); + success += 1; + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), + "JSONObject does not have a " + entry.getValue() + " field at root.test_field_type."); + } + } + assertEquals(1, success); + } + } + + @Test + public void testBQSchemaToProtobufferStructSimple() throws Exception { + JSONObject stringType = new JSONObject(); + stringType.put("test_field_type", "test"); + JSONObject json = new JSONObject(); + json.put("test_field_type", stringType); + + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json); + isProtoJsonEqual(protoMsg, json); + } + + @Test + public void testBQSchemaToProtobufferStructSimpleFail() throws Exception { + JSONObject stringType = new JSONObject(); + stringType.put("test_field_type", 1); + JSONObject json = new JSONObject(); + json.put("test_field_type", stringType); + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json); + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), + "JSONObject does not have a string field at root.test_field_type.test_field_type."); + } + } + + // @Test + // public void testBQSchemaToProtobufferStructComplex() throws Exception { + // Table.TableFieldSchema bqBytes = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.BYTES) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("bytes") + // .build(); + // Table.TableFieldSchema bqInt = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.INT64) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("int") + // .build(); + // Table.TableFieldSchema record1 = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.STRUCT) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("record1") + // .addFields(0, bqInt) + // .build(); + // Table.TableFieldSchema record = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.STRUCT) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("record") + // .addFields(0, bqInt) + // .addFields(1, bqBytes) + // .addFields(2, record1) + // .build(); + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = + // Table.TableSchema.newBuilder().addFields(0, record).addFields(1, bqDouble).build(); + // + // JSONObject jsonRecord1 = new JSONObject(); + // jsonRecord1.put("int", 2048); + // JSONObject jsonRecord = new JSONObject(); + // jsonRecord.put("int", 1024); + // jsonRecord.put("bytes", "testing"); + // jsonRecord.put("record1", jsonRecord1); + // JSONObject json = new JSONObject(); + // json.put("record", jsonRecord); + // json.put("float", 1.23); + // + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // assertTrue(isProtoJsonEqual(protoMsg, json)); + // } + // + // @Test + // public void testBQSchemaToProtobufferStructComplexFail() throws Exception { + // Table.TableFieldSchema bqInt = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.INT64) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("int") + // .build(); + // Table.TableFieldSchema record = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.STRUCT) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("record") + // .addFields(0, bqInt) + // .build(); + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = + // Table.TableSchema.newBuilder().addFields(0, record).addFields(1, bqDouble).build(); + // + // JSONObject json = new JSONObject(); + // json.put("record", 1.23); + // json.put("float", 1.23); + // try { + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // } catch (IllegalArgumentException e) { + // assertEquals(e.getMessage(), "JSONObject does not have an object field at .record."); + // } + // } + // + // @Test + // public void testBQSchemaToProtobufferRepeatedSimple() throws Exception { + // for (Map.Entry entry : + // BQTableTypeToDebugMessage.entrySet()) { + // Table.TableFieldSchema tableFieldSchema = + // Table.TableFieldSchema.newBuilder() + // .setType(entry.getKey()) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("test_field_type") + // .build(); + // Table.TableSchema tableSchema = + // Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); + // int success = 0; + // for (JSONObject json : simpleJSONArrays) { + // try { + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = + // JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // success += 1; + // } catch (IllegalArgumentException e) { + // assertEquals( + // e.getMessage(), + // "JSONObject does not have a " + // + entry.getValue() + // + " field at .test_field_type[0]."); + // } + // } + // assertEquals(1, success); + // } + // } + // + // @Test + // public void testBQSchemaToProtobufferRepeatedSimpleInt64() throws Exception { + // Table.TableFieldSchema bqByte = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.INT64) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("byte") + // .build(); + // Table.TableFieldSchema bqShort = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.INT64) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("short") + // .build(); + // Table.TableFieldSchema bqInt = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.INT64) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("int") + // .build(); + // Table.TableFieldSchema bqLong = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.INT64) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("long") + // .build(); + // Table.TableSchema tableSchema = + // Table.TableSchema.newBuilder() + // .addFields(0, bqByte) + // .addFields(1, bqShort) + // .addFields(2, bqInt) + // .addFields(3, bqLong) + // .build(); + // JSONObject json = new JSONObject(); + // byte[] byteArr = {(byte) 1, (byte) 2}; + // short[] shortArr = {(short) 1, (short) 2}; + // int[] intArr = {1, 2, 3, 4, 5}; + // long[] longArr = {1L, 2L, 3L, 4L, 5L}; + // json.put("byte", new JSONArray(byteArr)); + // json.put("short", new JSONArray(shortArr)); + // json.put("int", new JSONArray(intArr)); + // json.put("long", new JSONArray(longArr)); + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // assertTrue(isProtoJsonEqual(protoMsg, json)); + // } + // + // @Test + // public void testBQSchemaToProtobufferRepeatedSimpleDouble() throws Exception { + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("double") + // .build(); + // Table.TableFieldSchema bqFloat = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = + // Table.TableSchema.newBuilder().addFields(0, bqDouble).addFields(1, bqFloat).build(); + // JSONObject json = new JSONObject(); + // double[] doubleArr = {1.1, 2.2, 3.3, 4.4, 5.5}; + // float[] floatArr = {1.1f, 2.2f, 3.3f, 4.4f, 5.5f}; + // json.put("double", new JSONArray(doubleArr)); + // json.put("float", new JSONArray(floatArr)); + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // assertTrue(isProtoJsonEqual(protoMsg, json)); + // } + // + // @Test + // public void testBQSchemaToProtobufferRepeatedSimpleFail() throws Exception { + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, + // bqDouble).build(); + // JSONObject json = new JSONObject(); + // json.put("float", new JSONArray("[1.1, 2.2, 3.3, hello, 4.4]")); + // try { + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // } catch (IllegalArgumentException e) { + // assertEquals(e.getMessage(), "JSONObject does not have a double field at .float[3]."); + // } + // } + // + // @Test + // public void testBQSchemaToProtobufferRepeatedComplex() throws Exception { + // Table.TableFieldSchema bqString = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.STRING) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("string") + // .build(); + // Table.TableFieldSchema record = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.STRUCT) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("test_field_type") + // .addFields(0, bqString) + // .build(); + // Table.TableFieldSchema bqInt = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.INT64) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("int") + // .build(); + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = + // Table.TableSchema.newBuilder() + // .addFields(0, bqDouble) + // .addFields(1, bqInt) + // .addFields(2, record) + // .build(); + // JSONObject json = new JSONObject(); + // double[] doubleArr = {1.1, 2.2, 3.3, 4.4, 5.5}; + // String[] stringArr = {"hello", "this", "is", "a", "test"}; + // int[] intArr = {1, 2, 3, 4, 5}; + // json.put("float", new JSONArray(doubleArr)); + // json.put("int", new JSONArray(intArr)); + // JSONObject jsonRecord = new JSONObject(); + // jsonRecord.put("string", new JSONArray(stringArr)); + // json.put("test_field_type", jsonRecord); + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // assertTrue(isProtoJsonEqual(protoMsg, json)); + // } + // + // @Test + // public void testBQSchemaToProtobufferRepeatedComplexFail() throws Exception { + // Table.TableFieldSchema bqInt = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.INT64) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("int") + // .build(); + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.REPEATED) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = + // Table.TableSchema.newBuilder().addFields(0, bqDouble).addFields(1, bqInt).build(); + // JSONObject json = new JSONObject(); + // int[] intArr = {1, 2, 3, 4, 5}; + // json.put("float", "1"); + // json.put("int", new JSONArray(intArr)); + // try { + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // } catch (IllegalArgumentException e) { + // assertEquals(e.getMessage(), "JSONObject does not have an array field at .float."); + // } + // } + // + // @Test + // public void testBQSchemaToProtobufferAllowUnknownFields() throws Exception { + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, + // bqDouble).build(); + // JSONObject json = new JSONObject(); + // json.put("float", 1.1); + // json.put("string", "hello"); + // + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = + // JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json, true); + // assertTrue(isProtoJsonEqual(protoMsg, json)); + // } + // + // @Test + // public void testBQSchemaToProtobufferAllowUnknownFieldsFail() throws Exception { + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, + // bqDouble).build(); + // JSONObject json = new JSONObject(); + // json.put("float", 1.1); + // json.put("string", "hello"); + // try { + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // } catch (IllegalArgumentException e) { + // assertEquals( + // e.getMessage(), + // "JSONObject has unknown fields. Set allowUnknownFields to True to ignore unknown + // fields."); + // } + // } + // + // @Test + // public void testBQSchemaToProtobufferRequiredFail() throws Exception { + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.REQUIRED) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, + // bqDouble).build(); + // JSONObject json = new JSONObject(); + // json.put("float1", 1.1); + // try { + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // } catch (IllegalArgumentException e) { + // assertEquals(e.getMessage(), "JSONObject does not have the required field .float."); + // } + // } + // + // @Test + // public void testBQSchemaToProtobufferTopLevelMatchFail() throws Exception { + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, + // bqDouble).build(); + // JSONObject json = new JSONObject(); + // json.put("float1", 1.1); + // try { + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // } catch (IllegalArgumentException e) { + // assertEquals( + // e.getMessage(), + // "There are no matching fields found for the JSONObject and the BigQuery table."); + // } + // } + // + // @Test + // public void testBQSchemaToProtobufferOptional() throws Exception { + // Table.TableFieldSchema StringType = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.STRING) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("test_field_type") + // .build(); + // Table.TableFieldSchema tableFieldSchema = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.STRUCT) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("test_field_type") + // .addFields(0, StringType) + // .build(); + // Table.TableFieldSchema actualStringType = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.STRING) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("test_field_type1") + // .build(); + // Table.TableSchema tableSchema = + // Table.TableSchema.newBuilder() + // .addFields(0, tableFieldSchema) + // .addFields(1, actualStringType) + // .build(); + // + // JSONObject stringType = new JSONObject(); + // stringType.put("test_field_type1", 1); + // JSONObject json = new JSONObject(); + // json.put("test_field_type", stringType); + // json.put("test_field_type2", "hello"); + // + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // assertTrue(isProtoJsonEqual(protoMsg, json)); + // } + // + // @Test + // public void testBQSchemaToProtobufferNullJson() throws Exception { + // Table.TableFieldSchema bqDouble = + // Table.TableFieldSchema.newBuilder() + // .setType(Table.TableFieldSchema.Type.DOUBLE) + // .setMode(Table.TableFieldSchema.Mode.NULLABLE) + // .setName("float") + // .build(); + // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, + // bqDouble).build(); + // JSONObject json = new JSONObject(); + // json.put("float", JSONObject.NULL); + // try { + // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); + // } catch (IllegalArgumentException e) { + // assertEquals(e.getMessage(), "JSONObject does not have a double field at .float."); + // } + // } +} From d9530a43cd566c1f320511172a98b10d44b4c299 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Wed, 8 Jul 2020 11:31:53 -0500 Subject: [PATCH 02/21] Added more test cases --- .../storage/v1alpha2/JsonToProtoMessage.java | 11 +- .../v1alpha2/JsonToProtoMessageTest.java | 308 +++++++----------- .../src/test/proto/jsonTest.proto | 44 +++ 3 files changed, 171 insertions(+), 192 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index 2257ad667e..eb18f780af 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -200,7 +200,7 @@ private static void fillField( allowUnknownFields)); } catch (JSONException e) { throw new IllegalArgumentException( - "JSONObject does not have an object field at " + currentScope + "."); + "JSONObject does not have a object field at " + currentScope + "."); } break; } @@ -229,7 +229,7 @@ private static void fillRepeatedField( jsonArray = json.getJSONArray(fieldName); } catch (JSONException e) { throw new IllegalArgumentException( - "JSONObject does not have an array field at " + currentScope + "."); + "JSONObject does not have a array field at " + currentScope + "."); } switch (field.getType()) { @@ -332,12 +332,7 @@ private static void fillRepeatedField( allowUnknownFields)); } catch (JSONException e) { throw new IllegalArgumentException( - "JSONObject does not have an object field at " - + currentScope - + "[" - + i - + "]" - + "."); + "JSONObject does not have a object field at " + currentScope + "[" + i + "]" + "."); } } break; diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index 104baeb69a..5682e8a8f3 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.*; import static org.mockito.Mockito.*; +import com.google.cloud.bigquery.storage.test.JsonTest.*; import com.google.cloud.bigquery.storage.test.SchemaTest.*; import com.google.common.collect.ImmutableMap; import com.google.protobuf.Descriptors.Descriptor; @@ -34,33 +35,73 @@ @RunWith(JUnit4.class) public class JsonToProtoMessageTest { - private static ImmutableMap SimpleDescriptorsToDebugMessageTest = + private static ImmutableMap AllTypesToDebugMessageTest = new ImmutableMap.Builder() .put(BoolType.getDescriptor(), "boolean") .put(BytesType.getDescriptor(), "string") .put(Int64Type.getDescriptor(), "int64") .put(DoubleType.getDescriptor(), "double") .put(StringType.getDescriptor(), "string") + .put(RepeatedType.getDescriptor(), "array") + .put(ObjectType.getDescriptor(), "object") + .build(); + + private static ImmutableMap AllRepeatedTypesToDebugMessageTest = + new ImmutableMap.Builder() + .put(RepeatedBool.getDescriptor(), "boolean") + .put(RepeatedBytes.getDescriptor(), "string") + .put(RepeatedInt64.getDescriptor(), "int64") + .put(RepeatedDouble.getDescriptor(), "double") + .put(RepeatedObject.getDescriptor(), "object") .build(); private static JSONObject[] simpleJSONObjects = { new JSONObject().put("test_field_type", 123), new JSONObject().put("test_field_type", 1.23), new JSONObject().put("test_field_type", true), - new JSONObject().put("test_field_type", "test") + new JSONObject().put("test_field_type", "test"), + new JSONObject().put("test_field_type", new JSONArray("[1, 2, 3]")), + new JSONObject().put("test_field_type", new JSONObject().put("test_int", 1)) }; - // private static JSONObject[] simpleJSONArrays = { - // new JSONObject() - // .put( - // "test_field_type", - // new JSONArray(new Long[] {Long.MAX_VALUE, Long.MIN_VALUE, Integer.MAX_VALUE, - // Integer.MIN_VALUE, Short.MAX_VALUE, Short.MIN_VALUE, Byte.MAX_VALUE, Byte.MIN_VALUE, 0})), - // new JSONObject().put("test_field_type", new JSONArray(new Double[]{Double.MAX_VALUE, - // Double.MIN_VALUE})), - // new JSONObject().put("test_field_type", new JSONArray("[true, false]")), - // new JSONObject().put("test_field_type", new JSONArray("[hello, test]")) - // }; + private static JSONObject[] simpleJSONArrays = { + new JSONObject() + .put( + "test_repeated", + new JSONArray( + new Long[] { + Long.MAX_VALUE, + Long.MIN_VALUE, + (long) Integer.MAX_VALUE, + (long) Integer.MIN_VALUE, + (long) Short.MAX_VALUE, + (long) Short.MIN_VALUE, + (long) Byte.MAX_VALUE, + (long) Byte.MIN_VALUE, + 0L + })), + new JSONObject() + .put( + "test_repeated", + new JSONArray( + new Double[] { + Double.MAX_VALUE, + Double.MIN_VALUE, + (double) Float.MAX_VALUE, + (double) Float.MIN_VALUE + })), + new JSONObject().put("test_repeated", new JSONArray(new Boolean[] {true, false})), + new JSONObject().put("test_repeated", new JSONArray(new String[] {"hello", "test"})), + new JSONObject() + .put( + "test_repeated", + new JSONArray( + new JSONObject[] { + new JSONObject().put("test_int", 1), + new JSONObject().put("test_int", 2), + new JSONObject().put("test_int", 3) + })) + }; private void isProtoJsonEqual(DynamicMessage proto, JSONObject json) { for (Map.Entry entry : proto.getAllFields().entrySet()) { @@ -144,8 +185,8 @@ private void isProtoArrayJsonArrayEqual( } @Test - public void testSimpleTypes() throws Exception { - for (Map.Entry entry : SimpleDescriptorsToDebugMessageTest.entrySet()) { + public void testAllTypes() throws Exception { + for (Map.Entry entry : AllTypesToDebugMessageTest.entrySet()) { int success = 0; for (JSONObject json : simpleJSONObjects) { try { @@ -164,7 +205,28 @@ public void testSimpleTypes() throws Exception { } @Test - public void testBQSchemaToProtobufferStructSimple() throws Exception { + public void testRepeatedWithLimits() throws Exception { + for (Map.Entry entry : AllRepeatedTypesToDebugMessageTest.entrySet()) { + int success = 0; + for (JSONObject json : simpleJSONArrays) { + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json); + success += 1; + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), + "JSONObject does not have a " + + entry.getValue() + + " field at root.test_repeated[0]."); + } + } + assertEquals(1, success); + } + } + + @Test + public void testStructSimple() throws Exception { JSONObject stringType = new JSONObject(); stringType.put("test_field_type", "test"); JSONObject json = new JSONObject(); @@ -176,7 +238,7 @@ public void testBQSchemaToProtobufferStructSimple() throws Exception { } @Test - public void testBQSchemaToProtobufferStructSimpleFail() throws Exception { + public void testStructSimpleFail() throws Exception { JSONObject stringType = new JSONObject(); stringType.put("test_field_type", 1); JSONObject json = new JSONObject(); @@ -191,173 +253,51 @@ public void testBQSchemaToProtobufferStructSimpleFail() throws Exception { } } - // @Test - // public void testBQSchemaToProtobufferStructComplex() throws Exception { - // Table.TableFieldSchema bqBytes = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.BYTES) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("bytes") - // .build(); - // Table.TableFieldSchema bqInt = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.INT64) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("int") - // .build(); - // Table.TableFieldSchema record1 = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.STRUCT) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("record1") - // .addFields(0, bqInt) - // .build(); - // Table.TableFieldSchema record = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.STRUCT) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("record") - // .addFields(0, bqInt) - // .addFields(1, bqBytes) - // .addFields(2, record1) - // .build(); - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = - // Table.TableSchema.newBuilder().addFields(0, record).addFields(1, bqDouble).build(); - // - // JSONObject jsonRecord1 = new JSONObject(); - // jsonRecord1.put("int", 2048); - // JSONObject jsonRecord = new JSONObject(); - // jsonRecord.put("int", 1024); - // jsonRecord.put("bytes", "testing"); - // jsonRecord.put("record1", jsonRecord1); - // JSONObject json = new JSONObject(); - // json.put("record", jsonRecord); - // json.put("float", 1.23); - // - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // assertTrue(isProtoJsonEqual(protoMsg, json)); - // } - // - // @Test - // public void testBQSchemaToProtobufferStructComplexFail() throws Exception { - // Table.TableFieldSchema bqInt = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.INT64) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("int") - // .build(); - // Table.TableFieldSchema record = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.STRUCT) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("record") - // .addFields(0, bqInt) - // .build(); - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = - // Table.TableSchema.newBuilder().addFields(0, record).addFields(1, bqDouble).build(); - // - // JSONObject json = new JSONObject(); - // json.put("record", 1.23); - // json.put("float", 1.23); - // try { - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // } catch (IllegalArgumentException e) { - // assertEquals(e.getMessage(), "JSONObject does not have an object field at .record."); - // } - // } - // - // @Test - // public void testBQSchemaToProtobufferRepeatedSimple() throws Exception { - // for (Map.Entry entry : - // BQTableTypeToDebugMessage.entrySet()) { - // Table.TableFieldSchema tableFieldSchema = - // Table.TableFieldSchema.newBuilder() - // .setType(entry.getKey()) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("test_field_type") - // .build(); - // Table.TableSchema tableSchema = - // Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); - // int success = 0; - // for (JSONObject json : simpleJSONArrays) { - // try { - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = - // JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // success += 1; - // } catch (IllegalArgumentException e) { - // assertEquals( - // e.getMessage(), - // "JSONObject does not have a " - // + entry.getValue() - // + " field at .test_field_type[0]."); - // } - // } - // assertEquals(1, success); - // } - // } - // - // @Test - // public void testBQSchemaToProtobufferRepeatedSimpleInt64() throws Exception { - // Table.TableFieldSchema bqByte = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.INT64) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("byte") - // .build(); - // Table.TableFieldSchema bqShort = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.INT64) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("short") - // .build(); - // Table.TableFieldSchema bqInt = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.INT64) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("int") - // .build(); - // Table.TableFieldSchema bqLong = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.INT64) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("long") - // .build(); - // Table.TableSchema tableSchema = - // Table.TableSchema.newBuilder() - // .addFields(0, bqByte) - // .addFields(1, bqShort) - // .addFields(2, bqInt) - // .addFields(3, bqLong) - // .build(); - // JSONObject json = new JSONObject(); - // byte[] byteArr = {(byte) 1, (byte) 2}; - // short[] shortArr = {(short) 1, (short) 2}; - // int[] intArr = {1, 2, 3, 4, 5}; - // long[] longArr = {1L, 2L, 3L, 4L, 5L}; - // json.put("byte", new JSONArray(byteArr)); - // json.put("short", new JSONArray(shortArr)); - // json.put("int", new JSONArray(intArr)); - // json.put("long", new JSONArray(longArr)); - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // assertTrue(isProtoJsonEqual(protoMsg, json)); - // } - // + @Test + public void testStructComplex() throws Exception { + JSONObject complexLvl2 = new JSONObject(); + complexLvl2.put("test_int", 3); + + JSONObject complexLvl1 = new JSONObject(); + complexLvl1.put("test_int", 2); + complexLvl1.put("complexLvl2", complexLvl2); + + JSONObject json = new JSONObject(); + json.put("test_int", 1); + json.put("test_string", new JSONArray(new String[] {"a", "b", "c"})); + json.put("test_bytes", "hello"); + json.put("test_bool", true); + json.put("test_double", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4})); + json.put("complexLvl1", complexLvl1); + json.put("complexLvl2", complexLvl2); + + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json); + isProtoJsonEqual(protoMsg, json); + } + + @Test + public void testInt64() throws Exception { + JSONObject json = new JSONObject(); + json.put("byte", (byte) 1); + json.put("short", (short) 1); + json.put("int", 1); + json.put("long", 1L); + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + isProtoJsonEqual(protoMsg, json); + } + + @Test + public void testDouble() throws Exception { + JSONObject json = new JSONObject(); + json.put("double", 1.2); + json.put("float", 3.4f); + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestDouble.getDescriptor(), json); + isProtoJsonEqual(protoMsg, json); + } + // @Test // public void testBQSchemaToProtobufferRepeatedSimpleDouble() throws Exception { // Table.TableFieldSchema bqDouble = diff --git a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto index 26a3499c23..ab679a553c 100644 --- a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto +++ b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto @@ -32,6 +32,14 @@ message ComplexLvl2 { optional int64 test_int = 1; } +message ObjectType { + optional ComplexLvl2 test_field_type = 1; +} + +message RepeatedType { + repeated int64 test_field_type = 1; +} + message OptionTest { optional int64 test_optional = 1; required int64 test_required = 2; @@ -52,3 +60,39 @@ message ReuseLvl1 { message ReuseLvl2 { optional int64 test_int = 1; } + +message RepeatedInt64 { + repeated int64 test_repeated = 1; +} + +message RepeatedDouble { + repeated double test_repeated = 1; +} + +message RepeatedString { + repeated string test_repeated = 1; +} + +message RepeatedBool { + repeated bool test_repeated = 1; +} + +message RepeatedBytes { + repeated bytes test_repeated = 1; +} + +message RepeatedObject { + repeated ComplexLvl2 test_repeated = 1; +} + +message TestInt64 { + optional int64 byte = 1; + optional int64 short = 2; + optional int64 int = 3; + optional int64 long = 4; +} + +message TestDouble { + optional double double = 1; + optional float float = 2; +} From 16dd0bb82ea0b9355cc1f99f250b6c6c33d38d09 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Wed, 8 Jul 2020 13:15:16 -0500 Subject: [PATCH 03/21] Draft version of JsonToProtoMessage --- .../v1alpha2/JsonToProtoMessageTest.java | 473 +++++++----------- .../src/test/proto/jsonTest.proto | 11 + 2 files changed, 201 insertions(+), 283 deletions(-) diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index 5682e8a8f3..151f330edd 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -103,7 +103,7 @@ public class JsonToProtoMessageTest { })) }; - private void isProtoJsonEqual(DynamicMessage proto, JSONObject json) { + private void AreMatchingFieldsFilledIn(DynamicMessage proto, JSONObject json) { for (Map.Entry entry : proto.getAllFields().entrySet()) { FieldDescriptor key = entry.getKey(); java.lang.Object value = entry.getValue(); @@ -135,7 +135,7 @@ private void isProtoFieldJsonFieldEqual( assertTrue((double) value == json.getDouble(fieldName)); break; case MESSAGE: - isProtoJsonEqual((DynamicMessage) value, json.getJSONObject(fieldName)); + AreMatchingFieldsFilledIn((DynamicMessage) value, json.getJSONObject(fieldName)); break; } } @@ -178,12 +178,34 @@ private void isProtoArrayJsonArrayEqual( case MESSAGE: List messageArr = (List) value; for (int i = 0; i < jsonArray.length(); i++) { - isProtoJsonEqual(messageArr.get(i), jsonArray.getJSONObject(i)); + AreMatchingFieldsFilledIn(messageArr.get(i), jsonArray.getJSONObject(i)); } break; } } + @Test + public void testInt64() throws Exception { + JSONObject json = new JSONObject(); + json.put("byte", (byte) 1); + json.put("short", (short) 1); + json.put("int", 1); + json.put("long", 1L); + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + AreMatchingFieldsFilledIn(protoMsg, json); + } + + @Test + public void testDouble() throws Exception { + JSONObject json = new JSONObject(); + json.put("double", 1.2); + json.put("float", 3.4f); + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestDouble.getDescriptor(), json); + AreMatchingFieldsFilledIn(protoMsg, json); + } + @Test public void testAllTypes() throws Exception { for (Map.Entry entry : AllTypesToDebugMessageTest.entrySet()) { @@ -192,7 +214,7 @@ public void testAllTypes() throws Exception { try { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json); - isProtoJsonEqual(protoMsg, json); + AreMatchingFieldsFilledIn(protoMsg, json); success += 1; } catch (IllegalArgumentException e) { assertEquals( @@ -205,7 +227,7 @@ public void testAllTypes() throws Exception { } @Test - public void testRepeatedWithLimits() throws Exception { + public void testAllRepeatedTypesWithLimits() throws Exception { for (Map.Entry entry : AllRepeatedTypesToDebugMessageTest.entrySet()) { int success = 0; for (JSONObject json : simpleJSONArrays) { @@ -225,6 +247,30 @@ public void testRepeatedWithLimits() throws Exception { } } + @Test + public void testOptional() throws Exception { + JSONObject json = new JSONObject(); + json.put("byte", 1); + + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + AreMatchingFieldsFilledIn(protoMsg, json); + } + + @Test + public void testRequired() throws Exception { + JSONObject json = new JSONObject(); + json.put("test_required_float", 1.1); + json.put("optional_double", 1.1); + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestRequired.getDescriptor(), json); + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), "JSONObject does not have the required field root.required_float."); + } + } + @Test public void testStructSimple() throws Exception { JSONObject stringType = new JSONObject(); @@ -234,7 +280,7 @@ public void testStructSimple() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json); - isProtoJsonEqual(protoMsg, json); + AreMatchingFieldsFilledIn(protoMsg, json); } @Test @@ -273,293 +319,154 @@ public void testStructComplex() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json); - isProtoJsonEqual(protoMsg, json); + AreMatchingFieldsFilledIn(protoMsg, json); } @Test - public void testInt64() throws Exception { + public void testStructComplexFail() throws Exception { + JSONObject complexLvl2 = new JSONObject(); + complexLvl2.put("test_int", 3); + + JSONObject complexLvl1 = new JSONObject(); + complexLvl1.put("test_int", "not_int"); + complexLvl1.put("complexLvl2", complexLvl2); + JSONObject json = new JSONObject(); - json.put("byte", (byte) 1); - json.put("short", (short) 1); - json.put("int", 1); - json.put("long", 1L); + json.put("test_int", 1); + json.put("test_string", new JSONArray(new String[] {"a", "b", "c"})); + json.put("test_bytes", "hello"); + json.put("test_bool", true); + json.put("test_double", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4})); + json.put("complexLvl1", complexLvl1); + json.put("complexLvl2", complexLvl2); + + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json); + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), "JSONObject does not have a int64 field at root.complexLvl1.test_int."); + } + } + + @Test + public void testRepeatedWithMixedTypes() throws Exception { + JSONObject json = new JSONObject(); + json.put("test_repeated", new JSONArray("[1.1, 2.2, true]")); + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(RepeatedDouble.getDescriptor(), json); + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), "JSONObject does not have a double field at root.test_repeated[2]."); + } + } + + @Test + public void testNestedRepeatedComplex() throws Exception { + double[] doubleArr = {1.1, 2.2, 3.3, 4.4, 5.5}; + String[] stringArr = {"hello", "this", "is", "a", "test"}; + int[] intArr = {1, 2, 3, 4, 5}; + + JSONObject json = new JSONObject(); + json.put("double", new JSONArray(doubleArr)); + json.put("int", new JSONArray(intArr)); + JSONObject jsonRepeatedString = new JSONObject(); + jsonRepeatedString.put("test_repeated", new JSONArray(stringArr)); + json.put("repeated_string", jsonRepeatedString); + DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); - isProtoJsonEqual(protoMsg, json); + JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json); + AreMatchingFieldsFilledIn(protoMsg, json); } @Test - public void testDouble() throws Exception { + public void testNestedRepeatedComplexFail() throws Exception { + double[] doubleArr = {1.1, 2.2, 3.3, 4.4, 5.5}; + Boolean[] fakeStringArr = {true, false}; + int[] intArr = {1, 2, 3, 4, 5}; + JSONObject json = new JSONObject(); - json.put("double", 1.2); - json.put("float", 3.4f); + json.put("double", new JSONArray(doubleArr)); + json.put("int", new JSONArray(intArr)); + JSONObject jsonRepeatedString = new JSONObject(); + jsonRepeatedString.put("test_repeated", new JSONArray(fakeStringArr)); + json.put("repeated_string", jsonRepeatedString); + + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json); + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), + "JSONObject does not have a string field at root.repeated_string.test_repeated[0]."); + } + } + + @Test + public void testAllowUnknownFields() throws Exception { + JSONObject json = new JSONObject(); + json.put("test_repeated", new JSONArray(new int[] {1, 2, 3, 4, 5})); + json.put("string", "hello"); + DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestDouble.getDescriptor(), json); - isProtoJsonEqual(protoMsg, json); + JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json, true); + AreMatchingFieldsFilledIn(protoMsg, json); } - // @Test - // public void testBQSchemaToProtobufferRepeatedSimpleDouble() throws Exception { - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("double") - // .build(); - // Table.TableFieldSchema bqFloat = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = - // Table.TableSchema.newBuilder().addFields(0, bqDouble).addFields(1, bqFloat).build(); - // JSONObject json = new JSONObject(); - // double[] doubleArr = {1.1, 2.2, 3.3, 4.4, 5.5}; - // float[] floatArr = {1.1f, 2.2f, 3.3f, 4.4f, 5.5f}; - // json.put("double", new JSONArray(doubleArr)); - // json.put("float", new JSONArray(floatArr)); - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // assertTrue(isProtoJsonEqual(protoMsg, json)); - // } - // - // @Test - // public void testBQSchemaToProtobufferRepeatedSimpleFail() throws Exception { - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, - // bqDouble).build(); - // JSONObject json = new JSONObject(); - // json.put("float", new JSONArray("[1.1, 2.2, 3.3, hello, 4.4]")); - // try { - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // } catch (IllegalArgumentException e) { - // assertEquals(e.getMessage(), "JSONObject does not have a double field at .float[3]."); - // } - // } - // - // @Test - // public void testBQSchemaToProtobufferRepeatedComplex() throws Exception { - // Table.TableFieldSchema bqString = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.STRING) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("string") - // .build(); - // Table.TableFieldSchema record = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.STRUCT) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("test_field_type") - // .addFields(0, bqString) - // .build(); - // Table.TableFieldSchema bqInt = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.INT64) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("int") - // .build(); - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = - // Table.TableSchema.newBuilder() - // .addFields(0, bqDouble) - // .addFields(1, bqInt) - // .addFields(2, record) - // .build(); - // JSONObject json = new JSONObject(); - // double[] doubleArr = {1.1, 2.2, 3.3, 4.4, 5.5}; - // String[] stringArr = {"hello", "this", "is", "a", "test"}; - // int[] intArr = {1, 2, 3, 4, 5}; - // json.put("float", new JSONArray(doubleArr)); - // json.put("int", new JSONArray(intArr)); - // JSONObject jsonRecord = new JSONObject(); - // jsonRecord.put("string", new JSONArray(stringArr)); - // json.put("test_field_type", jsonRecord); - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // assertTrue(isProtoJsonEqual(protoMsg, json)); - // } - // - // @Test - // public void testBQSchemaToProtobufferRepeatedComplexFail() throws Exception { - // Table.TableFieldSchema bqInt = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.INT64) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("int") - // .build(); - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.REPEATED) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = - // Table.TableSchema.newBuilder().addFields(0, bqDouble).addFields(1, bqInt).build(); - // JSONObject json = new JSONObject(); - // int[] intArr = {1, 2, 3, 4, 5}; - // json.put("float", "1"); - // json.put("int", new JSONArray(intArr)); - // try { - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // } catch (IllegalArgumentException e) { - // assertEquals(e.getMessage(), "JSONObject does not have an array field at .float."); - // } - // } - // - // @Test - // public void testBQSchemaToProtobufferAllowUnknownFields() throws Exception { - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, - // bqDouble).build(); - // JSONObject json = new JSONObject(); - // json.put("float", 1.1); - // json.put("string", "hello"); - // - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = - // JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json, true); - // assertTrue(isProtoJsonEqual(protoMsg, json)); - // } - // - // @Test - // public void testBQSchemaToProtobufferAllowUnknownFieldsFail() throws Exception { - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, - // bqDouble).build(); - // JSONObject json = new JSONObject(); - // json.put("float", 1.1); - // json.put("string", "hello"); - // try { - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // } catch (IllegalArgumentException e) { - // assertEquals( - // e.getMessage(), - // "JSONObject has unknown fields. Set allowUnknownFields to True to ignore unknown - // fields."); - // } - // } - // - // @Test - // public void testBQSchemaToProtobufferRequiredFail() throws Exception { - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.REQUIRED) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, - // bqDouble).build(); - // JSONObject json = new JSONObject(); - // json.put("float1", 1.1); - // try { - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // } catch (IllegalArgumentException e) { - // assertEquals(e.getMessage(), "JSONObject does not have the required field .float."); - // } - // } - // - // @Test - // public void testBQSchemaToProtobufferTopLevelMatchFail() throws Exception { - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, - // bqDouble).build(); - // JSONObject json = new JSONObject(); - // json.put("float1", 1.1); - // try { - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // } catch (IllegalArgumentException e) { - // assertEquals( - // e.getMessage(), - // "There are no matching fields found for the JSONObject and the BigQuery table."); - // } - // } - // - // @Test - // public void testBQSchemaToProtobufferOptional() throws Exception { - // Table.TableFieldSchema StringType = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.STRING) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("test_field_type") - // .build(); - // Table.TableFieldSchema tableFieldSchema = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.STRUCT) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("test_field_type") - // .addFields(0, StringType) - // .build(); - // Table.TableFieldSchema actualStringType = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.STRING) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("test_field_type1") - // .build(); - // Table.TableSchema tableSchema = - // Table.TableSchema.newBuilder() - // .addFields(0, tableFieldSchema) - // .addFields(1, actualStringType) - // .build(); - // - // JSONObject stringType = new JSONObject(); - // stringType.put("test_field_type1", 1); - // JSONObject json = new JSONObject(); - // json.put("test_field_type", stringType); - // json.put("test_field_type2", "hello"); - // - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // assertTrue(isProtoJsonEqual(protoMsg, json)); - // } - // - // @Test - // public void testBQSchemaToProtobufferNullJson() throws Exception { - // Table.TableFieldSchema bqDouble = - // Table.TableFieldSchema.newBuilder() - // .setType(Table.TableFieldSchema.Type.DOUBLE) - // .setMode(Table.TableFieldSchema.Mode.NULLABLE) - // .setName("float") - // .build(); - // Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, - // bqDouble).build(); - // JSONObject json = new JSONObject(); - // json.put("float", JSONObject.NULL); - // try { - // Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - // DynamicMessage protoMsg = JsonToProtoConverter.protoSchemaToProtoMessage(descriptor, json); - // } catch (IllegalArgumentException e) { - // assertEquals(e.getMessage(), "JSONObject does not have a double field at .float."); - // } - // } + @Test + public void testAllowUnknownFieldsError() throws Exception { + JSONObject json = new JSONObject(); + json.put("double", 1.1); + json.put("string", "hello"); + + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json); + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), + "JSONObject has unknown fields. Set allowUnknownFields to True to ignore unknown fields."); + } + } + + @Test + public void testTopLevelMatchFail() throws Exception { + JSONObject json = new JSONObject(); + json.put("double", 1.1); + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), json); + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), + "There are no matching fields found for the JSONObject and the protocol buffer descriptor."); + } + } + + @Test + public void testTopLevelMatchSecondLevelNoMatch() throws Exception { + JSONObject complexLvl2 = new JSONObject(); + complexLvl2.put("no_match", 1); + JSONObject json = new JSONObject(); + json.put("test_int", 1); + json.put("complexLvl2", complexLvl2); + + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json); + AreMatchingFieldsFilledIn(protoMsg, json); + } + + @Test + public void testJsonNullValue() throws Exception { + JSONObject json = new JSONObject(); + json.put("long", JSONObject.NULL); + json.put("int", 1); + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "JSONObject does not have a int64 field at root.long."); + } + } } diff --git a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto index ab679a553c..670e5e9b17 100644 --- a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto +++ b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto @@ -96,3 +96,14 @@ message TestDouble { optional double double = 1; optional float float = 2; } + +message NestedRepeated { + repeated int64 int = 1; + repeated double double = 2; + optional RepeatedString repeated_string = 3; +} + +message TestRequired { + optional double optional_double = 1; + required float required_float = 2; +} From 87489a08ed2ca391bdeea5e7bf61ef1c089d3863 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Wed, 8 Jul 2020 14:58:31 -0500 Subject: [PATCH 04/21] Fixed float types in jsonTest.proto, and added a test that checks if repeated types are optional --- .../storage/v1alpha2/JsonToProtoMessageTest.java | 14 ++++++++++++-- .../src/test/proto/jsonTest.proto | 9 +++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index 151f330edd..5468ba92eb 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -257,17 +257,27 @@ public void testOptional() throws Exception { AreMatchingFieldsFilledIn(protoMsg, json); } + @Test + public void testRepeatedIsOptional() throws Exception { + JSONObject json = new JSONObject(); + json.put("required_double", 1.1); + + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestRepeatedIsOptional.getDescriptor(), json); + AreMatchingFieldsFilledIn(protoMsg, json); + } + @Test public void testRequired() throws Exception { JSONObject json = new JSONObject(); - json.put("test_required_float", 1.1); + json.put("test_required_double", 1.1); json.put("optional_double", 1.1); try { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestRequired.getDescriptor(), json); } catch (IllegalArgumentException e) { assertEquals( - e.getMessage(), "JSONObject does not have the required field root.required_float."); + e.getMessage(), "JSONObject does not have the required field root.required_double."); } } diff --git a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto index 670e5e9b17..43517e369f 100644 --- a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto +++ b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto @@ -94,7 +94,7 @@ message TestInt64 { message TestDouble { optional double double = 1; - optional float float = 2; + optional double float = 2; } message NestedRepeated { @@ -105,5 +105,10 @@ message NestedRepeated { message TestRequired { optional double optional_double = 1; - required float required_float = 2; + required double required_double = 2; +} + +message TestRepeatedIsOptional { + optional double required_double = 1; + repeated double repeated_double = 2; } From ce5c045adcc9c1e6e89f605851735aa099850413 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Thu, 9 Jul 2020 12:11:44 -0500 Subject: [PATCH 05/21] Fix according to PR (added case insensitive feature, added support to int32 for BQDate) --- .../storage/v1alpha2/JsonToProtoMessage.java | 149 +++++++++++++----- .../v1alpha2/JsonToProtoMessageTest.java | 106 +++++++++++-- .../src/test/proto/jsonTest.proto | 10 ++ 3 files changed, 211 insertions(+), 54 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index eb18f780af..46cdacad77 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -19,6 +19,7 @@ import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.DynamicMessage; import com.google.protobuf.Message; +import java.util.HashMap; import java.util.List; import org.json.JSONArray; import org.json.JSONException; @@ -77,15 +78,21 @@ private static DynamicMessage convertJsonToProtoMessageImpl( if (JSONObject.getNames(json).length > protoFields.size()) { if (!allowUnknownFields) { throw new IllegalArgumentException( - "JSONObject has unknown fields. Set allowUnknownFields to True to ignore unknown fields."); + "JSONObject has fields unknown to BigQuery: f1. Set allowUnknownFields to True to allow unknown fields."); } } + HashMap jsonLowercaseNameToActualName = new HashMap(); + String[] actualNames = JSONObject.getNames(json); + for (int i = 0; i < actualNames.length; i++) { + jsonLowercaseNameToActualName.put(actualNames[i].toLowerCase(), actualNames[i]); + } for (FieldDescriptor field : protoFields) { String fieldName = field.getName(); + String lowercaseFieldName = fieldName.toLowerCase(); String currentScope = jsonScope + "." + fieldName; - if (!json.has(fieldName)) { + if (!jsonLowercaseNameToActualName.containsKey(lowercaseFieldName)) { if (field.isRequired()) { throw new IllegalArgumentException( "JSONObject does not have the required field " + currentScope + "."); @@ -95,9 +102,21 @@ private static DynamicMessage convertJsonToProtoMessageImpl( } matchedFields++; if (!field.isRepeated()) { - fillField(protoMsg, field, json, currentScope, allowUnknownFields); + fillField( + protoMsg, + field, + json, + jsonLowercaseNameToActualName.get(lowercaseFieldName), + currentScope, + allowUnknownFields); } else { - fillRepeatedField(protoMsg, field, json, currentScope, allowUnknownFields); + fillRepeatedField( + protoMsg, + field, + json, + jsonLowercaseNameToActualName.get(lowercaseFieldName), + currentScope, + allowUnknownFields); } } if (matchedFields == 0 && topLevel) { @@ -110,26 +129,27 @@ private static DynamicMessage convertJsonToProtoMessageImpl( /** * Fills a non-repetaed protoField with the json data. * - * @param protoMsg - * @param field + * @param protoMsg The protocol buffer message being constructed + * @param fieldDescriptor * @param json + * @param actualJsonKeyName Actual key name in JSONObject instead of lowercased version * @param currentScope Debugging purposes * @param allowUnknownFields Ignores unknown JSON fields. * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. */ private static void fillField( DynamicMessage.Builder protoMsg, - FieldDescriptor field, + FieldDescriptor fieldDescriptor, JSONObject json, + String actualJsonKeyName, String currentScope, boolean allowUnknownFields) throws IllegalArgumentException { - String fieldName = field.getName(); - switch (field.getType()) { + switch (fieldDescriptor.getType()) { case BOOL: try { - protoMsg.setField(field, new Boolean(json.getBoolean(fieldName))); + protoMsg.setField(fieldDescriptor, new Boolean(json.getBoolean(actualJsonKeyName))); } catch (JSONException e) { throw new IllegalArgumentException( "JSONObject does not have a boolean field at " + currentScope + "."); @@ -137,7 +157,7 @@ private static void fillField( break; case BYTES: try { - protoMsg.setField(field, json.getString(fieldName).getBytes()); + protoMsg.setField(fieldDescriptor, json.getString(actualJsonKeyName).getBytes()); } catch (JSONException e) { throw new IllegalArgumentException( "JSONObject does not have a string field at " + currentScope + "."); @@ -145,15 +165,15 @@ private static void fillField( break; case INT64: try { - java.lang.Object val = json.get(fieldName); + java.lang.Object val = json.get(actualJsonKeyName); if (val instanceof Byte) { - protoMsg.setField(field, new Long((Byte) val)); + protoMsg.setField(fieldDescriptor, new Long((Byte) val)); } else if (val instanceof Short) { - protoMsg.setField(field, new Long((Short) val)); + protoMsg.setField(fieldDescriptor, new Long((Short) val)); } else if (val instanceof Integer) { - protoMsg.setField(field, new Long((Integer) val)); + protoMsg.setField(fieldDescriptor, new Long((Integer) val)); } else if (val instanceof Long) { - protoMsg.setField(field, new Long((Long) val)); + protoMsg.setField(fieldDescriptor, new Long((Long) val)); } else { throw new IllegalArgumentException( "JSONObject does not have a int64 field at " + currentScope + "."); @@ -163,9 +183,27 @@ private static void fillField( "JSONObject does not have a int64 field at " + currentScope + "."); } break; + case INT32: + try { + java.lang.Object val = json.get(actualJsonKeyName); + if (val instanceof Byte) { + protoMsg.setField(fieldDescriptor, new Integer((Byte) val)); + } else if (val instanceof Short) { + protoMsg.setField(fieldDescriptor, new Integer((Short) val)); + } else if (val instanceof Integer) { + protoMsg.setField(fieldDescriptor, new Integer((Integer) val)); + } else { + throw new IllegalArgumentException( + "JSONObject does not have a int32 field at " + currentScope + "."); + } + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a int32 field at " + currentScope + "."); + } + break; case STRING: try { - protoMsg.setField(field, json.getString(fieldName)); + protoMsg.setField(fieldDescriptor, json.getString(actualJsonKeyName)); } catch (JSONException e) { throw new IllegalArgumentException( "JSONObject does not have a string field at " + currentScope + "."); @@ -173,11 +211,11 @@ private static void fillField( break; case DOUBLE: try { - java.lang.Object val = json.get(fieldName); + java.lang.Object val = json.get(actualJsonKeyName); if (val instanceof Double) { - protoMsg.setField(field, new Double((double) val)); + protoMsg.setField(fieldDescriptor, new Double((double) val)); } else if (val instanceof Float) { - protoMsg.setField(field, new Double((float) val)); + protoMsg.setField(fieldDescriptor, new Double((float) val)); } else { throw new IllegalArgumentException( "JSONObject does not have a double field at " + currentScope + "."); @@ -188,13 +226,13 @@ private static void fillField( } break; case MESSAGE: - Message.Builder message = protoMsg.newBuilderForField(field); + Message.Builder message = protoMsg.newBuilderForField(fieldDescriptor); try { protoMsg.setField( - field, + fieldDescriptor, convertJsonToProtoMessageImpl( - field.getMessageType(), - json.getJSONObject(fieldName), + fieldDescriptor.getMessageType(), + json.getJSONObject(actualJsonKeyName), currentScope, false, allowUnknownFields)); @@ -209,34 +247,36 @@ private static void fillField( /** * Fills a repeated protoField with the json data. * - * @param protoMsg - * @param field + * @param protoMsg The protocol buffer message being constructed + * @param fieldDescriptor * @param json If root level has no matching fields, throws exception. + * @param actualJsonKeyName Actual key name in JSONObject instead of lowercased version * @param currentScope Debugging purposes * @param allowUnknownFields Ignores unknown JSON fields. * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. */ private static void fillRepeatedField( DynamicMessage.Builder protoMsg, - FieldDescriptor field, + FieldDescriptor fieldDescriptor, JSONObject json, + String actualJsonKeyName, String currentScope, boolean allowUnknownFields) throws IllegalArgumentException { - String fieldName = field.getName(); + JSONArray jsonArray; try { - jsonArray = json.getJSONArray(fieldName); + jsonArray = json.getJSONArray(actualJsonKeyName); } catch (JSONException e) { throw new IllegalArgumentException( "JSONObject does not have a array field at " + currentScope + "."); } - switch (field.getType()) { + switch (fieldDescriptor.getType()) { case BOOL: for (int i = 0; i < jsonArray.length(); i++) { try { - protoMsg.addRepeatedField(field, new Boolean(jsonArray.getBoolean(i))); + protoMsg.addRepeatedField(fieldDescriptor, new Boolean(jsonArray.getBoolean(i))); } catch (JSONException e) { throw new IllegalArgumentException( "JSONObject does not have a boolean field at " @@ -251,7 +291,7 @@ private static void fillRepeatedField( case BYTES: for (int i = 0; i < jsonArray.length(); i++) { try { - protoMsg.addRepeatedField(field, jsonArray.getString(i).getBytes()); + protoMsg.addRepeatedField(fieldDescriptor, jsonArray.getString(i).getBytes()); } catch (JSONException e) { throw new IllegalArgumentException( "JSONObject does not have a string field at " + currentScope + "[" + i + "]" + "."); @@ -263,13 +303,13 @@ private static void fillRepeatedField( try { java.lang.Object val = jsonArray.get(i); if (val instanceof Byte) { - protoMsg.addRepeatedField(field, new Long((Byte) val)); + protoMsg.addRepeatedField(fieldDescriptor, new Long((Byte) val)); } else if (val instanceof Short) { - protoMsg.addRepeatedField(field, new Long((Short) val)); + protoMsg.addRepeatedField(fieldDescriptor, new Long((Short) val)); } else if (val instanceof Integer) { - protoMsg.addRepeatedField(field, new Long((Integer) val)); + protoMsg.addRepeatedField(fieldDescriptor, new Long((Integer) val)); } else if (val instanceof Long) { - protoMsg.addRepeatedField(field, new Long((Long) val)); + protoMsg.addRepeatedField(fieldDescriptor, new Long((Long) val)); } else { throw new IllegalArgumentException( "JSONObject does not have a int64 field at " @@ -285,10 +325,35 @@ private static void fillRepeatedField( } } break; + case INT32: + for (int i = 0; i < jsonArray.length(); i++) { + try { + java.lang.Object val = jsonArray.get(i); + if (val instanceof Byte) { + protoMsg.addRepeatedField(fieldDescriptor, new Integer((Byte) val)); + } else if (val instanceof Short) { + protoMsg.addRepeatedField(fieldDescriptor, new Integer((Short) val)); + } else if (val instanceof Integer) { + protoMsg.addRepeatedField(fieldDescriptor, new Integer((Integer) val)); + } else { + throw new IllegalArgumentException( + "JSONObject does not have a int32 field at " + + currentScope + + "[" + + i + + "]" + + "."); + } + } catch (JSONException e) { + throw new IllegalArgumentException( + "JSONObject does not have a int32 field at " + currentScope + "[" + i + "]" + "."); + } + } + break; case STRING: for (int i = 0; i < jsonArray.length(); i++) { try { - protoMsg.addRepeatedField(field, jsonArray.getString(i)); + protoMsg.addRepeatedField(fieldDescriptor, jsonArray.getString(i)); } catch (JSONException e) { throw new IllegalArgumentException( "JSONObject does not have a string field at " + currentScope + "[" + i + "]" + "."); @@ -300,9 +365,9 @@ private static void fillRepeatedField( try { java.lang.Object val = jsonArray.get(i); if (val instanceof Double) { - protoMsg.addRepeatedField(field, new Double((double) val)); + protoMsg.addRepeatedField(fieldDescriptor, new Double((double) val)); } else if (val instanceof Float) { - protoMsg.addRepeatedField(field, new Double((float) val)); + protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val)); } else { throw new IllegalArgumentException( "JSONObject does not have a double field at " @@ -321,11 +386,11 @@ private static void fillRepeatedField( case MESSAGE: for (int i = 0; i < jsonArray.length(); i++) { try { - Message.Builder message = protoMsg.newBuilderForField(field); + Message.Builder message = protoMsg.newBuilderForField(fieldDescriptor); protoMsg.addRepeatedField( - field, + fieldDescriptor, convertJsonToProtoMessageImpl( - field.getMessageType(), + fieldDescriptor.getMessageType(), jsonArray.getJSONObject(i), currentScope, false, diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index 5468ba92eb..14a7c349b0 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -25,6 +25,7 @@ import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.DynamicMessage; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; import org.json.JSONArray; @@ -40,6 +41,7 @@ public class JsonToProtoMessageTest { .put(BoolType.getDescriptor(), "boolean") .put(BytesType.getDescriptor(), "string") .put(Int64Type.getDescriptor(), "int64") + .put(Int32Type.getDescriptor(), "int32") .put(DoubleType.getDescriptor(), "double") .put(StringType.getDescriptor(), "string") .put(RepeatedType.getDescriptor(), "array") @@ -51,12 +53,14 @@ public class JsonToProtoMessageTest { .put(RepeatedBool.getDescriptor(), "boolean") .put(RepeatedBytes.getDescriptor(), "string") .put(RepeatedInt64.getDescriptor(), "int64") + .put(RepeatedInt32.getDescriptor(), "int32") .put(RepeatedDouble.getDescriptor(), "double") .put(RepeatedObject.getDescriptor(), "object") .build(); private static JSONObject[] simpleJSONObjects = { - new JSONObject().put("test_field_type", 123), + new JSONObject().put("test_field_type", Long.MAX_VALUE), + new JSONObject().put("test_field_type", Integer.MAX_VALUE), new JSONObject().put("test_field_type", 1.23), new JSONObject().put("test_field_type", true), new JSONObject().put("test_field_type", "test"), @@ -80,6 +84,19 @@ public class JsonToProtoMessageTest { (long) Byte.MIN_VALUE, 0L })), + new JSONObject() + .put( + "test_repeated", + new JSONArray( + new Integer[] { + (int) Integer.MAX_VALUE, + (int) Integer.MIN_VALUE, + (int) Short.MAX_VALUE, + (int) Short.MIN_VALUE, + (int) Byte.MAX_VALUE, + (int) Byte.MIN_VALUE, + 0 + })), new JSONObject() .put( "test_repeated", @@ -104,20 +121,28 @@ public class JsonToProtoMessageTest { }; private void AreMatchingFieldsFilledIn(DynamicMessage proto, JSONObject json) { + HashMap jsonLowercaseNameToActualName = new HashMap(); + String[] actualNames = JSONObject.getNames(json); + for (int i = 0; i < actualNames.length; i++) { + jsonLowercaseNameToActualName.put(actualNames[i].toLowerCase(), actualNames[i]); + } for (Map.Entry entry : proto.getAllFields().entrySet()) { FieldDescriptor key = entry.getKey(); java.lang.Object value = entry.getValue(); if (key.isRepeated()) { - isProtoArrayJsonArrayEqual(key, value, json); + isProtoArrayJsonArrayEqual(key, value, json, jsonLowercaseNameToActualName); } else { - isProtoFieldJsonFieldEqual(key, value, json); + isProtoFieldJsonFieldEqual(key, value, json, jsonLowercaseNameToActualName); } } } private void isProtoFieldJsonFieldEqual( - FieldDescriptor key, java.lang.Object value, JSONObject json) { - String fieldName = key.getName(); + FieldDescriptor key, + java.lang.Object value, + JSONObject json, + HashMap jsonLowercaseNameToActualName) { + String fieldName = jsonLowercaseNameToActualName.get(key.getName().toLowerCase()); switch (key.getType()) { case BOOL: assertTrue((Boolean) value == json.getBoolean(fieldName)); @@ -128,6 +153,9 @@ private void isProtoFieldJsonFieldEqual( case INT64: assertTrue((long) value == json.getLong(fieldName)); break; + case INT32: + assertTrue((int) value == json.getInt(fieldName)); + break; case STRING: assertTrue(((String) value).equals(json.getString(fieldName))); break; @@ -141,8 +169,11 @@ private void isProtoFieldJsonFieldEqual( } private void isProtoArrayJsonArrayEqual( - FieldDescriptor key, java.lang.Object value, JSONObject json) { - String fieldName = key.getName(); + FieldDescriptor key, + java.lang.Object value, + JSONObject json, + HashMap jsonLowercaseNameToActualName) { + String fieldName = jsonLowercaseNameToActualName.get(key.getName().toLowerCase()); JSONArray jsonArray = json.getJSONArray(fieldName); switch (key.getType()) { case BOOL: @@ -163,6 +194,12 @@ private void isProtoArrayJsonArrayEqual( assertTrue((longArr.get(i) == jsonArray.getLong(i))); } break; + case INT32: + List intArr = (List) value; + for (int i = 0; i < jsonArray.length(); i++) { + assertTrue((intArr.get(i) == jsonArray.getInt(i))); + } + break; case STRING: List stringArr = (List) value; for (int i = 0; i < jsonArray.length(); i++) { @@ -184,6 +221,18 @@ private void isProtoArrayJsonArrayEqual( } } + @Test + public void testDifferentNameCasing() throws Exception { + JSONObject json = new JSONObject(); + json.put("bYtE", (byte) 1); + json.put("SHORT", (short) 1); + json.put("inT", 1); + json.put("lONg", 1L); + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + AreMatchingFieldsFilledIn(protoMsg, json); + } + @Test public void testInt64() throws Exception { JSONObject json = new JSONObject(); @@ -196,6 +245,31 @@ public void testInt64() throws Exception { AreMatchingFieldsFilledIn(protoMsg, json); } + @Test + public void testInt32() throws Exception { + JSONObject json = new JSONObject(); + json.put("byte", (byte) 1); + json.put("short", (short) 1); + json.put("int", 1); + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json); + AreMatchingFieldsFilledIn(protoMsg, json); + } + + @Test + public void testInt32NotMatchInt64() throws Exception { + JSONObject json = new JSONObject(); + json.put("byte", (byte) 1); + json.put("short", (short) 1); + json.put("int", 1L); + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "JSONObject does not have a int32 field at root.int."); + } + } + @Test public void testDouble() throws Exception { JSONObject json = new JSONObject(); @@ -222,7 +296,11 @@ public void testAllTypes() throws Exception { "JSONObject does not have a " + entry.getValue() + " field at root.test_field_type."); } } - assertEquals(1, success); + if (entry.getKey() == Int64Type.getDescriptor()) { + assertEquals(2, success); + } else { + assertEquals(1, success); + } } } @@ -243,7 +321,11 @@ public void testAllRepeatedTypesWithLimits() throws Exception { + " field at root.test_repeated[0]."); } } - assertEquals(1, success); + if (entry.getKey() == RepeatedInt64.getDescriptor()) { + assertEquals(2, success); + } else { + assertEquals(1, success); + } } } @@ -323,9 +405,9 @@ public void testStructComplex() throws Exception { json.put("test_string", new JSONArray(new String[] {"a", "b", "c"})); json.put("test_bytes", "hello"); json.put("test_bool", true); - json.put("test_double", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4})); + json.put("test_DOUBLe", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4})); json.put("complexLvl1", complexLvl1); - json.put("complexLvl2", complexLvl2); + json.put("complexLVL2", complexLvl2); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json); @@ -436,7 +518,7 @@ public void testAllowUnknownFieldsError() throws Exception { } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), - "JSONObject has unknown fields. Set allowUnknownFields to True to ignore unknown fields."); + "JSONObject has fields unknown to BigQuery: f1. Set allowUnknownFields to True to allow unknown fields."); } } diff --git a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto index 43517e369f..6e06edc534 100644 --- a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto +++ b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto @@ -65,6 +65,10 @@ message RepeatedInt64 { repeated int64 test_repeated = 1; } +message RepeatedInt32 { + repeated int32 test_repeated = 1; +} + message RepeatedDouble { repeated double test_repeated = 1; } @@ -92,6 +96,12 @@ message TestInt64 { optional int64 long = 4; } +message TestInt32 { + optional int32 byte = 1; + optional int32 short = 2; + optional int32 int = 3; +} + message TestDouble { optional double double = 1; optional double float = 2; From d3d3505953a27c13c7e1305d6b516a1ebe6112ac Mon Sep 17 00:00:00 2001 From: Allen Chen Date: Mon, 13 Jul 2020 21:24:19 +0000 Subject: [PATCH 06/21] Update JsonToProtoMessage to fix allowUnknownField behavior --- .../storage/v1alpha2/JsonToProtoMessage.java | 197 +++++++----------- .../v1alpha2/JsonToProtoMessageTest.java | 70 ++++--- 2 files changed, 115 insertions(+), 152 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index 46cdacad77..f8ab491686 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -20,6 +20,7 @@ import com.google.protobuf.DynamicMessage; import com.google.protobuf.Message; import java.util.HashMap; +import java.util.Map; import java.util.List; import org.json.JSONArray; import org.json.JSONException; @@ -28,18 +29,6 @@ /** Converts Json data to protocol buffer messages given the protocol buffer descriptor. */ public class JsonToProtoMessage { - /** - * Converts Json data to protocol buffer messages given the protocol buffer descriptor. - * - * @param protoSchema - * @param json - * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. - */ - public static DynamicMessage convertJsonToProtoMessage(Descriptor protoSchema, JSONObject json) - throws IllegalArgumentException { - return convertJsonToProtoMessage(protoSchema, json, false); - } - /** * Converts Json data to protocol buffer messages given the protocol buffer descriptor. * @@ -51,6 +40,9 @@ public static DynamicMessage convertJsonToProtoMessage(Descriptor protoSchema, J public static DynamicMessage convertJsonToProtoMessage( Descriptor protoSchema, JSONObject json, boolean allowUnknownFields) throws IllegalArgumentException { + if (json.length() == 0) { + throw new IllegalArgumentException("JSONObject is empty."); + } return convertJsonToProtoMessageImpl(protoSchema, json, "root", true, allowUnknownFields); } @@ -60,8 +52,8 @@ public static DynamicMessage convertJsonToProtoMessage( * @param protoSchema * @param json * @param jsonScope Debugging purposes - * @param topLevel If root level has no matching fields, throws exception. * @param allowUnknownFields Ignores unknown JSON fields. + * @param topLevel checks if root level has any matching fields. * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. */ private static DynamicMessage convertJsonToProtoMessageImpl( @@ -71,26 +63,33 @@ private static DynamicMessage convertJsonToProtoMessageImpl( boolean topLevel, boolean allowUnknownFields) throws IllegalArgumentException { + DynamicMessage.Builder protoMsg = DynamicMessage.newBuilder(protoSchema); - int matchedFields = 0; List protoFields = protoSchema.getFields(); - - if (JSONObject.getNames(json).length > protoFields.size()) { - if (!allowUnknownFields) { - throw new IllegalArgumentException( - "JSONObject has fields unknown to BigQuery: f1. Set allowUnknownFields to True to allow unknown fields."); - } + HashMap protoLowercaseNameToString = new HashMap(); + for (FieldDescriptor field : protoFields) { + protoLowercaseNameToString.put(field.getName().toLowerCase(), field); } + HashMap jsonLowercaseNameToActualName = new HashMap(); String[] actualNames = JSONObject.getNames(json); for (int i = 0; i < actualNames.length; i++) { jsonLowercaseNameToActualName.put(actualNames[i].toLowerCase(), actualNames[i]); } + if (!allowUnknownFields) { + for (String jsonLowercaseField : jsonLowercaseNameToActualName.keySet()) { + if (!protoLowercaseNameToString.containsKey(jsonLowercaseField)) { + throw new IllegalArgumentException( + "JSONObject has fields unknown to BigQuery: " + jsonScope + "." + jsonLowercaseNameToActualName.get(jsonLowercaseField) + ". Set allowUnknownFields to True to allow unknown fields."); + } + } + } - for (FieldDescriptor field : protoFields) { - String fieldName = field.getName(); - String lowercaseFieldName = fieldName.toLowerCase(); - String currentScope = jsonScope + "." + fieldName; + int matchedFields = 0; + for (Map.Entry protoEntry : protoLowercaseNameToString.entrySet()) { + String lowercaseFieldName = protoEntry.getKey(); + FieldDescriptor field = protoEntry.getValue(); + String currentScope = jsonScope + "." + field.getName(); if (!jsonLowercaseNameToActualName.containsKey(lowercaseFieldName)) { if (field.isRequired()) { @@ -145,7 +144,7 @@ private static void fillField( String currentScope, boolean allowUnknownFields) throws IllegalArgumentException { - + java.lang.Object val; switch (fieldDescriptor.getType()) { case BOOL: try { @@ -164,39 +163,21 @@ private static void fillField( } break; case INT64: - try { - java.lang.Object val = json.get(actualJsonKeyName); - if (val instanceof Byte) { - protoMsg.setField(fieldDescriptor, new Long((Byte) val)); - } else if (val instanceof Short) { - protoMsg.setField(fieldDescriptor, new Long((Short) val)); - } else if (val instanceof Integer) { - protoMsg.setField(fieldDescriptor, new Long((Integer) val)); - } else if (val instanceof Long) { - protoMsg.setField(fieldDescriptor, new Long((Long) val)); - } else { - throw new IllegalArgumentException( - "JSONObject does not have a int64 field at " + currentScope + "."); - } - } catch (JSONException e) { + val = json.get(actualJsonKeyName); + if (val instanceof Integer) { + protoMsg.setField(fieldDescriptor, new Long((Integer) val)); + } else if (val instanceof Long) { + protoMsg.setField(fieldDescriptor, new Long((Long) val)); + } else { throw new IllegalArgumentException( "JSONObject does not have a int64 field at " + currentScope + "."); } break; case INT32: - try { - java.lang.Object val = json.get(actualJsonKeyName); - if (val instanceof Byte) { - protoMsg.setField(fieldDescriptor, new Integer((Byte) val)); - } else if (val instanceof Short) { - protoMsg.setField(fieldDescriptor, new Integer((Short) val)); - } else if (val instanceof Integer) { - protoMsg.setField(fieldDescriptor, new Integer((Integer) val)); - } else { - throw new IllegalArgumentException( - "JSONObject does not have a int32 field at " + currentScope + "."); - } - } catch (JSONException e) { + val = json.get(actualJsonKeyName); + if (val instanceof Integer) { + protoMsg.setField(fieldDescriptor, new Integer((Integer) val)); + } else { throw new IllegalArgumentException( "JSONObject does not have a int32 field at " + currentScope + "."); } @@ -210,17 +191,12 @@ private static void fillField( } break; case DOUBLE: - try { - java.lang.Object val = json.get(actualJsonKeyName); - if (val instanceof Double) { - protoMsg.setField(fieldDescriptor, new Double((double) val)); - } else if (val instanceof Float) { - protoMsg.setField(fieldDescriptor, new Double((float) val)); - } else { - throw new IllegalArgumentException( - "JSONObject does not have a double field at " + currentScope + "."); - } - } catch (JSONException e) { + val = json.get(actualJsonKeyName); + if (val instanceof Double) { + protoMsg.setField(fieldDescriptor, new Double((double) val)); + } else if (val instanceof Float) { + protoMsg.setField(fieldDescriptor, new Double((float) val)); + } else { throw new IllegalArgumentException( "JSONObject does not have a double field at " + currentScope + "."); } @@ -271,7 +247,7 @@ private static void fillRepeatedField( throw new IllegalArgumentException( "JSONObject does not have a array field at " + currentScope + "."); } - + java.lang.Object val; switch (fieldDescriptor.getType()) { case BOOL: for (int i = 0; i < jsonArray.length(); i++) { @@ -300,53 +276,35 @@ private static void fillRepeatedField( break; case INT64: for (int i = 0; i < jsonArray.length(); i++) { - try { - java.lang.Object val = jsonArray.get(i); - if (val instanceof Byte) { - protoMsg.addRepeatedField(fieldDescriptor, new Long((Byte) val)); - } else if (val instanceof Short) { - protoMsg.addRepeatedField(fieldDescriptor, new Long((Short) val)); - } else if (val instanceof Integer) { - protoMsg.addRepeatedField(fieldDescriptor, new Long((Integer) val)); - } else if (val instanceof Long) { - protoMsg.addRepeatedField(fieldDescriptor, new Long((Long) val)); - } else { - throw new IllegalArgumentException( - "JSONObject does not have a int64 field at " - + currentScope - + "[" - + i - + "]" - + "."); - } - } catch (JSONException e) { + val = jsonArray.get(i); + if (val instanceof Integer) { + protoMsg.addRepeatedField(fieldDescriptor, new Long((Integer) val)); + } else if (val instanceof Long) { + protoMsg.addRepeatedField(fieldDescriptor, new Long((Long) val)); + } else { throw new IllegalArgumentException( - "JSONObject does not have a int64 field at " + currentScope + "[" + i + "]" + "."); + "JSONObject does not have a int64 field at " + + currentScope + + "[" + + i + + "]" + + "."); } } break; case INT32: for (int i = 0; i < jsonArray.length(); i++) { - try { - java.lang.Object val = jsonArray.get(i); - if (val instanceof Byte) { - protoMsg.addRepeatedField(fieldDescriptor, new Integer((Byte) val)); - } else if (val instanceof Short) { - protoMsg.addRepeatedField(fieldDescriptor, new Integer((Short) val)); - } else if (val instanceof Integer) { - protoMsg.addRepeatedField(fieldDescriptor, new Integer((Integer) val)); - } else { - throw new IllegalArgumentException( - "JSONObject does not have a int32 field at " - + currentScope - + "[" - + i - + "]" - + "."); - } - } catch (JSONException e) { + val = jsonArray.get(i); + if (val instanceof Integer) { + protoMsg.addRepeatedField(fieldDescriptor, new Integer((Integer) val)); + } else { throw new IllegalArgumentException( - "JSONObject does not have a int32 field at " + currentScope + "[" + i + "]" + "."); + "JSONObject does not have a int32 field at " + + currentScope + + "[" + + i + + "]" + + "."); } } break; @@ -362,24 +320,19 @@ private static void fillRepeatedField( break; case DOUBLE: for (int i = 0; i < jsonArray.length(); i++) { - try { - java.lang.Object val = jsonArray.get(i); - if (val instanceof Double) { - protoMsg.addRepeatedField(fieldDescriptor, new Double((double) val)); - } else if (val instanceof Float) { - protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val)); - } else { - throw new IllegalArgumentException( - "JSONObject does not have a double field at " - + currentScope - + "[" - + i - + "]" - + "."); - } - } catch (JSONException e) { + val = jsonArray.get(i); + if (val instanceof Double) { + protoMsg.addRepeatedField(fieldDescriptor, new Double((double) val)); + } else if (val instanceof Float) { + protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val)); + } else { throw new IllegalArgumentException( - "JSONObject does not have a double field at " + currentScope + "[" + i + "]" + "."); + "JSONObject does not have a double field at " + + currentScope + + "[" + + i + + "]" + + "."); } } break; diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index 14a7c349b0..4676764aed 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -107,6 +107,14 @@ public class JsonToProtoMessageTest { (double) Float.MAX_VALUE, (double) Float.MIN_VALUE })), + new JSONObject() + .put( + "test_repeated", + new JSONArray( + new Float[] { + Float.MAX_VALUE, + Float.MIN_VALUE + })), new JSONObject().put("test_repeated", new JSONArray(new Boolean[] {true, false})), new JSONObject().put("test_repeated", new JSONArray(new String[] {"hello", "test"})), new JSONObject() @@ -229,7 +237,7 @@ public void testDifferentNameCasing() throws Exception { json.put("inT", 1); json.put("lONg", 1L); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); } @@ -241,7 +249,7 @@ public void testInt64() throws Exception { json.put("int", 1); json.put("long", 1L); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); } @@ -252,7 +260,7 @@ public void testInt32() throws Exception { json.put("short", (short) 1); json.put("int", 1); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); } @@ -264,7 +272,7 @@ public void testInt32NotMatchInt64() throws Exception { json.put("int", 1L); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json, false); } catch (IllegalArgumentException e) { assertEquals(e.getMessage(), "JSONObject does not have a int32 field at root.int."); } @@ -276,7 +284,7 @@ public void testDouble() throws Exception { json.put("double", 1.2); json.put("float", 3.4f); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestDouble.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(TestDouble.getDescriptor(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); } @@ -287,7 +295,7 @@ public void testAllTypes() throws Exception { for (JSONObject json : simpleJSONObjects) { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json); + JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); success += 1; } catch (IllegalArgumentException e) { @@ -311,7 +319,7 @@ public void testAllRepeatedTypesWithLimits() throws Exception { for (JSONObject json : simpleJSONArrays) { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json); + JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json, false); success += 1; } catch (IllegalArgumentException e) { assertEquals( @@ -321,7 +329,7 @@ public void testAllRepeatedTypesWithLimits() throws Exception { + " field at root.test_repeated[0]."); } } - if (entry.getKey() == RepeatedInt64.getDescriptor()) { + if (entry.getKey() == RepeatedInt64.getDescriptor() || entry.getKey() == RepeatedDouble.getDescriptor()) { assertEquals(2, success); } else { assertEquals(1, success); @@ -335,7 +343,7 @@ public void testOptional() throws Exception { json.put("byte", 1); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); } @@ -345,18 +353,17 @@ public void testRepeatedIsOptional() throws Exception { json.put("required_double", 1.1); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestRepeatedIsOptional.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(TestRepeatedIsOptional.getDescriptor(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); } @Test public void testRequired() throws Exception { JSONObject json = new JSONObject(); - json.put("test_required_double", 1.1); json.put("optional_double", 1.1); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestRequired.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(TestRequired.getDescriptor(), json, false); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), "JSONObject does not have the required field root.required_double."); @@ -371,7 +378,7 @@ public void testStructSimple() throws Exception { json.put("test_field_type", stringType); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); } @@ -383,7 +390,7 @@ public void testStructSimpleFail() throws Exception { json.put("test_field_type", stringType); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json, false); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), @@ -410,7 +417,7 @@ public void testStructComplex() throws Exception { json.put("complexLVL2", complexLvl2); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); } @@ -434,7 +441,7 @@ public void testStructComplexFail() throws Exception { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json, false); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), "JSONObject does not have a int64 field at root.complexLvl1.test_int."); @@ -447,7 +454,7 @@ public void testRepeatedWithMixedTypes() throws Exception { json.put("test_repeated", new JSONArray("[1.1, 2.2, true]")); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(RepeatedDouble.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(RepeatedDouble.getDescriptor(), json, false); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), "JSONObject does not have a double field at root.test_repeated[2]."); @@ -468,7 +475,7 @@ public void testNestedRepeatedComplex() throws Exception { json.put("repeated_string", jsonRepeatedString); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); } @@ -487,7 +494,7 @@ public void testNestedRepeatedComplexFail() throws Exception { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json, false); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), @@ -510,29 +517,27 @@ public void testAllowUnknownFields() throws Exception { public void testAllowUnknownFieldsError() throws Exception { JSONObject json = new JSONObject(); json.put("double", 1.1); - json.put("string", "hello"); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json, false); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), - "JSONObject has fields unknown to BigQuery: f1. Set allowUnknownFields to True to allow unknown fields."); + "JSONObject has fields unknown to BigQuery: root.double. Set allowUnknownFields to True to allow unknown fields."); } } @Test - public void testTopLevelMatchFail() throws Exception { + public void testEmptyJSONObject() throws Exception { JSONObject json = new JSONObject(); - json.put("double", 1.1); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), json, false); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), - "There are no matching fields found for the JSONObject and the protocol buffer descriptor."); + "JSONObject is empty."); } } @@ -544,9 +549,14 @@ public void testTopLevelMatchSecondLevelNoMatch() throws Exception { json.put("test_int", 1); json.put("complexLvl2", complexLvl2); - DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json); - AreMatchingFieldsFilledIn(protoMsg, json); + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, false); + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), + "JSONObject has fields unknown to BigQuery: root.complexLvl2.no_match. Set allowUnknownFields to True to allow unknown fields."); + } } @Test @@ -556,7 +566,7 @@ public void testJsonNullValue() throws Exception { json.put("int", 1); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); } catch (IllegalArgumentException e) { assertEquals(e.getMessage(), "JSONObject does not have a int64 field at root.long."); } From 5c7afe9167e67b0396f163b902104eb978ecb62c Mon Sep 17 00:00:00 2001 From: allenc3 Date: Mon, 13 Jul 2020 17:25:48 -0500 Subject: [PATCH 07/21] Fix according to PR, used case insensitive treeset for allowUnknownFields check. --- .../storage/v1alpha2/JsonToProtoMessage.java | 65 ++++++++----------- .../v1alpha2/JsonToProtoMessageTest.java | 54 ++++++++++----- .../src/test/proto/jsonTest.proto | 4 ++ 3 files changed, 71 insertions(+), 52 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index f8ab491686..ccda28cf57 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -20,8 +20,9 @@ import com.google.protobuf.DynamicMessage; import com.google.protobuf.Message; import java.util.HashMap; -import java.util.Map; import java.util.List; +import java.util.Set; +import java.util.TreeSet; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -40,9 +41,9 @@ public class JsonToProtoMessage { public static DynamicMessage convertJsonToProtoMessage( Descriptor protoSchema, JSONObject json, boolean allowUnknownFields) throws IllegalArgumentException { - if (json.length() == 0) { - throw new IllegalArgumentException("JSONObject is empty."); - } + if (json.length() == 0) { + throw new IllegalArgumentException("JSONObject is empty."); + } return convertJsonToProtoMessageImpl(protoSchema, json, "root", true, allowUnknownFields); } @@ -66,32 +67,37 @@ private static DynamicMessage convertJsonToProtoMessageImpl( DynamicMessage.Builder protoMsg = DynamicMessage.newBuilder(protoSchema); List protoFields = protoSchema.getFields(); - HashMap protoLowercaseNameToString = new HashMap(); + + Set protoFieldNames = new TreeSet(String.CASE_INSENSITIVE_ORDER); for (FieldDescriptor field : protoFields) { - protoLowercaseNameToString.put(field.getName().toLowerCase(), field); + protoFieldNames.add(field.getName()); } - HashMap jsonLowercaseNameToActualName = new HashMap(); - String[] actualNames = JSONObject.getNames(json); - for (int i = 0; i < actualNames.length; i++) { - jsonLowercaseNameToActualName.put(actualNames[i].toLowerCase(), actualNames[i]); + HashMap jsonLowercaseNameToName = new HashMap(); + String[] jsonNames = JSONObject.getNames(json); + for (int i = 0; i < jsonNames.length; i++) { + jsonLowercaseNameToName.put(jsonNames[i].toLowerCase(), jsonNames[i]); } + if (!allowUnknownFields) { - for (String jsonLowercaseField : jsonLowercaseNameToActualName.keySet()) { - if (!protoLowercaseNameToString.containsKey(jsonLowercaseField)) { + for (int i = 0; i < jsonNames.length; i++) { + if (!protoFieldNames.contains(jsonNames[i])) { throw new IllegalArgumentException( - "JSONObject has fields unknown to BigQuery: " + jsonScope + "." + jsonLowercaseNameToActualName.get(jsonLowercaseField) + ". Set allowUnknownFields to True to allow unknown fields."); + "JSONObject has fields unknown to BigQuery: " + + jsonScope + + "." + + jsonNames[i] + + ". Set allowUnknownFields to True to allow unknown fields."); } } } int matchedFields = 0; - for (Map.Entry protoEntry : protoLowercaseNameToString.entrySet()) { - String lowercaseFieldName = protoEntry.getKey(); - FieldDescriptor field = protoEntry.getValue(); + for (FieldDescriptor field : protoFields) { + String lowercaseFieldName = field.getName().toLowerCase(); String currentScope = jsonScope + "." + field.getName(); - if (!jsonLowercaseNameToActualName.containsKey(lowercaseFieldName)) { + if (!jsonLowercaseNameToName.containsKey(lowercaseFieldName)) { if (field.isRequired()) { throw new IllegalArgumentException( "JSONObject does not have the required field " + currentScope + "."); @@ -105,7 +111,7 @@ private static DynamicMessage convertJsonToProtoMessageImpl( protoMsg, field, json, - jsonLowercaseNameToActualName.get(lowercaseFieldName), + jsonLowercaseNameToName.get(lowercaseFieldName), currentScope, allowUnknownFields); } else { @@ -113,7 +119,7 @@ private static DynamicMessage convertJsonToProtoMessageImpl( protoMsg, field, json, - jsonLowercaseNameToActualName.get(lowercaseFieldName), + jsonLowercaseNameToName.get(lowercaseFieldName), currentScope, allowUnknownFields); } @@ -283,12 +289,7 @@ private static void fillRepeatedField( protoMsg.addRepeatedField(fieldDescriptor, new Long((Long) val)); } else { throw new IllegalArgumentException( - "JSONObject does not have a int64 field at " - + currentScope - + "[" - + i - + "]" - + "."); + "JSONObject does not have a int64 field at " + currentScope + "[" + i + "]" + "."); } } break; @@ -299,12 +300,7 @@ private static void fillRepeatedField( protoMsg.addRepeatedField(fieldDescriptor, new Integer((Integer) val)); } else { throw new IllegalArgumentException( - "JSONObject does not have a int32 field at " - + currentScope - + "[" - + i - + "]" - + "."); + "JSONObject does not have a int32 field at " + currentScope + "[" + i + "]" + "."); } } break; @@ -327,12 +323,7 @@ private static void fillRepeatedField( protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val)); } else { throw new IllegalArgumentException( - "JSONObject does not have a double field at " - + currentScope - + "[" - + i - + "]" - + "."); + "JSONObject does not have a double field at " + currentScope + "[" + i + "]" + "."); } } break; diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index 4676764aed..4a711b0293 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -108,13 +108,7 @@ public class JsonToProtoMessageTest { (double) Float.MIN_VALUE })), new JSONObject() - .put( - "test_repeated", - new JSONArray( - new Float[] { - Float.MAX_VALUE, - Float.MIN_VALUE - })), + .put("test_repeated", new JSONArray(new Float[] {Float.MAX_VALUE, Float.MIN_VALUE})), new JSONObject().put("test_repeated", new JSONArray(new Boolean[] {true, false})), new JSONObject().put("test_repeated", new JSONArray(new String[] {"hello", "test"})), new JSONObject() @@ -329,7 +323,8 @@ public void testAllRepeatedTypesWithLimits() throws Exception { + " field at root.test_repeated[0]."); } } - if (entry.getKey() == RepeatedInt64.getDescriptor() || entry.getKey() == RepeatedDouble.getDescriptor()) { + if (entry.getKey() == RepeatedInt64.getDescriptor() + || entry.getKey() == RepeatedDouble.getDescriptor()) { assertEquals(2, success); } else { assertEquals(1, success); @@ -353,7 +348,8 @@ public void testRepeatedIsOptional() throws Exception { json.put("required_double", 1.1); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestRepeatedIsOptional.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage( + TestRepeatedIsOptional.getDescriptor(), json, false); AreMatchingFieldsFilledIn(protoMsg, json); } @@ -516,7 +512,8 @@ public void testAllowUnknownFields() throws Exception { @Test public void testAllowUnknownFieldsError() throws Exception { JSONObject json = new JSONObject(); - json.put("double", 1.1); + json.put("test_repeated", new JSONArray(new int[] {1, 2, 3, 4, 5})); + json.put("string", "hello"); try { DynamicMessage protoMsg = @@ -524,7 +521,7 @@ public void testAllowUnknownFieldsError() throws Exception { } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), - "JSONObject has fields unknown to BigQuery: root.double. Set allowUnknownFields to True to allow unknown fields."); + "JSONObject has fields unknown to BigQuery: root.string. Set allowUnknownFields to True to allow unknown fields."); } } @@ -535,14 +532,12 @@ public void testEmptyJSONObject() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), json, false); } catch (IllegalArgumentException e) { - assertEquals( - e.getMessage(), - "JSONObject is empty."); + assertEquals(e.getMessage(), "JSONObject is empty."); } } @Test - public void testTopLevelMatchSecondLevelNoMatch() throws Exception { + public void testAllowUnknownFieldsSecondLevel() throws Exception { JSONObject complexLvl2 = new JSONObject(); complexLvl2.put("no_match", 1); JSONObject json = new JSONObject(); @@ -559,6 +554,35 @@ public void testTopLevelMatchSecondLevelNoMatch() throws Exception { } } + @Test + public void testTopLevelMismatch() throws Exception { + JSONObject json = new JSONObject(); + json.put("no_match", 1.1); + + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage( + TopLevelMismatch.getDescriptor(), json, true); + } catch (IllegalArgumentException e) { + assertEquals( + e.getMessage(), + "There are no matching fields found for the JSONObject and the protocol buffer descriptor."); + } + } + + @Test + public void testTopLevelMatchSecondLevelMismatch() throws Exception { + JSONObject complexLvl2 = new JSONObject(); + complexLvl2.put("no_match", 1); + JSONObject json = new JSONObject(); + json.put("test_int", 1); + json.put("complexLvl2", complexLvl2); + + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, true); + AreMatchingFieldsFilledIn(protoMsg, json); + } + @Test public void testJsonNullValue() throws Exception { JSONObject json = new JSONObject(); diff --git a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto index 6e06edc534..8e3d1d9675 100644 --- a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto +++ b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto @@ -122,3 +122,7 @@ message TestRepeatedIsOptional { optional double required_double = 1; repeated double repeated_double = 2; } + +message TopLevelMismatch { + optional double mismatch_double = 1; +} From e552748e0c7e9a84f46b7afec5c90cc227bf8f19 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 14 Jul 2020 12:26:51 -0500 Subject: [PATCH 08/21] Fixed according to PR --- .../storage/v1alpha2/JsonToProtoMessage.java | 169 +++++++++--------- 1 file changed, 83 insertions(+), 86 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index ccda28cf57..d85f478a6d 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -15,6 +15,7 @@ */ package com.google.cloud.bigquery.storage.v1alpha2; +import com.google.common.collect.ImmutableMap; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.DynamicMessage; @@ -29,6 +30,16 @@ /** Converts Json data to protocol buffer messages given the protocol buffer descriptor. */ public class JsonToProtoMessage { + private static ImmutableMap FieldTypeToDebugMessage = + new ImmutableMap.Builder() + .put(FieldDescriptor.Type.BOOL, "boolean") + .put(FieldDescriptor.Type.BYTES, "string") + .put(FieldDescriptor.Type.INT32, "int32") + .put(FieldDescriptor.Type.DOUBLE, "double") + .put(FieldDescriptor.Type.INT64, "int64") + .put(FieldDescriptor.Type.STRING, "string") + .put(FieldDescriptor.Type.MESSAGE, "object") + .build(); /** * Converts Json data to protocol buffer messages given the protocol buffer descriptor. @@ -44,7 +55,8 @@ public static DynamicMessage convertJsonToProtoMessage( if (json.length() == 0) { throw new IllegalArgumentException("JSONObject is empty."); } - return convertJsonToProtoMessageImpl(protoSchema, json, "root", true, allowUnknownFields); + return convertJsonToProtoMessageImpl( + protoSchema, json, "root", /*topLevel=*/ true, allowUnknownFields); } /** @@ -73,21 +85,19 @@ private static DynamicMessage convertJsonToProtoMessageImpl( protoFieldNames.add(field.getName()); } - HashMap jsonLowercaseNameToName = new HashMap(); + HashMap jsonLowercaseNameToJsonName = new HashMap(); String[] jsonNames = JSONObject.getNames(json); for (int i = 0; i < jsonNames.length; i++) { - jsonLowercaseNameToName.put(jsonNames[i].toLowerCase(), jsonNames[i]); + jsonLowercaseNameToJsonName.put(jsonNames[i].toLowerCase(), jsonNames[i]); } if (!allowUnknownFields) { for (int i = 0; i < jsonNames.length; i++) { if (!protoFieldNames.contains(jsonNames[i])) { throw new IllegalArgumentException( - "JSONObject has fields unknown to BigQuery: " - + jsonScope - + "." - + jsonNames[i] - + ". Set allowUnknownFields to True to allow unknown fields."); + String.format( + "JSONObject has fields unknown to BigQuery: %s.%s. Set allowUnknownFields to True to allow unknown fields.", + jsonScope, jsonNames[i])); } } } @@ -97,7 +107,7 @@ private static DynamicMessage convertJsonToProtoMessageImpl( String lowercaseFieldName = field.getName().toLowerCase(); String currentScope = jsonScope + "." + field.getName(); - if (!jsonLowercaseNameToName.containsKey(lowercaseFieldName)) { + if (!jsonLowercaseNameToJsonName.containsKey(lowercaseFieldName)) { if (field.isRequired()) { throw new IllegalArgumentException( "JSONObject does not have the required field " + currentScope + "."); @@ -111,7 +121,7 @@ private static DynamicMessage convertJsonToProtoMessageImpl( protoMsg, field, json, - jsonLowercaseNameToName.get(lowercaseFieldName), + jsonLowercaseNameToJsonName.get(lowercaseFieldName), currentScope, allowUnknownFields); } else { @@ -119,7 +129,7 @@ private static DynamicMessage convertJsonToProtoMessageImpl( protoMsg, field, json, - jsonLowercaseNameToName.get(lowercaseFieldName), + jsonLowercaseNameToJsonName.get(lowercaseFieldName), currentScope, allowUnknownFields); } @@ -151,78 +161,63 @@ private static void fillField( boolean allowUnknownFields) throws IllegalArgumentException { java.lang.Object val; - switch (fieldDescriptor.getType()) { - case BOOL: - try { + String error; + try { + switch (fieldDescriptor.getType()) { + case BOOL: protoMsg.setField(fieldDescriptor, new Boolean(json.getBoolean(actualJsonKeyName))); - } catch (JSONException e) { - throw new IllegalArgumentException( - "JSONObject does not have a boolean field at " + currentScope + "."); - } - break; - case BYTES: - try { + break; + case BYTES: protoMsg.setField(fieldDescriptor, json.getString(actualJsonKeyName).getBytes()); - } catch (JSONException e) { - throw new IllegalArgumentException( - "JSONObject does not have a string field at " + currentScope + "."); - } - break; - case INT64: - val = json.get(actualJsonKeyName); - if (val instanceof Integer) { - protoMsg.setField(fieldDescriptor, new Long((Integer) val)); - } else if (val instanceof Long) { - protoMsg.setField(fieldDescriptor, new Long((Long) val)); - } else { - throw new IllegalArgumentException( - "JSONObject does not have a int64 field at " + currentScope + "."); - } - break; - case INT32: - val = json.get(actualJsonKeyName); - if (val instanceof Integer) { - protoMsg.setField(fieldDescriptor, new Integer((Integer) val)); - } else { - throw new IllegalArgumentException( - "JSONObject does not have a int32 field at " + currentScope + "."); - } - break; - case STRING: - try { + break; + case INT64: + val = json.get(actualJsonKeyName); + if (val instanceof Integer) { + protoMsg.setField(fieldDescriptor, new Long((Integer) val)); + } else if (val instanceof Long) { + protoMsg.setField(fieldDescriptor, new Long((Long) val)); + } else { + throw new JSONException(""); + } + break; + case INT32: + val = json.get(actualJsonKeyName); + if (val instanceof Integer) { + protoMsg.setField(fieldDescriptor, new Integer((Integer) val)); + } else { + throw new JSONException(""); + } + break; + case STRING: protoMsg.setField(fieldDescriptor, json.getString(actualJsonKeyName)); - } catch (JSONException e) { - throw new IllegalArgumentException( - "JSONObject does not have a string field at " + currentScope + "."); - } - break; - case DOUBLE: - val = json.get(actualJsonKeyName); - if (val instanceof Double) { - protoMsg.setField(fieldDescriptor, new Double((double) val)); - } else if (val instanceof Float) { - protoMsg.setField(fieldDescriptor, new Double((float) val)); - } else { - throw new IllegalArgumentException( - "JSONObject does not have a double field at " + currentScope + "."); - } - break; - case MESSAGE: - Message.Builder message = protoMsg.newBuilderForField(fieldDescriptor); - try { + break; + case DOUBLE: + val = json.get(actualJsonKeyName); + if (val instanceof Double) { + protoMsg.setField(fieldDescriptor, new Double((double) val)); + } else if (val instanceof Float) { + protoMsg.setField(fieldDescriptor, new Double((float) val)); + } else { + throw new JSONException(""); + } + break; + case MESSAGE: + Message.Builder message = protoMsg.newBuilderForField(fieldDescriptor); protoMsg.setField( fieldDescriptor, convertJsonToProtoMessageImpl( fieldDescriptor.getMessageType(), json.getJSONObject(actualJsonKeyName), currentScope, - false, + /*topLevel =*/ false, allowUnknownFields)); - } catch (JSONException e) { - throw new IllegalArgumentException( - "JSONObject does not have a object field at " + currentScope + "."); - } - break; + break; + } + } catch (JSONException e) { + throw new IllegalArgumentException( + String.format( + "JSONObject does not have a %s field at %s.", + FieldTypeToDebugMessage.get(fieldDescriptor.getType()), currentScope)); } } @@ -261,12 +256,8 @@ private static void fillRepeatedField( protoMsg.addRepeatedField(fieldDescriptor, new Boolean(jsonArray.getBoolean(i))); } catch (JSONException e) { throw new IllegalArgumentException( - "JSONObject does not have a boolean field at " - + currentScope - + "[" - + i - + "]" - + "."); + String.format( + "JSONObject does not have a boolean field at %s[%d].", currentScope, i)); } } break; @@ -276,7 +267,8 @@ private static void fillRepeatedField( protoMsg.addRepeatedField(fieldDescriptor, jsonArray.getString(i).getBytes()); } catch (JSONException e) { throw new IllegalArgumentException( - "JSONObject does not have a string field at " + currentScope + "[" + i + "]" + "."); + String.format( + "JSONObject does not have a string field at %s[%d].", currentScope, i)); } } break; @@ -289,7 +281,8 @@ private static void fillRepeatedField( protoMsg.addRepeatedField(fieldDescriptor, new Long((Long) val)); } else { throw new IllegalArgumentException( - "JSONObject does not have a int64 field at " + currentScope + "[" + i + "]" + "."); + String.format( + "JSONObject does not have a int64 field at %s[%d].", currentScope, i)); } } break; @@ -300,7 +293,8 @@ private static void fillRepeatedField( protoMsg.addRepeatedField(fieldDescriptor, new Integer((Integer) val)); } else { throw new IllegalArgumentException( - "JSONObject does not have a int32 field at " + currentScope + "[" + i + "]" + "."); + String.format( + "JSONObject does not have a int32 field at %s[%d].", currentScope, i)); } } break; @@ -310,7 +304,8 @@ private static void fillRepeatedField( protoMsg.addRepeatedField(fieldDescriptor, jsonArray.getString(i)); } catch (JSONException e) { throw new IllegalArgumentException( - "JSONObject does not have a string field at " + currentScope + "[" + i + "]" + "."); + String.format( + "JSONObject does not have a string field at %s[%d].", currentScope, i)); } } break; @@ -323,7 +318,8 @@ private static void fillRepeatedField( protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val)); } else { throw new IllegalArgumentException( - "JSONObject does not have a double field at " + currentScope + "[" + i + "]" + "."); + String.format( + "JSONObject does not have a double field at %s[%d].", currentScope, i)); } } break; @@ -337,11 +333,12 @@ private static void fillRepeatedField( fieldDescriptor.getMessageType(), jsonArray.getJSONObject(i), currentScope, - false, + /*topLevel =*/ false, allowUnknownFields)); } catch (JSONException e) { throw new IllegalArgumentException( - "JSONObject does not have a object field at " + currentScope + "[" + i + "]" + "."); + String.format( + "JSONObject does not have a object field at %s[%d].", currentScope, i)); } } break; From 1ae4ece129dad94834662fe9daca60571a5b0e42 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 14 Jul 2020 12:39:10 -0500 Subject: [PATCH 09/21] Fix test errors --- .../cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index 4a711b0293..0016c2b3d1 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -409,6 +409,7 @@ public void testStructComplex() throws Exception { json.put("test_bytes", "hello"); json.put("test_bool", true); json.put("test_DOUBLe", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4})); + json.put("test_date", 1); json.put("complexLvl1", complexLvl1); json.put("complexLVL2", complexLvl2); @@ -432,6 +433,7 @@ public void testStructComplexFail() throws Exception { json.put("test_bytes", "hello"); json.put("test_bool", true); json.put("test_double", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4})); + json.put("test_date", 1); json.put("complexLvl1", complexLvl1); json.put("complexLvl2", complexLvl2); From cd3d5bfff84bca43347ffae1ee60a18a9a0334eb Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 14 Jul 2020 16:39:21 -0500 Subject: [PATCH 10/21] Change loopiing all proto fields to looping all json fields since # of json fields <= # of proto fields --- .../storage/v1alpha2/JsonToProtoMessage.java | 88 ++++++++----------- .../v1alpha2/JsonToProtoMessageTest.java | 78 +++++++++------- .../src/test/proto/jsonTest.proto | 10 --- 3 files changed, 81 insertions(+), 95 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index d85f478a6d..fce3113fcf 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -20,10 +20,8 @@ import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.DynamicMessage; import com.google.protobuf.Message; -import java.util.HashMap; -import java.util.List; -import java.util.Set; -import java.util.TreeSet; +import com.google.protobuf.UninitializedMessageException; +import java.util.HashSet; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -76,69 +74,53 @@ private static DynamicMessage convertJsonToProtoMessageImpl( boolean topLevel, boolean allowUnknownFields) throws IllegalArgumentException { - DynamicMessage.Builder protoMsg = DynamicMessage.newBuilder(protoSchema); - List protoFields = protoSchema.getFields(); - - Set protoFieldNames = new TreeSet(String.CASE_INSENSITIVE_ORDER); - for (FieldDescriptor field : protoFields) { + HashSet protoFieldNames = new HashSet(); + for (FieldDescriptor field : protoSchema.getFields()) { protoFieldNames.add(field.getName()); } - - HashMap jsonLowercaseNameToJsonName = new HashMap(); String[] jsonNames = JSONObject.getNames(json); - for (int i = 0; i < jsonNames.length; i++) { - jsonLowercaseNameToJsonName.put(jsonNames[i].toLowerCase(), jsonNames[i]); - } - - if (!allowUnknownFields) { + int matchedFields = 0; + if (jsonNames != null) { for (int i = 0; i < jsonNames.length; i++) { - if (!protoFieldNames.contains(jsonNames[i])) { - throw new IllegalArgumentException( - String.format( - "JSONObject has fields unknown to BigQuery: %s.%s. Set allowUnknownFields to True to allow unknown fields.", - jsonScope, jsonNames[i])); + String jsonName = jsonNames[i]; + String jsonLowercaseName = jsonName.toLowerCase(); + String currentScope = jsonScope + "." + jsonName; + if (!protoFieldNames.contains(jsonLowercaseName)) { + if (!allowUnknownFields) { + throw new IllegalArgumentException( + String.format( + "JSONObject has fields unknown to BigQuery: %s. Set allowUnknownFields to True to allow unknown fields.", + currentScope)); + } else { + continue; + } } - } - } - - int matchedFields = 0; - for (FieldDescriptor field : protoFields) { - String lowercaseFieldName = field.getName().toLowerCase(); - String currentScope = jsonScope + "." + field.getName(); - - if (!jsonLowercaseNameToJsonName.containsKey(lowercaseFieldName)) { - if (field.isRequired()) { - throw new IllegalArgumentException( - "JSONObject does not have the required field " + currentScope + "."); + FieldDescriptor field = protoSchema.findFieldByName(jsonLowercaseName); + matchedFields++; + if (!field.isRepeated()) { + fillField(protoMsg, field, json, jsonName, currentScope, allowUnknownFields); } else { - continue; + fillRepeatedField(protoMsg, field, json, jsonName, currentScope, allowUnknownFields); } } - matchedFields++; - if (!field.isRepeated()) { - fillField( - protoMsg, - field, - json, - jsonLowercaseNameToJsonName.get(lowercaseFieldName), - currentScope, - allowUnknownFields); - } else { - fillRepeatedField( - protoMsg, - field, - json, - jsonLowercaseNameToJsonName.get(lowercaseFieldName), - currentScope, - allowUnknownFields); - } } if (matchedFields == 0 && topLevel) { throw new IllegalArgumentException( "There are no matching fields found for the JSONObject and the protocol buffer descriptor."); } - return protoMsg.build(); + DynamicMessage msg; + try { + msg = protoMsg.build(); + } catch (UninitializedMessageException e) { + String errorMsg = e.getMessage(); + int idxOfColon = errorMsg.indexOf(":"); + String missingFieldName = errorMsg.substring(idxOfColon + 2); + throw new IllegalArgumentException( + String.format( + "JSONObject does not have the required field %s.%s.", jsonScope, missingFieldName)); + } + return msg; } /** diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index 0016c2b3d1..e9d553cc84 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -125,16 +125,18 @@ public class JsonToProtoMessageTest { private void AreMatchingFieldsFilledIn(DynamicMessage proto, JSONObject json) { HashMap jsonLowercaseNameToActualName = new HashMap(); String[] actualNames = JSONObject.getNames(json); - for (int i = 0; i < actualNames.length; i++) { - jsonLowercaseNameToActualName.put(actualNames[i].toLowerCase(), actualNames[i]); - } - for (Map.Entry entry : proto.getAllFields().entrySet()) { - FieldDescriptor key = entry.getKey(); - java.lang.Object value = entry.getValue(); - if (key.isRepeated()) { - isProtoArrayJsonArrayEqual(key, value, json, jsonLowercaseNameToActualName); - } else { - isProtoFieldJsonFieldEqual(key, value, json, jsonLowercaseNameToActualName); + if (actualNames != null) { + for (int i = 0; i < actualNames.length; i++) { + jsonLowercaseNameToActualName.put(actualNames[i].toLowerCase(), actualNames[i]); + } + for (Map.Entry entry : proto.getAllFields().entrySet()) { + FieldDescriptor key = entry.getKey(); + java.lang.Object value = entry.getValue(); + if (key.isRepeated()) { + isProtoArrayJsonArrayEqual(key, value, json, jsonLowercaseNameToActualName); + } else { + isProtoFieldJsonFieldEqual(key, value, json, jsonLowercaseNameToActualName); + } } } } @@ -396,12 +398,12 @@ public void testStructSimpleFail() throws Exception { @Test public void testStructComplex() throws Exception { - JSONObject complexLvl2 = new JSONObject(); - complexLvl2.put("test_int", 3); + JSONObject complex_lvl2 = new JSONObject(); + complex_lvl2.put("test_int", 3); - JSONObject complexLvl1 = new JSONObject(); - complexLvl1.put("test_int", 2); - complexLvl1.put("complexLvl2", complexLvl2); + JSONObject complex_lvl1 = new JSONObject(); + complex_lvl1.put("test_int", 2); + complex_lvl1.put("complex_lvl2", complex_lvl2); JSONObject json = new JSONObject(); json.put("test_int", 1); @@ -410,8 +412,8 @@ public void testStructComplex() throws Exception { json.put("test_bool", true); json.put("test_DOUBLe", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4})); json.put("test_date", 1); - json.put("complexLvl1", complexLvl1); - json.put("complexLVL2", complexLvl2); + json.put("complex_lvl1", complex_lvl1); + json.put("complex_lvl2", complex_lvl2); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json, false); @@ -420,12 +422,12 @@ public void testStructComplex() throws Exception { @Test public void testStructComplexFail() throws Exception { - JSONObject complexLvl2 = new JSONObject(); - complexLvl2.put("test_int", 3); + JSONObject complex_lvl2 = new JSONObject(); + complex_lvl2.put("test_int", 3); - JSONObject complexLvl1 = new JSONObject(); - complexLvl1.put("test_int", "not_int"); - complexLvl1.put("complexLvl2", complexLvl2); + JSONObject complex_lvl1 = new JSONObject(); + complex_lvl1.put("test_int", "not_int"); + complex_lvl1.put("complex_lvl2", complex_lvl2); JSONObject json = new JSONObject(); json.put("test_int", 1); @@ -434,15 +436,15 @@ public void testStructComplexFail() throws Exception { json.put("test_bool", true); json.put("test_double", new JSONArray(new Double[] {1.1, 2.2, 3.3, 4.4})); json.put("test_date", 1); - json.put("complexLvl1", complexLvl1); - json.put("complexLvl2", complexLvl2); + json.put("complex_lvl1", complex_lvl1); + json.put("complex_lvl2", complex_lvl2); try { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json, false); } catch (IllegalArgumentException e) { assertEquals( - e.getMessage(), "JSONObject does not have a int64 field at root.complexLvl1.test_int."); + e.getMessage(), "JSONObject does not have a int64 field at root.complex_lvl1.test_int."); } } @@ -511,6 +513,18 @@ public void testAllowUnknownFields() throws Exception { AreMatchingFieldsFilledIn(protoMsg, json); } + @Test + public void testEmptySecondLevelObject() throws Exception { + JSONObject complexLvl2 = new JSONObject(); + JSONObject json = new JSONObject(); + json.put("test_int", 1); + json.put("complex_lvl2", complexLvl2); + + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, true); + AreMatchingFieldsFilledIn(protoMsg, json); + } + @Test public void testAllowUnknownFieldsError() throws Exception { JSONObject json = new JSONObject(); @@ -540,11 +554,11 @@ public void testEmptyJSONObject() throws Exception { @Test public void testAllowUnknownFieldsSecondLevel() throws Exception { - JSONObject complexLvl2 = new JSONObject(); - complexLvl2.put("no_match", 1); + JSONObject complex_lvl2 = new JSONObject(); + complex_lvl2.put("no_match", 1); JSONObject json = new JSONObject(); json.put("test_int", 1); - json.put("complexLvl2", complexLvl2); + json.put("complex_lvl2", complex_lvl2); try { DynamicMessage protoMsg = @@ -552,7 +566,7 @@ public void testAllowUnknownFieldsSecondLevel() throws Exception { } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), - "JSONObject has fields unknown to BigQuery: root.complexLvl2.no_match. Set allowUnknownFields to True to allow unknown fields."); + "JSONObject has fields unknown to BigQuery: root.complex_lvl2.no_match. Set allowUnknownFields to True to allow unknown fields."); } } @@ -574,11 +588,11 @@ public void testTopLevelMismatch() throws Exception { @Test public void testTopLevelMatchSecondLevelMismatch() throws Exception { - JSONObject complexLvl2 = new JSONObject(); - complexLvl2.put("no_match", 1); + JSONObject complex_lvl2 = new JSONObject(); + complex_lvl2.put("no_match", 1); JSONObject json = new JSONObject(); json.put("test_int", 1); - json.put("complexLvl2", complexLvl2); + json.put("complex_lvl2", complex_lvl2); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, true); diff --git a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto index 8e3d1d9675..9f44996e27 100644 --- a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto +++ b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto @@ -13,16 +13,6 @@ message ComplexRoot { required ComplexLvl2 complex_lvl2 = 8; } -message CasingComplex { - optional int64 test_int = 1; - repeated string test_string = 2; - required bytes test_bytes = 3; - optional bool test_bool = 4; - repeated double test_double = 5; - required int32 test_date = 6; - required OptionTest option_test = 7; -} - message ComplexLvl1 { optional int64 test_int = 1; required ComplexLvl2 complex_lvl2 = 2; From 4804ea9623a39caa34d7ce2ac28565d85c55e88f Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 14 Jul 2020 17:35:04 -0500 Subject: [PATCH 11/21] Remove unuse variable --- .../cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index fce3113fcf..ef96cdf4d5 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -26,7 +26,9 @@ import org.json.JSONException; import org.json.JSONObject; -/** Converts Json data to protocol buffer messages given the protocol buffer descriptor. */ +/** Converts Json data to protocol buffer messages given the protocol buffer descriptor. + * The protobuf descriptor must have all fields lowercased. + */ public class JsonToProtoMessage { private static ImmutableMap FieldTypeToDebugMessage = new ImmutableMap.Builder() @@ -76,6 +78,7 @@ private static DynamicMessage convertJsonToProtoMessageImpl( throws IllegalArgumentException { DynamicMessage.Builder protoMsg = DynamicMessage.newBuilder(protoSchema); HashSet protoFieldNames = new HashSet(); + // These protofields should already be lowercased. for (FieldDescriptor field : protoSchema.getFields()) { protoFieldNames.add(field.getName()); } @@ -143,7 +146,6 @@ private static void fillField( boolean allowUnknownFields) throws IllegalArgumentException { java.lang.Object val; - String error; try { switch (fieldDescriptor.getType()) { case BOOL: From 7f27de6c6badadd0676b0c407ae9169db5ff3144 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 14 Jul 2020 17:35:20 -0500 Subject: [PATCH 12/21] Remove unuse variable --- .../cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index ef96cdf4d5..3c235ef744 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -26,8 +26,9 @@ import org.json.JSONException; import org.json.JSONObject; -/** Converts Json data to protocol buffer messages given the protocol buffer descriptor. - * The protobuf descriptor must have all fields lowercased. +/** + * Converts Json data to protocol buffer messages given the protocol buffer descriptor. The protobuf + * descriptor must have all fields lowercased. */ public class JsonToProtoMessage { private static ImmutableMap FieldTypeToDebugMessage = From 46db080bc8ff8447f411d3831b35faa3ef7c4b85 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 14 Jul 2020 17:43:15 -0500 Subject: [PATCH 13/21] Removed unnecessary set --- .../storage/v1alpha2/JsonToProtoMessage.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index 3c235ef744..4cab8493ed 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -21,7 +21,6 @@ import com.google.protobuf.DynamicMessage; import com.google.protobuf.Message; import com.google.protobuf.UninitializedMessageException; -import java.util.HashSet; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -77,20 +76,18 @@ private static DynamicMessage convertJsonToProtoMessageImpl( boolean topLevel, boolean allowUnknownFields) throws IllegalArgumentException { + DynamicMessage.Builder protoMsg = DynamicMessage.newBuilder(protoSchema); - HashSet protoFieldNames = new HashSet(); - // These protofields should already be lowercased. - for (FieldDescriptor field : protoSchema.getFields()) { - protoFieldNames.add(field.getName()); - } String[] jsonNames = JSONObject.getNames(json); int matchedFields = 0; + // JsonNames will be null if the JSONObject is empty. if (jsonNames != null) { for (int i = 0; i < jsonNames.length; i++) { String jsonName = jsonNames[i]; String jsonLowercaseName = jsonName.toLowerCase(); String currentScope = jsonScope + "." + jsonName; - if (!protoFieldNames.contains(jsonLowercaseName)) { + FieldDescriptor field = protoSchema.findFieldByName(jsonLowercaseName); + if (field == null) { if (!allowUnknownFields) { throw new IllegalArgumentException( String.format( @@ -100,7 +97,6 @@ private static DynamicMessage convertJsonToProtoMessageImpl( continue; } } - FieldDescriptor field = protoSchema.findFieldByName(jsonLowercaseName); matchedFields++; if (!field.isRepeated()) { fillField(protoMsg, field, json, jsonName, currentScope, allowUnknownFields); From df4247386ac9758f2fc1eece933974170fcdab5f Mon Sep 17 00:00:00 2001 From: allenc3 Date: Wed, 15 Jul 2020 13:57:54 -0500 Subject: [PATCH 14/21] Fixed according to PR, and added proto message empty test case. --- .../storage/v1alpha2/JsonToProtoMessage.java | 208 +++++++++--------- .../v1alpha2/JsonToProtoMessageTest.java | 24 ++ 2 files changed, 125 insertions(+), 107 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index 4cab8493ed..b454eeb3b0 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -52,6 +52,9 @@ public class JsonToProtoMessage { public static DynamicMessage convertJsonToProtoMessage( Descriptor protoSchema, JSONObject json, boolean allowUnknownFields) throws IllegalArgumentException { + if (json == null) { + throw new IllegalArgumentException("JSONObject is null."); + } if (json.length() == 0) { throw new IllegalArgumentException("JSONObject is empty."); } @@ -79,32 +82,35 @@ private static DynamicMessage convertJsonToProtoMessageImpl( DynamicMessage.Builder protoMsg = DynamicMessage.newBuilder(protoSchema); String[] jsonNames = JSONObject.getNames(json); + if (jsonNames == null) { + return protoMsg.build(); + } int matchedFields = 0; - // JsonNames will be null if the JSONObject is empty. - if (jsonNames != null) { - for (int i = 0; i < jsonNames.length; i++) { - String jsonName = jsonNames[i]; - String jsonLowercaseName = jsonName.toLowerCase(); - String currentScope = jsonScope + "." + jsonName; - FieldDescriptor field = protoSchema.findFieldByName(jsonLowercaseName); - if (field == null) { - if (!allowUnknownFields) { - throw new IllegalArgumentException( - String.format( - "JSONObject has fields unknown to BigQuery: %s. Set allowUnknownFields to True to allow unknown fields.", - currentScope)); - } else { - continue; - } - } - matchedFields++; - if (!field.isRepeated()) { - fillField(protoMsg, field, json, jsonName, currentScope, allowUnknownFields); + for (int i = 0; i < jsonNames.length; i++) { + String jsonName = jsonNames[i]; + // We want lowercase here to support case-insensitive data writes. + // The protobuf descriptor that is used is assumed to have all lowercased fields + String jsonLowercaseName = jsonName.toLowerCase(); + String currentScope = jsonScope + "." + jsonName; + FieldDescriptor field = protoSchema.findFieldByName(jsonLowercaseName); + if (field == null) { + if (!allowUnknownFields) { + throw new IllegalArgumentException( + String.format( + "JSONObject has fields unknown to BigQuery: %s. Set allowUnknownFields to True to allow unknown fields.", + currentScope)); } else { - fillRepeatedField(protoMsg, field, json, jsonName, currentScope, allowUnknownFields); + continue; } } + matchedFields++; + if (!field.isRepeated()) { + fillField(protoMsg, field, json, jsonName, currentScope, allowUnknownFields); + } else { + fillRepeatedField(protoMsg, field, json, jsonName, currentScope, allowUnknownFields); + } } + if (matchedFields == 0 && topLevel) { throw new IllegalArgumentException( "There are no matching fields found for the JSONObject and the protocol buffer descriptor."); @@ -120,6 +126,9 @@ private static DynamicMessage convertJsonToProtoMessageImpl( String.format( "JSONObject does not have the required field %s.%s.", jsonScope, missingFieldName)); } + if (topLevel && msg.getSerializedSize() == 0) { + throw new IllegalArgumentException("The created protobuf message is empty."); + } return msg; } @@ -129,7 +138,7 @@ private static DynamicMessage convertJsonToProtoMessageImpl( * @param protoMsg The protocol buffer message being constructed * @param fieldDescriptor * @param json - * @param actualJsonKeyName Actual key name in JSONObject instead of lowercased version + * @param exactJsonKeyName Actual key name in JSONObject instead of lowercased version * @param currentScope Debugging purposes * @param allowUnknownFields Ignores unknown JSON fields. * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. @@ -138,7 +147,7 @@ private static void fillField( DynamicMessage.Builder protoMsg, FieldDescriptor fieldDescriptor, JSONObject json, - String actualJsonKeyName, + String exactJsonKeyName, String currentScope, boolean allowUnknownFields) throws IllegalArgumentException { @@ -146,13 +155,13 @@ private static void fillField( try { switch (fieldDescriptor.getType()) { case BOOL: - protoMsg.setField(fieldDescriptor, new Boolean(json.getBoolean(actualJsonKeyName))); + protoMsg.setField(fieldDescriptor, new Boolean(json.getBoolean(exactJsonKeyName))); break; case BYTES: - protoMsg.setField(fieldDescriptor, json.getString(actualJsonKeyName).getBytes()); + protoMsg.setField(fieldDescriptor, json.getString(exactJsonKeyName).getBytes()); break; case INT64: - val = json.get(actualJsonKeyName); + val = json.get(exactJsonKeyName); if (val instanceof Integer) { protoMsg.setField(fieldDescriptor, new Long((Integer) val)); } else if (val instanceof Long) { @@ -162,7 +171,7 @@ private static void fillField( } break; case INT32: - val = json.get(actualJsonKeyName); + val = json.get(exactJsonKeyName); if (val instanceof Integer) { protoMsg.setField(fieldDescriptor, new Integer((Integer) val)); } else { @@ -170,10 +179,10 @@ private static void fillField( } break; case STRING: - protoMsg.setField(fieldDescriptor, json.getString(actualJsonKeyName)); + protoMsg.setField(fieldDescriptor, json.getString(exactJsonKeyName)); break; case DOUBLE: - val = json.get(actualJsonKeyName); + val = json.get(exactJsonKeyName); if (val instanceof Double) { protoMsg.setField(fieldDescriptor, new Double((double) val)); } else if (val instanceof Float) { @@ -188,7 +197,7 @@ private static void fillField( fieldDescriptor, convertJsonToProtoMessageImpl( fieldDescriptor.getMessageType(), - json.getJSONObject(actualJsonKeyName), + json.getJSONObject(exactJsonKeyName), currentScope, /*topLevel =*/ false, allowUnknownFields)); @@ -208,7 +217,7 @@ private static void fillField( * @param protoMsg The protocol buffer message being constructed * @param fieldDescriptor * @param json If root level has no matching fields, throws exception. - * @param actualJsonKeyName Actual key name in JSONObject instead of lowercased version + * @param exactJsonKeyName Actual key name in JSONObject instead of lowercased version * @param currentScope Debugging purposes * @param allowUnknownFields Ignores unknown JSON fields. * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. @@ -217,96 +226,80 @@ private static void fillRepeatedField( DynamicMessage.Builder protoMsg, FieldDescriptor fieldDescriptor, JSONObject json, - String actualJsonKeyName, + String exactJsonKeyName, String currentScope, boolean allowUnknownFields) throws IllegalArgumentException { JSONArray jsonArray; try { - jsonArray = json.getJSONArray(actualJsonKeyName); + jsonArray = json.getJSONArray(exactJsonKeyName); } catch (JSONException e) { throw new IllegalArgumentException( "JSONObject does not have a array field at " + currentScope + "."); } java.lang.Object val; - switch (fieldDescriptor.getType()) { - case BOOL: - for (int i = 0; i < jsonArray.length(); i++) { - try { + int index = -1; + try { + switch (fieldDescriptor.getType()) { + case BOOL: + for (int i = 0; i < jsonArray.length(); i++) { + index = i; protoMsg.addRepeatedField(fieldDescriptor, new Boolean(jsonArray.getBoolean(i))); - } catch (JSONException e) { - throw new IllegalArgumentException( - String.format( - "JSONObject does not have a boolean field at %s[%d].", currentScope, i)); } - } - break; - case BYTES: - for (int i = 0; i < jsonArray.length(); i++) { - try { + break; + case BYTES: + for (int i = 0; i < jsonArray.length(); i++) { + index = i; protoMsg.addRepeatedField(fieldDescriptor, jsonArray.getString(i).getBytes()); - } catch (JSONException e) { - throw new IllegalArgumentException( - String.format( - "JSONObject does not have a string field at %s[%d].", currentScope, i)); } - } - break; - case INT64: - for (int i = 0; i < jsonArray.length(); i++) { - val = jsonArray.get(i); - if (val instanceof Integer) { - protoMsg.addRepeatedField(fieldDescriptor, new Long((Integer) val)); - } else if (val instanceof Long) { - protoMsg.addRepeatedField(fieldDescriptor, new Long((Long) val)); - } else { - throw new IllegalArgumentException( - String.format( - "JSONObject does not have a int64 field at %s[%d].", currentScope, i)); + break; + case INT64: + for (int i = 0; i < jsonArray.length(); i++) { + index = i; + val = jsonArray.get(i); + if (val instanceof Integer) { + protoMsg.addRepeatedField(fieldDescriptor, new Long((Integer) val)); + } else if (val instanceof Long) { + protoMsg.addRepeatedField(fieldDescriptor, new Long((Long) val)); + } else { + throw new JSONException(""); + } } - } - break; - case INT32: - for (int i = 0; i < jsonArray.length(); i++) { - val = jsonArray.get(i); - if (val instanceof Integer) { - protoMsg.addRepeatedField(fieldDescriptor, new Integer((Integer) val)); - } else { - throw new IllegalArgumentException( - String.format( - "JSONObject does not have a int32 field at %s[%d].", currentScope, i)); + break; + case INT32: + for (int i = 0; i < jsonArray.length(); i++) { + index = i; + val = jsonArray.get(i); + if (val instanceof Integer) { + protoMsg.addRepeatedField(fieldDescriptor, new Integer((Integer) val)); + } else { + throw new JSONException(""); + } } - } - break; - case STRING: - for (int i = 0; i < jsonArray.length(); i++) { - try { + break; + case STRING: + for (int i = 0; i < jsonArray.length(); i++) { + index = i; protoMsg.addRepeatedField(fieldDescriptor, jsonArray.getString(i)); - } catch (JSONException e) { - throw new IllegalArgumentException( - String.format( - "JSONObject does not have a string field at %s[%d].", currentScope, i)); } - } - break; - case DOUBLE: - for (int i = 0; i < jsonArray.length(); i++) { - val = jsonArray.get(i); - if (val instanceof Double) { - protoMsg.addRepeatedField(fieldDescriptor, new Double((double) val)); - } else if (val instanceof Float) { - protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val)); - } else { - throw new IllegalArgumentException( - String.format( - "JSONObject does not have a double field at %s[%d].", currentScope, i)); + break; + case DOUBLE: + for (int i = 0; i < jsonArray.length(); i++) { + index = i; + val = jsonArray.get(i); + if (val instanceof Double) { + protoMsg.addRepeatedField(fieldDescriptor, new Double((double) val)); + } else if (val instanceof Float) { + protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val)); + } else { + throw new JSONException(""); + } } - } - break; - case MESSAGE: - for (int i = 0; i < jsonArray.length(); i++) { - try { + break; + case MESSAGE: + for (int i = 0; i < jsonArray.length(); i++) { + index = i; Message.Builder message = protoMsg.newBuilderForField(fieldDescriptor); protoMsg.addRepeatedField( fieldDescriptor, @@ -316,13 +309,14 @@ private static void fillRepeatedField( currentScope, /*topLevel =*/ false, allowUnknownFields)); - } catch (JSONException e) { - throw new IllegalArgumentException( - String.format( - "JSONObject does not have a object field at %s[%d].", currentScope, i)); } - } - break; + break; + } + } catch (JSONException e) { + throw new IllegalArgumentException( + String.format( + "JSONObject does not have a %s field at %s[%d].", + FieldTypeToDebugMessage.get(fieldDescriptor.getType()), currentScope, index)); } } } diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index e9d553cc84..ffe563aa30 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -541,6 +541,20 @@ public void testAllowUnknownFieldsError() throws Exception { } } + @Test + public void testEmptyProtoMessage() throws Exception { + JSONObject json = new JSONObject(); + json.put("test_repeated", new JSONArray(new int[0])); + json.put("string", "hello"); + + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json, true); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "The created protobuf message is empty."); + } + } + @Test public void testEmptyJSONObject() throws Exception { JSONObject json = new JSONObject(); @@ -552,6 +566,16 @@ public void testEmptyJSONObject() throws Exception { } } + @Test + public void testNull() throws Exception { + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), null, false); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "JSONObject is null."); + } + } + @Test public void testAllowUnknownFieldsSecondLevel() throws Exception { JSONObject complex_lvl2 = new JSONObject(); From 155e5ffc605589878eba361e7eac36911440b0a9 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Thu, 16 Jul 2020 11:27:40 -0500 Subject: [PATCH 15/21] Pushed for loop to be outside of the try catches to remove unnecessary for loops. --- .../storage/v1alpha2/JsonToProtoMessage.java | 72 +++++++------------ .../v1alpha2/JsonToProtoMessageTest.java | 69 +++++++----------- 2 files changed, 53 insertions(+), 88 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index b454eeb3b0..eb6b863434 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -138,7 +138,7 @@ private static DynamicMessage convertJsonToProtoMessageImpl( * @param protoMsg The protocol buffer message being constructed * @param fieldDescriptor * @param json - * @param exactJsonKeyName Actual key name in JSONObject instead of lowercased version + * @param exactJsonKeyName Exact key name in JSONObject instead of lowercased version * @param currentScope Debugging purposes * @param allowUnknownFields Ignores unknown JSON fields. * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. @@ -165,7 +165,7 @@ private static void fillField( if (val instanceof Integer) { protoMsg.setField(fieldDescriptor, new Long((Integer) val)); } else if (val instanceof Long) { - protoMsg.setField(fieldDescriptor, new Long((Long) val)); + protoMsg.setField(fieldDescriptor, (Long) val); } else { throw new JSONException(""); } @@ -173,7 +173,7 @@ private static void fillField( case INT32: val = json.get(exactJsonKeyName); if (val instanceof Integer) { - protoMsg.setField(fieldDescriptor, new Integer((Integer) val)); + protoMsg.setField(fieldDescriptor, (Integer) val); } else { throw new JSONException(""); } @@ -184,9 +184,9 @@ private static void fillField( case DOUBLE: val = json.get(exactJsonKeyName); if (val instanceof Double) { - protoMsg.setField(fieldDescriptor, new Double((double) val)); + protoMsg.setField(fieldDescriptor, (Double) val); } else if (val instanceof Float) { - protoMsg.setField(fieldDescriptor, new Double((float) val)); + protoMsg.setField(fieldDescriptor, new Double((Float) val)); } else { throw new JSONException(""); } @@ -217,7 +217,7 @@ private static void fillField( * @param protoMsg The protocol buffer message being constructed * @param fieldDescriptor * @param json If root level has no matching fields, throws exception. - * @param exactJsonKeyName Actual key name in JSONObject instead of lowercased version + * @param exactJsonKeyName Exact key name in JSONObject instead of lowercased version * @param currentScope Debugging purposes * @param allowUnknownFields Ignores unknown JSON fields. * @throws IllegalArgumentException when JSON data is not compatible with proto descriptor. @@ -241,65 +241,47 @@ private static void fillRepeatedField( java.lang.Object val; int index = -1; try { - switch (fieldDescriptor.getType()) { - case BOOL: - for (int i = 0; i < jsonArray.length(); i++) { - index = i; + for (int i = 0; i < jsonArray.length(); i++) { + index = i; + switch (fieldDescriptor.getType()) { + case BOOL: protoMsg.addRepeatedField(fieldDescriptor, new Boolean(jsonArray.getBoolean(i))); - } - break; - case BYTES: - for (int i = 0; i < jsonArray.length(); i++) { - index = i; + break; + case BYTES: protoMsg.addRepeatedField(fieldDescriptor, jsonArray.getString(i).getBytes()); - } - break; - case INT64: - for (int i = 0; i < jsonArray.length(); i++) { - index = i; + break; + case INT64: val = jsonArray.get(i); if (val instanceof Integer) { protoMsg.addRepeatedField(fieldDescriptor, new Long((Integer) val)); } else if (val instanceof Long) { - protoMsg.addRepeatedField(fieldDescriptor, new Long((Long) val)); + protoMsg.addRepeatedField(fieldDescriptor, (Long) val); } else { throw new JSONException(""); } - } - break; - case INT32: - for (int i = 0; i < jsonArray.length(); i++) { - index = i; + break; + case INT32: val = jsonArray.get(i); if (val instanceof Integer) { - protoMsg.addRepeatedField(fieldDescriptor, new Integer((Integer) val)); + protoMsg.addRepeatedField(fieldDescriptor, (Integer) val); } else { throw new JSONException(""); } - } - break; - case STRING: - for (int i = 0; i < jsonArray.length(); i++) { - index = i; + break; + case STRING: protoMsg.addRepeatedField(fieldDescriptor, jsonArray.getString(i)); - } - break; - case DOUBLE: - for (int i = 0; i < jsonArray.length(); i++) { - index = i; + break; + case DOUBLE: val = jsonArray.get(i); if (val instanceof Double) { - protoMsg.addRepeatedField(fieldDescriptor, new Double((double) val)); + protoMsg.addRepeatedField(fieldDescriptor, (Double) val); } else if (val instanceof Float) { protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val)); } else { throw new JSONException(""); } - } - break; - case MESSAGE: - for (int i = 0; i < jsonArray.length(); i++) { - index = i; + break; + case MESSAGE: Message.Builder message = protoMsg.newBuilderForField(fieldDescriptor); protoMsg.addRepeatedField( fieldDescriptor, @@ -309,8 +291,8 @@ private static void fillRepeatedField( currentScope, /*topLevel =*/ false, allowUnknownFields)); - } - break; + break; + } } } catch (JSONException e) { throw new IllegalArgumentException( diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index ffe563aa30..e2d8369c76 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -179,49 +179,32 @@ private void isProtoArrayJsonArrayEqual( HashMap jsonLowercaseNameToActualName) { String fieldName = jsonLowercaseNameToActualName.get(key.getName().toLowerCase()); JSONArray jsonArray = json.getJSONArray(fieldName); - switch (key.getType()) { - case BOOL: - List boolArr = (List) value; - for (int i = 0; i < jsonArray.length(); i++) { - assertTrue((boolArr.get(i) == jsonArray.getBoolean(i))); - } - break; - case BYTES: - List byteArr = (List) value; - for (int i = 0; i < jsonArray.length(); i++) { - assertTrue(Arrays.equals(byteArr.get(i), jsonArray.getString(i).getBytes())); - } - break; - case INT64: - List longArr = (List) value; - for (int i = 0; i < jsonArray.length(); i++) { - assertTrue((longArr.get(i) == jsonArray.getLong(i))); - } - break; - case INT32: - List intArr = (List) value; - for (int i = 0; i < jsonArray.length(); i++) { - assertTrue((intArr.get(i) == jsonArray.getInt(i))); - } - break; - case STRING: - List stringArr = (List) value; - for (int i = 0; i < jsonArray.length(); i++) { - assertTrue(stringArr.get(i).equals(jsonArray.getString(i))); - } - break; - case DOUBLE: - List doubleArr = (List) value; - for (int i = 0; i < jsonArray.length(); i++) { - assertTrue((doubleArr.get(i) == jsonArray.getDouble(i))); - } - break; - case MESSAGE: - List messageArr = (List) value; - for (int i = 0; i < jsonArray.length(); i++) { - AreMatchingFieldsFilledIn(messageArr.get(i), jsonArray.getJSONObject(i)); - } - break; + for (int i = 0; i < jsonArray.length(); i++) { + switch (key.getType()) { + case BOOL: + assertTrue((((List) value).get(i) == jsonArray.getBoolean(i))); + break; + case BYTES: + assertTrue( + Arrays.equals(((List) value).get(i), jsonArray.getString(i).getBytes())); + break; + case INT64: + assertTrue((((List) value).get(i) == jsonArray.getLong(i))); + break; + case INT32: + assertTrue((((List) value).get(i) == jsonArray.getInt(i))); + break; + case STRING: + assertTrue(((List) value).get(i).equals(jsonArray.getString(i))); + break; + case DOUBLE: + assertTrue((((List) value).get(i) == jsonArray.getDouble(i))); + break; + case MESSAGE: + AreMatchingFieldsFilledIn( + ((List) value).get(i), jsonArray.getJSONObject(i)); + break; + } } } From 2d564a0ad82a204007964f16d5c569696355c95e Mon Sep 17 00:00:00 2001 From: allenc3 Date: Thu, 16 Jul 2020 16:14:59 -0500 Subject: [PATCH 16/21] Made tests based on self created protos --- .../v1alpha2/JsonToProtoMessageTest.java | 326 ++++++++++++------ 1 file changed, 223 insertions(+), 103 deletions(-) diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index e2d8369c76..9d226716b6 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -21,12 +21,11 @@ import com.google.cloud.bigquery.storage.test.JsonTest.*; import com.google.cloud.bigquery.storage.test.SchemaTest.*; import com.google.common.collect.ImmutableMap; +import com.google.protobuf.ByteString; import com.google.protobuf.Descriptors.Descriptor; -import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.DynamicMessage; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; +import com.google.protobuf.Message; +import java.util.ArrayList; import java.util.Map; import org.json.JSONArray; import org.json.JSONObject; @@ -48,6 +47,56 @@ public class JsonToProtoMessageTest { .put(ObjectType.getDescriptor(), "object") .build(); + private static ImmutableMap AllTypesToCorrectProto = + new ImmutableMap.Builder() + .put( + BoolType.getDescriptor(), + new Message[] {BoolType.newBuilder().setTestFieldType(true).build()}) + .put( + BytesType.getDescriptor(), + new Message[] { + BytesType.newBuilder() + .setTestFieldType(ByteString.copyFrom("test".getBytes())) + .build() + }) + .put( + Int64Type.getDescriptor(), + new Message[] { + Int64Type.newBuilder().setTestFieldType(Long.MAX_VALUE).build(), + Int64Type.newBuilder().setTestFieldType(new Long(Integer.MAX_VALUE)).build() + }) + .put( + Int32Type.getDescriptor(), + new Message[] {Int32Type.newBuilder().setTestFieldType(Integer.MAX_VALUE).build()}) + .put( + DoubleType.getDescriptor(), + new Message[] {DoubleType.newBuilder().setTestFieldType(1.23).build()}) + .put( + StringType.getDescriptor(), + new Message[] {StringType.newBuilder().setTestFieldType("test").build()}) + .put( + RepeatedType.getDescriptor(), + new Message[] { + RepeatedType.newBuilder() + .addAllTestFieldType( + new ArrayList() { + { + add(1L); + add(2L); + add(3L); + } + }) + .build() + }) + .put( + ObjectType.getDescriptor(), + new Message[] { + ObjectType.newBuilder() + .setTestFieldType(ComplexLvl2.newBuilder().setTestInt(1).build()) + .build() + }) + .build(); + private static ImmutableMap AllRepeatedTypesToDebugMessageTest = new ImmutableMap.Builder() .put(RepeatedBool.getDescriptor(), "boolean") @@ -55,9 +104,92 @@ public class JsonToProtoMessageTest { .put(RepeatedInt64.getDescriptor(), "int64") .put(RepeatedInt32.getDescriptor(), "int32") .put(RepeatedDouble.getDescriptor(), "double") + .put(RepeatedString.getDescriptor(), "string") .put(RepeatedObject.getDescriptor(), "object") .build(); + private static ImmutableMap AllRepeatedTypesToCorrectProto = + new ImmutableMap.Builder() + .put( + RepeatedBool.getDescriptor(), + new Message[] { + RepeatedBool.newBuilder().addTestRepeated(true).addTestRepeated(false).build() + }) + .put( + RepeatedBytes.getDescriptor(), + new Message[] { + RepeatedBytes.newBuilder() + .addTestRepeated(ByteString.copyFrom("hello".getBytes())) + .addTestRepeated(ByteString.copyFrom("test".getBytes())) + .build() + }) + .put( + RepeatedString.getDescriptor(), + new Message[] { + RepeatedString.newBuilder().addTestRepeated("hello").addTestRepeated("test").build() + }) + .put( + RepeatedInt64.getDescriptor(), + new Message[] { + RepeatedInt64.newBuilder() + .addTestRepeated(Long.MAX_VALUE) + .addTestRepeated(Long.MIN_VALUE) + .addTestRepeated(Integer.MAX_VALUE) + .addTestRepeated(Integer.MIN_VALUE) + .addTestRepeated(Short.MAX_VALUE) + .addTestRepeated(Short.MIN_VALUE) + .addTestRepeated(Byte.MAX_VALUE) + .addTestRepeated(Byte.MIN_VALUE) + .addTestRepeated(0) + .build(), + RepeatedInt64.newBuilder() + .addTestRepeated(Integer.MAX_VALUE) + .addTestRepeated(Integer.MIN_VALUE) + .addTestRepeated(Short.MAX_VALUE) + .addTestRepeated(Short.MIN_VALUE) + .addTestRepeated(Byte.MAX_VALUE) + .addTestRepeated(Byte.MIN_VALUE) + .addTestRepeated(0) + .build() + }) + .put( + RepeatedInt32.getDescriptor(), + new Message[] { + RepeatedInt32.newBuilder() + .addTestRepeated(Integer.MAX_VALUE) + .addTestRepeated(Integer.MIN_VALUE) + .addTestRepeated(Short.MAX_VALUE) + .addTestRepeated(Short.MIN_VALUE) + .addTestRepeated(Byte.MAX_VALUE) + .addTestRepeated(Byte.MIN_VALUE) + .addTestRepeated(0) + .build() + }) + .put( + RepeatedDouble.getDescriptor(), + new Message[] { + RepeatedDouble.newBuilder() + .addTestRepeated(Double.MAX_VALUE) + .addTestRepeated(Double.MIN_VALUE) + .addTestRepeated(Float.MAX_VALUE) + .addTestRepeated(Float.MIN_VALUE) + .build(), + RepeatedDouble.newBuilder() + .addTestRepeated(Float.MAX_VALUE) + .addTestRepeated(Float.MIN_VALUE) + .build() + }) + .put( + RepeatedObject.getDescriptor(), + new Message[] { + RepeatedObject.newBuilder() + .addTestRepeated(ComplexLvl2.newBuilder().setTestInt(1).build()) + .addTestRepeated(ComplexLvl2.newBuilder().setTestInt(2).build()) + .addTestRepeated(ComplexLvl2.newBuilder().setTestInt(3).build()) + .build() + }) + .build(); + private static JSONObject[] simpleJSONObjects = { new JSONObject().put("test_field_type", Long.MAX_VALUE), new JSONObject().put("test_field_type", Integer.MAX_VALUE), @@ -122,94 +254,11 @@ public class JsonToProtoMessageTest { })) }; - private void AreMatchingFieldsFilledIn(DynamicMessage proto, JSONObject json) { - HashMap jsonLowercaseNameToActualName = new HashMap(); - String[] actualNames = JSONObject.getNames(json); - if (actualNames != null) { - for (int i = 0; i < actualNames.length; i++) { - jsonLowercaseNameToActualName.put(actualNames[i].toLowerCase(), actualNames[i]); - } - for (Map.Entry entry : proto.getAllFields().entrySet()) { - FieldDescriptor key = entry.getKey(); - java.lang.Object value = entry.getValue(); - if (key.isRepeated()) { - isProtoArrayJsonArrayEqual(key, value, json, jsonLowercaseNameToActualName); - } else { - isProtoFieldJsonFieldEqual(key, value, json, jsonLowercaseNameToActualName); - } - } - } - } - - private void isProtoFieldJsonFieldEqual( - FieldDescriptor key, - java.lang.Object value, - JSONObject json, - HashMap jsonLowercaseNameToActualName) { - String fieldName = jsonLowercaseNameToActualName.get(key.getName().toLowerCase()); - switch (key.getType()) { - case BOOL: - assertTrue((Boolean) value == json.getBoolean(fieldName)); - break; - case BYTES: - assertTrue(Arrays.equals((byte[]) value, json.getString(fieldName).getBytes())); - break; - case INT64: - assertTrue((long) value == json.getLong(fieldName)); - break; - case INT32: - assertTrue((int) value == json.getInt(fieldName)); - break; - case STRING: - assertTrue(((String) value).equals(json.getString(fieldName))); - break; - case DOUBLE: - assertTrue((double) value == json.getDouble(fieldName)); - break; - case MESSAGE: - AreMatchingFieldsFilledIn((DynamicMessage) value, json.getJSONObject(fieldName)); - break; - } - } - - private void isProtoArrayJsonArrayEqual( - FieldDescriptor key, - java.lang.Object value, - JSONObject json, - HashMap jsonLowercaseNameToActualName) { - String fieldName = jsonLowercaseNameToActualName.get(key.getName().toLowerCase()); - JSONArray jsonArray = json.getJSONArray(fieldName); - for (int i = 0; i < jsonArray.length(); i++) { - switch (key.getType()) { - case BOOL: - assertTrue((((List) value).get(i) == jsonArray.getBoolean(i))); - break; - case BYTES: - assertTrue( - Arrays.equals(((List) value).get(i), jsonArray.getString(i).getBytes())); - break; - case INT64: - assertTrue((((List) value).get(i) == jsonArray.getLong(i))); - break; - case INT32: - assertTrue((((List) value).get(i) == jsonArray.getInt(i))); - break; - case STRING: - assertTrue(((List) value).get(i).equals(jsonArray.getString(i))); - break; - case DOUBLE: - assertTrue((((List) value).get(i) == jsonArray.getDouble(i))); - break; - case MESSAGE: - AreMatchingFieldsFilledIn( - ((List) value).get(i), jsonArray.getJSONObject(i)); - break; - } - } - } - @Test public void testDifferentNameCasing() throws Exception { + TestInt64 correctProto = + TestInt64.newBuilder().setByte(1).setShort(1).setInt(1).setLong(1).build(); + JSONObject json = new JSONObject(); json.put("bYtE", (byte) 1); json.put("SHORT", (short) 1); @@ -217,11 +266,13 @@ public void testDifferentNameCasing() throws Exception { json.put("lONg", 1L); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test public void testInt64() throws Exception { + TestInt64 correctProto = + TestInt64.newBuilder().setByte(1).setShort(1).setInt(1).setLong(1).build(); JSONObject json = new JSONObject(); json.put("byte", (byte) 1); json.put("short", (short) 1); @@ -229,18 +280,19 @@ public void testInt64() throws Exception { json.put("long", 1L); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test public void testInt32() throws Exception { + TestInt32 correctProto = TestInt32.newBuilder().setByte(1).setShort(1).setInt(1).build(); JSONObject json = new JSONObject(); json.put("byte", (byte) 1); json.put("short", (short) 1); json.put("int", 1); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json, false); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test @@ -259,12 +311,13 @@ public void testInt32NotMatchInt64() throws Exception { @Test public void testDouble() throws Exception { + TestDouble correctProto = TestDouble.newBuilder().setDouble(1.2).setFloat(3.4f).build(); JSONObject json = new JSONObject(); json.put("double", 1.2); json.put("float", 3.4f); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestDouble.getDescriptor(), json, false); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test @@ -275,7 +328,7 @@ public void testAllTypes() throws Exception { try { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json, false); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, AllTypesToCorrectProto.get(entry.getKey())[success]); success += 1; } catch (IllegalArgumentException e) { assertEquals( @@ -299,6 +352,7 @@ public void testAllRepeatedTypesWithLimits() throws Exception { try { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json, false); + assertEquals(protoMsg, AllRepeatedTypesToCorrectProto.get(entry.getKey())[success]); success += 1; } catch (IllegalArgumentException e) { assertEquals( @@ -319,23 +373,26 @@ public void testAllRepeatedTypesWithLimits() throws Exception { @Test public void testOptional() throws Exception { + TestInt64 correctProto = TestInt64.newBuilder().setByte(1).build(); JSONObject json = new JSONObject(); json.put("byte", 1); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test public void testRepeatedIsOptional() throws Exception { + TestRepeatedIsOptional correctProto = + TestRepeatedIsOptional.newBuilder().setRequiredDouble(1.1).build(); JSONObject json = new JSONObject(); json.put("required_double", 1.1); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage( TestRepeatedIsOptional.getDescriptor(), json, false); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test @@ -353,6 +410,10 @@ public void testRequired() throws Exception { @Test public void testStructSimple() throws Exception { + MessageType correctProto = + MessageType.newBuilder() + .setTestFieldType(StringType.newBuilder().setTestFieldType("test").build()) + .build(); JSONObject stringType = new JSONObject(); stringType.put("test_field_type", "test"); JSONObject json = new JSONObject(); @@ -360,7 +421,7 @@ public void testStructSimple() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json, false); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test @@ -381,6 +442,26 @@ public void testStructSimpleFail() throws Exception { @Test public void testStructComplex() throws Exception { + ComplexRoot correctProto = + ComplexRoot.newBuilder() + .setTestInt(1) + .addTestString("a") + .addTestString("b") + .addTestString("c") + .setTestBytes(ByteString.copyFrom("hello".getBytes())) + .setTestBool(true) + .addTestDouble(1.1) + .addTestDouble(2.2) + .addTestDouble(3.3) + .addTestDouble(4.4) + .setTestDate(1) + .setComplexLvl1( + ComplexLvl1.newBuilder() + .setTestInt(2) + .setComplexLvl2(ComplexLvl2.newBuilder().setTestInt(3).build()) + .build()) + .setComplexLvl2(ComplexLvl2.newBuilder().setTestInt(3).build()) + .build(); JSONObject complex_lvl2 = new JSONObject(); complex_lvl2.put("test_int", 3); @@ -400,7 +481,7 @@ public void testStructComplex() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json, false); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test @@ -446,6 +527,27 @@ public void testRepeatedWithMixedTypes() throws Exception { @Test public void testNestedRepeatedComplex() throws Exception { + NestedRepeated correctProto = + NestedRepeated.newBuilder() + .addDouble(1.1) + .addDouble(2.2) + .addDouble(3.3) + .addDouble(4.4) + .addDouble(5.5) + .addInt(1) + .addInt(2) + .addInt(3) + .addInt(4) + .addInt(5) + .setRepeatedString( + RepeatedString.newBuilder() + .addTestRepeated("hello") + .addTestRepeated("this") + .addTestRepeated("is") + .addTestRepeated("a") + .addTestRepeated("test") + .build()) + .build(); double[] doubleArr = {1.1, 2.2, 3.3, 4.4, 5.5}; String[] stringArr = {"hello", "this", "is", "a", "test"}; int[] intArr = {1, 2, 3, 4, 5}; @@ -459,7 +561,7 @@ public void testNestedRepeatedComplex() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json, false); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test @@ -487,17 +589,30 @@ public void testNestedRepeatedComplexFail() throws Exception { @Test public void testAllowUnknownFields() throws Exception { + RepeatedInt64 correctProto = + RepeatedInt64.newBuilder() + .addTestRepeated(1) + .addTestRepeated(2) + .addTestRepeated(3) + .addTestRepeated(4) + .addTestRepeated(5) + .build(); JSONObject json = new JSONObject(); json.put("test_repeated", new JSONArray(new int[] {1, 2, 3, 4, 5})); json.put("string", "hello"); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json, true); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test public void testEmptySecondLevelObject() throws Exception { + ComplexLvl1 correctProto = + ComplexLvl1.newBuilder() + .setTestInt(1) + .setComplexLvl2(ComplexLvl2.newBuilder().build()) + .build(); JSONObject complexLvl2 = new JSONObject(); JSONObject json = new JSONObject(); json.put("test_int", 1); @@ -505,7 +620,7 @@ public void testEmptySecondLevelObject() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, true); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test @@ -595,6 +710,11 @@ public void testTopLevelMismatch() throws Exception { @Test public void testTopLevelMatchSecondLevelMismatch() throws Exception { + ComplexLvl1 correctProto = + ComplexLvl1.newBuilder() + .setTestInt(1) + .setComplexLvl2(ComplexLvl2.newBuilder().build()) + .build(); JSONObject complex_lvl2 = new JSONObject(); complex_lvl2.put("no_match", 1); JSONObject json = new JSONObject(); @@ -603,7 +723,7 @@ public void testTopLevelMatchSecondLevelMismatch() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, true); - AreMatchingFieldsFilledIn(protoMsg, json); + assertEquals(protoMsg, correctProto); } @Test From 426b7fd82e184c17d15def8a51308cb465ad05c7 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Thu, 16 Jul 2020 17:21:36 -0500 Subject: [PATCH 17/21] Fix jsonTest.proto --- .../src/test/proto/jsonTest.proto | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto index 9f44996e27..8e3d1d9675 100644 --- a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto +++ b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto @@ -13,6 +13,16 @@ message ComplexRoot { required ComplexLvl2 complex_lvl2 = 8; } +message CasingComplex { + optional int64 test_int = 1; + repeated string test_string = 2; + required bytes test_bytes = 3; + optional bool test_bool = 4; + repeated double test_double = 5; + required int32 test_date = 6; + required OptionTest option_test = 7; +} + message ComplexLvl1 { optional int64 test_int = 1; required ComplexLvl2 complex_lvl2 = 2; From 5a7c6b40673d9ab66eefe7787886a80988e2b095 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Fri, 17 Jul 2020 16:03:25 -0500 Subject: [PATCH 18/21] Changed throwing empty JsonException to checking for the type of the values first. This prevents catching unexpected JsonExceptions and blocking the actual error message. --- .../storage/v1alpha2/JsonToProtoMessage.java | 227 ++++++++++-------- .../v1alpha2/JsonToProtoMessageTest.java | 68 +++--- 2 files changed, 162 insertions(+), 133 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java index eb6b863434..c6aced9f17 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java @@ -15,6 +15,7 @@ */ package com.google.cloud.bigquery.storage.v1alpha2; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; @@ -52,12 +53,10 @@ public class JsonToProtoMessage { public static DynamicMessage convertJsonToProtoMessage( Descriptor protoSchema, JSONObject json, boolean allowUnknownFields) throws IllegalArgumentException { - if (json == null) { - throw new IllegalArgumentException("JSONObject is null."); - } - if (json.length() == 0) { - throw new IllegalArgumentException("JSONObject is empty."); - } + Preconditions.checkNotNull(json, "JSONObject is null."); + Preconditions.checkNotNull(protoSchema, "Protobuf descriptor is null."); + Preconditions.checkState(json.length() != 0, "JSONObject is empty."); + return convertJsonToProtoMessageImpl( protoSchema, json, "root", /*topLevel=*/ true, allowUnknownFields); } @@ -151,47 +150,53 @@ private static void fillField( String currentScope, boolean allowUnknownFields) throws IllegalArgumentException { - java.lang.Object val; - try { - switch (fieldDescriptor.getType()) { - case BOOL: - protoMsg.setField(fieldDescriptor, new Boolean(json.getBoolean(exactJsonKeyName))); - break; - case BYTES: - protoMsg.setField(fieldDescriptor, json.getString(exactJsonKeyName).getBytes()); - break; - case INT64: - val = json.get(exactJsonKeyName); - if (val instanceof Integer) { - protoMsg.setField(fieldDescriptor, new Long((Integer) val)); - } else if (val instanceof Long) { - protoMsg.setField(fieldDescriptor, (Long) val); - } else { - throw new JSONException(""); - } - break; - case INT32: - val = json.get(exactJsonKeyName); - if (val instanceof Integer) { - protoMsg.setField(fieldDescriptor, (Integer) val); - } else { - throw new JSONException(""); - } - break; - case STRING: - protoMsg.setField(fieldDescriptor, json.getString(exactJsonKeyName)); - break; - case DOUBLE: - val = json.get(exactJsonKeyName); - if (val instanceof Double) { - protoMsg.setField(fieldDescriptor, (Double) val); - } else if (val instanceof Float) { - protoMsg.setField(fieldDescriptor, new Double((Float) val)); - } else { - throw new JSONException(""); - } - break; - case MESSAGE: + + java.lang.Object val = json.get(exactJsonKeyName); + switch (fieldDescriptor.getType()) { + case BOOL: + if (val instanceof Boolean) { + protoMsg.setField(fieldDescriptor, (Boolean) val); + return; + } + break; + case BYTES: + if (val instanceof String) { + protoMsg.setField(fieldDescriptor, ((String) val).getBytes()); + return; + } + break; + case INT64: + if (val instanceof Integer) { + protoMsg.setField(fieldDescriptor, new Long((Integer) val)); + return; + } else if (val instanceof Long) { + protoMsg.setField(fieldDescriptor, (Long) val); + return; + } + break; + case INT32: + if (val instanceof Integer) { + protoMsg.setField(fieldDescriptor, (Integer) val); + return; + } + break; + case STRING: + if (val instanceof String) { + protoMsg.setField(fieldDescriptor, (String) val); + return; + } + break; + case DOUBLE: + if (val instanceof Double) { + protoMsg.setField(fieldDescriptor, (Double) val); + return; + } else if (val instanceof Float) { + protoMsg.setField(fieldDescriptor, new Double((Float) val)); + return; + } + break; + case MESSAGE: + if (val instanceof JSONObject) { Message.Builder message = protoMsg.newBuilderForField(fieldDescriptor); protoMsg.setField( fieldDescriptor, @@ -201,14 +206,14 @@ private static void fillField( currentScope, /*topLevel =*/ false, allowUnknownFields)); - break; - } - } catch (JSONException e) { - throw new IllegalArgumentException( - String.format( - "JSONObject does not have a %s field at %s.", - FieldTypeToDebugMessage.get(fieldDescriptor.getType()), currentScope)); + return; + } + break; } + throw new IllegalArgumentException( + String.format( + "JSONObject does not have a %s field at %s.", + FieldTypeToDebugMessage.get(fieldDescriptor.getType()), currentScope)); } /** @@ -239,49 +244,60 @@ private static void fillRepeatedField( "JSONObject does not have a array field at " + currentScope + "."); } java.lang.Object val; - int index = -1; - try { - for (int i = 0; i < jsonArray.length(); i++) { - index = i; - switch (fieldDescriptor.getType()) { - case BOOL: - protoMsg.addRepeatedField(fieldDescriptor, new Boolean(jsonArray.getBoolean(i))); - break; - case BYTES: - protoMsg.addRepeatedField(fieldDescriptor, jsonArray.getString(i).getBytes()); - break; - case INT64: - val = jsonArray.get(i); - if (val instanceof Integer) { - protoMsg.addRepeatedField(fieldDescriptor, new Long((Integer) val)); - } else if (val instanceof Long) { - protoMsg.addRepeatedField(fieldDescriptor, (Long) val); - } else { - throw new JSONException(""); - } - break; - case INT32: - val = jsonArray.get(i); - if (val instanceof Integer) { - protoMsg.addRepeatedField(fieldDescriptor, (Integer) val); - } else { - throw new JSONException(""); - } - break; - case STRING: - protoMsg.addRepeatedField(fieldDescriptor, jsonArray.getString(i)); - break; - case DOUBLE: - val = jsonArray.get(i); - if (val instanceof Double) { - protoMsg.addRepeatedField(fieldDescriptor, (Double) val); - } else if (val instanceof Float) { - protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val)); - } else { - throw new JSONException(""); - } - break; - case MESSAGE: + int index; + boolean fail = false; + for (int i = 0; i < jsonArray.length(); i++) { + val = jsonArray.get(i); + index = i; + switch (fieldDescriptor.getType()) { + case BOOL: + if (val instanceof Boolean) { + protoMsg.addRepeatedField(fieldDescriptor, (Boolean) val); + } else { + fail = true; + } + break; + case BYTES: + if (val instanceof String) { + protoMsg.addRepeatedField(fieldDescriptor, ((String) val).getBytes()); + } else { + fail = true; + } + break; + case INT64: + if (val instanceof Integer) { + protoMsg.addRepeatedField(fieldDescriptor, new Long((Integer) val)); + } else if (val instanceof Long) { + protoMsg.addRepeatedField(fieldDescriptor, (Long) val); + } else { + fail = true; + } + break; + case INT32: + if (val instanceof Integer) { + protoMsg.addRepeatedField(fieldDescriptor, (Integer) val); + } else { + fail = true; + } + break; + case STRING: + if (val instanceof String) { + protoMsg.addRepeatedField(fieldDescriptor, (String) val); + } else { + fail = true; + } + break; + case DOUBLE: + if (val instanceof Double) { + protoMsg.addRepeatedField(fieldDescriptor, (Double) val); + } else if (val instanceof Float) { + protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val)); + } else { + fail = true; + } + break; + case MESSAGE: + if (val instanceof JSONObject) { Message.Builder message = protoMsg.newBuilderForField(fieldDescriptor); protoMsg.addRepeatedField( fieldDescriptor, @@ -291,14 +307,17 @@ private static void fillRepeatedField( currentScope, /*topLevel =*/ false, allowUnknownFields)); - break; - } + } else { + fail = true; + } + break; + } + if (fail) { + throw new IllegalArgumentException( + String.format( + "JSONObject does not have a %s field at %s[%d].", + FieldTypeToDebugMessage.get(fieldDescriptor.getType()), currentScope, index)); } - } catch (JSONException e) { - throw new IllegalArgumentException( - String.format( - "JSONObject does not have a %s field at %s[%d].", - FieldTypeToDebugMessage.get(fieldDescriptor.getType()), currentScope, index)); } } } diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index 9d226716b6..3ea1208a74 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -221,8 +221,8 @@ public class JsonToProtoMessageTest { "test_repeated", new JSONArray( new Integer[] { - (int) Integer.MAX_VALUE, - (int) Integer.MIN_VALUE, + Integer.MAX_VALUE, + Integer.MIN_VALUE, (int) Short.MAX_VALUE, (int) Short.MIN_VALUE, (int) Byte.MAX_VALUE, @@ -256,7 +256,7 @@ public class JsonToProtoMessageTest { @Test public void testDifferentNameCasing() throws Exception { - TestInt64 correctProto = + TestInt64 expectedProto = TestInt64.newBuilder().setByte(1).setShort(1).setInt(1).setLong(1).build(); JSONObject json = new JSONObject(); @@ -266,12 +266,12 @@ public void testDifferentNameCasing() throws Exception { json.put("lONg", 1L); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test public void testInt64() throws Exception { - TestInt64 correctProto = + TestInt64 expectedProto = TestInt64.newBuilder().setByte(1).setShort(1).setInt(1).setLong(1).build(); JSONObject json = new JSONObject(); json.put("byte", (byte) 1); @@ -280,19 +280,19 @@ public void testInt64() throws Exception { json.put("long", 1L); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test public void testInt32() throws Exception { - TestInt32 correctProto = TestInt32.newBuilder().setByte(1).setShort(1).setInt(1).build(); + TestInt32 expectedProto = TestInt32.newBuilder().setByte(1).setShort(1).setInt(1).build(); JSONObject json = new JSONObject(); json.put("byte", (byte) 1); json.put("short", (short) 1); json.put("int", 1); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json, false); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test @@ -311,13 +311,13 @@ public void testInt32NotMatchInt64() throws Exception { @Test public void testDouble() throws Exception { - TestDouble correctProto = TestDouble.newBuilder().setDouble(1.2).setFloat(3.4f).build(); + TestDouble expectedProto = TestDouble.newBuilder().setDouble(1.2).setFloat(3.4f).build(); JSONObject json = new JSONObject(); json.put("double", 1.2); json.put("float", 3.4f); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestDouble.getDescriptor(), json, false); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test @@ -373,18 +373,18 @@ public void testAllRepeatedTypesWithLimits() throws Exception { @Test public void testOptional() throws Exception { - TestInt64 correctProto = TestInt64.newBuilder().setByte(1).build(); + TestInt64 expectedProto = TestInt64.newBuilder().setByte(1).build(); JSONObject json = new JSONObject(); json.put("byte", 1); DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test public void testRepeatedIsOptional() throws Exception { - TestRepeatedIsOptional correctProto = + TestRepeatedIsOptional expectedProto = TestRepeatedIsOptional.newBuilder().setRequiredDouble(1.1).build(); JSONObject json = new JSONObject(); json.put("required_double", 1.1); @@ -392,7 +392,7 @@ public void testRepeatedIsOptional() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage( TestRepeatedIsOptional.getDescriptor(), json, false); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test @@ -410,7 +410,7 @@ public void testRequired() throws Exception { @Test public void testStructSimple() throws Exception { - MessageType correctProto = + MessageType expectedProto = MessageType.newBuilder() .setTestFieldType(StringType.newBuilder().setTestFieldType("test").build()) .build(); @@ -421,7 +421,7 @@ public void testStructSimple() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json, false); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test @@ -442,7 +442,7 @@ public void testStructSimpleFail() throws Exception { @Test public void testStructComplex() throws Exception { - ComplexRoot correctProto = + ComplexRoot expectedProto = ComplexRoot.newBuilder() .setTestInt(1) .addTestString("a") @@ -481,7 +481,7 @@ public void testStructComplex() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json, false); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test @@ -527,7 +527,7 @@ public void testRepeatedWithMixedTypes() throws Exception { @Test public void testNestedRepeatedComplex() throws Exception { - NestedRepeated correctProto = + NestedRepeated expectedProto = NestedRepeated.newBuilder() .addDouble(1.1) .addDouble(2.2) @@ -561,7 +561,7 @@ public void testNestedRepeatedComplex() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json, false); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test @@ -589,7 +589,7 @@ public void testNestedRepeatedComplexFail() throws Exception { @Test public void testAllowUnknownFields() throws Exception { - RepeatedInt64 correctProto = + RepeatedInt64 expectedProto = RepeatedInt64.newBuilder() .addTestRepeated(1) .addTestRepeated(2) @@ -603,12 +603,12 @@ public void testAllowUnknownFields() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json, true); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test public void testEmptySecondLevelObject() throws Exception { - ComplexLvl1 correctProto = + ComplexLvl1 expectedProto = ComplexLvl1.newBuilder() .setTestInt(1) .setComplexLvl2(ComplexLvl2.newBuilder().build()) @@ -620,7 +620,7 @@ public void testEmptySecondLevelObject() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, true); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test @@ -659,21 +659,31 @@ public void testEmptyJSONObject() throws Exception { try { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), json, false); - } catch (IllegalArgumentException e) { + } catch (IllegalStateException e) { assertEquals(e.getMessage(), "JSONObject is empty."); } } @Test - public void testNull() throws Exception { + public void testNullJson() throws Exception { try { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), null, false); - } catch (IllegalArgumentException e) { + } catch (NullPointerException e) { assertEquals(e.getMessage(), "JSONObject is null."); } } + @Test + public void testNullDescriptor() throws Exception { + try { + DynamicMessage protoMsg = + JsonToProtoMessage.convertJsonToProtoMessage(null, new JSONObject(), false); + } catch (NullPointerException e) { + assertEquals(e.getMessage(), "Protobuf descriptor is null."); + } + } + @Test public void testAllowUnknownFieldsSecondLevel() throws Exception { JSONObject complex_lvl2 = new JSONObject(); @@ -710,7 +720,7 @@ public void testTopLevelMismatch() throws Exception { @Test public void testTopLevelMatchSecondLevelMismatch() throws Exception { - ComplexLvl1 correctProto = + ComplexLvl1 expectedProto = ComplexLvl1.newBuilder() .setTestInt(1) .setComplexLvl2(ComplexLvl2.newBuilder().build()) @@ -723,7 +733,7 @@ public void testTopLevelMatchSecondLevelMismatch() throws Exception { DynamicMessage protoMsg = JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, true); - assertEquals(protoMsg, correctProto); + assertEquals(protoMsg, expectedProto); } @Test From 4966c5300ee6e6c70c435fd3449b7fbb9a77178a Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 21 Jul 2020 12:41:26 -0500 Subject: [PATCH 19/21] Fix by specifying version in parent pom only and remove * imports for JsonToProtoMessageTests --- google-cloud-bigquerystorage/pom.xml | 1 - .../bigquery/storage/v1alpha2/JsonToProtoMessageTest.java | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/google-cloud-bigquerystorage/pom.xml b/google-cloud-bigquerystorage/pom.xml index d8bddfabd3..f903b2bbd1 100644 --- a/google-cloud-bigquerystorage/pom.xml +++ b/google-cloud-bigquerystorage/pom.xml @@ -111,7 +111,6 @@ org.json json - 20200518 diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java index 3ea1208a74..7108367ea6 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java @@ -15,8 +15,7 @@ */ package com.google.cloud.bigquery.storage.v1alpha2; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; +import static org.junit.Assert.assertEquals; import com.google.cloud.bigquery.storage.test.JsonTest.*; import com.google.cloud.bigquery.storage.test.SchemaTest.*; From ec1c463c51ae46d250d28b1573db3abcdad8d472 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 21 Jul 2020 12:42:23 -0500 Subject: [PATCH 20/21] Add in parent pom --- pom.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pom.xml b/pom.xml index 7601a899c2..b1638886ef 100644 --- a/pom.xml +++ b/pom.xml @@ -155,6 +155,12 @@ commons-lang3 ${commons-lang3.version} + + org.json + json + 20200518 + + From 1065d26ee76d332ac39b55533ff36860f9ae1f1d Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 21 Jul 2020 12:43:26 -0500 Subject: [PATCH 21/21] Refactor parent pom --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index b1638886ef..d002e5673f 100644 --- a/pom.xml +++ b/pom.xml @@ -159,7 +159,7 @@ org.json json 20200518 - +