From 09881051a98f7d1675c3ec0850ef36dbe2ffa481 Mon Sep 17 00:00:00 2001 From: Stephanie Wang Date: Wed, 30 Dec 2020 15:52:08 -0500 Subject: [PATCH] feat: remove IgnoreUnknownFields support on JsonStreamWriter (#757) * feat! Remove IgnoreUnknownFields support for JsonStreamWriter * . * . * . * . * . * . * . * . * . * add clirr-ignored-differences.xml for breaking changes Co-authored-by: yirutang --- .../clirr-ignored-differences.xml | 26 +++ .../storage/v1beta2/JsonStreamWriter.java | 12 +- .../storage/v1beta2/JsonToProtoMessage.java | 48 ++--- .../storage/v1alpha2/StreamWriterTest.java | 2 + .../storage/v1beta2/JsonStreamWriterTest.java | 50 ++---- .../v1beta2/JsonToProtoMessageTest.java | 165 ++++++++---------- .../storage/v1beta2/StreamWriterTest.java | 2 + .../it/ITBigQueryWriteManualClientTest.java | 18 +- 8 files changed, 136 insertions(+), 187 deletions(-) create mode 100644 google-cloud-bigquerystorage/clirr-ignored-differences.xml diff --git a/google-cloud-bigquerystorage/clirr-ignored-differences.xml b/google-cloud-bigquerystorage/clirr-ignored-differences.xml new file mode 100644 index 0000000000..d8507fe060 --- /dev/null +++ b/google-cloud-bigquerystorage/clirr-ignored-differences.xml @@ -0,0 +1,26 @@ + + + + + + com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriter + 7005 + com.google.api.core.ApiFuture append(org.json.JSONArray, boolean) + com.google.api.core.ApiFuture append(org.json.JSONArray, boolean) + com.google.api.core.ApiFuture append(org.json.JSONArray, long) + + + com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriter + 7004 + com.google.api.core.ApiFuture append(org.json.JSONArray, long, boolean) + com.google.api.core.ApiFuture append(org.json.JSONArray, long, boolean) + com.google.api.core.ApiFuture append(org.json.JSONArray) + + + com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessage + 7004 + com.google.protobuf.DynamicMessage convertJsonToProtoMessage(com.google.protobuf.Descriptors$Descriptor, org.json.JSONObject, boolean) + com.google.protobuf.DynamicMessage convertJsonToProtoMessage(com.google.protobuf.Descriptors$Descriptor, org.json.JSONObject, boolean) + com.google.protobuf.DynamicMessage convertJsonToProtoMessage(com.google.protobuf.Descriptors$Descriptor, org.json.JSONObject) + + \ No newline at end of file diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriter.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriter.java index 89c417461e..1978c30f67 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriter.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriter.java @@ -94,12 +94,11 @@ private JsonStreamWriter(Builder builder) * schema update, the OnSchemaUpdateRunnable will be used to determine what actions to perform. * * @param jsonArr The JSON array that contains JSONObjects to be written - * @param allowUnknownFields if true, json data can have fields unknown to the BigQuery table. * @return ApiFuture returns an AppendRowsResponse message wrapped in an * ApiFuture */ - public ApiFuture append(JSONArray jsonArr, boolean allowUnknownFields) { - return append(jsonArr, -1, allowUnknownFields); + public ApiFuture append(JSONArray jsonArr) { + return append(jsonArr, -1); } /** @@ -109,20 +108,17 @@ public ApiFuture append(JSONArray jsonArr, boolean allowUnkn * * @param jsonArr The JSON array that contains JSONObjects to be written * @param offset Offset for deduplication - * @param allowUnknownFields if true, json data can have fields unknown to the BigQuery table. * @return ApiFuture returns an AppendRowsResponse message wrapped in an * ApiFuture */ - public ApiFuture append( - JSONArray jsonArr, long offset, boolean allowUnknownFields) { + public ApiFuture append(JSONArray jsonArr, long offset) { ProtoRows.Builder rowsBuilder = ProtoRows.newBuilder(); // Any error in convertJsonToProtoMessage will throw an // IllegalArgumentException/IllegalStateException/NullPointerException and will halt processing // of JSON data. for (int i = 0; i < jsonArr.length(); i++) { JSONObject json = jsonArr.getJSONObject(i); - Message protoMessage = - JsonToProtoMessage.convertJsonToProtoMessage(this.descriptor, json, allowUnknownFields); + Message protoMessage = JsonToProtoMessage.convertJsonToProtoMessage(this.descriptor, json); rowsBuilder.addSerializedRows(protoMessage.toByteString()); } AppendRowsRequest.ProtoData.Builder data = AppendRowsRequest.ProtoData.newBuilder(); diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessage.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessage.java index 8182e21176..a560495841 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessage.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessage.java @@ -47,18 +47,15 @@ public class JsonToProtoMessage { * * @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) + public static DynamicMessage convertJsonToProtoMessage(Descriptor protoSchema, JSONObject json) throws IllegalArgumentException { 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); + return convertJsonToProtoMessageImpl(protoSchema, json, "root", /*topLevel=*/ true); } /** @@ -67,16 +64,11 @@ public static DynamicMessage convertJsonToProtoMessage( * @param protoSchema * @param json * @param jsonScope Debugging purposes - * @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( - Descriptor protoSchema, - JSONObject json, - String jsonScope, - boolean topLevel, - boolean allowUnknownFields) + Descriptor protoSchema, JSONObject json, String jsonScope, boolean topLevel) throws IllegalArgumentException { DynamicMessage.Builder protoMsg = DynamicMessage.newBuilder(protoSchema); @@ -84,7 +76,6 @@ private static DynamicMessage convertJsonToProtoMessageImpl( if (jsonNames == null) { return protoMsg.build(); } - int matchedFields = 0; for (int i = 0; i < jsonNames.length; i++) { String jsonName = jsonNames[i]; // We want lowercase here to support case-insensitive data writes. @@ -93,27 +84,16 @@ private static DynamicMessage convertJsonToProtoMessageImpl( 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; - } + throw new IllegalArgumentException( + String.format("JSONObject has fields unknown to BigQuery: %s.", currentScope)); } - matchedFields++; if (!field.isRepeated()) { - fillField(protoMsg, field, json, jsonName, currentScope, allowUnknownFields); + fillField(protoMsg, field, json, jsonName, currentScope); } else { - fillRepeatedField(protoMsg, field, json, jsonName, currentScope, allowUnknownFields); + fillRepeatedField(protoMsg, field, json, jsonName, currentScope); } } - if (matchedFields == 0 && topLevel) { - throw new IllegalArgumentException( - "There are no matching fields found for the JSONObject and the protocol buffer descriptor."); - } DynamicMessage msg; try { msg = protoMsg.build(); @@ -139,7 +119,6 @@ private static DynamicMessage convertJsonToProtoMessageImpl( * @param json * @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. */ private static void fillField( @@ -147,8 +126,7 @@ private static void fillField( FieldDescriptor fieldDescriptor, JSONObject json, String exactJsonKeyName, - String currentScope, - boolean allowUnknownFields) + String currentScope) throws IllegalArgumentException { java.lang.Object val = json.get(exactJsonKeyName); @@ -204,8 +182,7 @@ private static void fillField( fieldDescriptor.getMessageType(), json.getJSONObject(exactJsonKeyName), currentScope, - /*topLevel =*/ false, - allowUnknownFields)); + /*topLevel =*/ false)); return; } break; @@ -224,7 +201,6 @@ private static void fillField( * @param json If root level has no matching fields, throws exception. * @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. */ private static void fillRepeatedField( @@ -232,8 +208,7 @@ private static void fillRepeatedField( FieldDescriptor fieldDescriptor, JSONObject json, String exactJsonKeyName, - String currentScope, - boolean allowUnknownFields) + String currentScope) throws IllegalArgumentException { JSONArray jsonArray; @@ -305,8 +280,7 @@ private static void fillRepeatedField( fieldDescriptor.getMessageType(), jsonArray.getJSONObject(i), currentScope, - /*topLevel =*/ false, - allowUnknownFields)); + /*topLevel =*/ false)); } else { fail = true; } diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriterTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriterTest.java index 778164f6b0..6a265c3910 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriterTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriterTest.java @@ -876,6 +876,8 @@ public void testFlushAllFailed() throws Exception { .build(); testBigQueryWrite.addException(Status.DATA_LOSS.asException()); + testBigQueryWrite.addException(Status.DATA_LOSS.asException()); + testBigQueryWrite.addException(Status.DATA_LOSS.asException()); ApiFuture appendFuture1 = sendTestMessage(writer, new String[] {"A"}); ApiFuture appendFuture2 = sendTestMessage(writer, new String[] {"B"}); diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriterTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriterTest.java index ebe712cc86..a64aaca01a 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriterTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonStreamWriterTest.java @@ -251,8 +251,7 @@ public void testSingleAppendSimpleJson() throws Exception { AppendRowsResponse.AppendResult.newBuilder().setOffset(Int64Value.of(0)).build()) .build()); - ApiFuture appendFuture = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture = writer.append(jsonArr); assertEquals(0L, appendFuture.get().getAppendResult().getOffset().getValue()); appendFuture.get(); assertEquals( @@ -299,8 +298,7 @@ public void testSingleAppendMultipleSimpleJson() throws Exception { AppendRowsResponse.AppendResult.newBuilder().setOffset(Int64Value.of(0)).build()) .build()); - ApiFuture appendFuture = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture = writer.append(jsonArr); assertEquals(0L, appendFuture.get().getAppendResult().getOffset().getValue()); appendFuture.get(); @@ -357,7 +355,7 @@ public void testMultipleAppendSimpleJson() throws Exception { .build()); ApiFuture appendFuture; for (int i = 0; i < 4; i++) { - appendFuture = writer.append(jsonArr, -1, /* allowUnknownFields */ false); + appendFuture = writer.append(jsonArr); assertEquals((long) i, appendFuture.get().getAppendResult().getOffset().getValue()); appendFuture.get(); assertEquals( @@ -443,8 +441,7 @@ public void testSingleAppendComplexJson() throws Exception { AppendRowsResponse.AppendResult.newBuilder().setOffset(Int64Value.of(0)).build()) .build()); - ApiFuture appendFuture = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture = writer.append(jsonArr); assertEquals(0L, appendFuture.get().getAppendResult().getOffset().getValue()); appendFuture.get(); @@ -495,8 +492,7 @@ public void testAppendMultipleSchemaUpdate() throws Exception { JSONArray jsonArr = new JSONArray(); jsonArr.put(foo); - ApiFuture appendFuture1 = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture1 = writer.append(jsonArr); int millis = 0; while (millis <= 10000) { @@ -532,8 +528,7 @@ public void testAppendMultipleSchemaUpdate() throws Exception { JSONArray updatedJsonArr = new JSONArray(); updatedJsonArr.put(updatedFoo); - ApiFuture appendFuture2 = - writer.append(updatedJsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture2 = writer.append(updatedJsonArr); millis = 0; while (millis <= 10000) { @@ -570,8 +565,7 @@ public void testAppendMultipleSchemaUpdate() throws Exception { JSONArray updatedJsonArr2 = new JSONArray(); updatedJsonArr2.put(updatedFoo2); - ApiFuture appendFuture3 = - writer.append(updatedJsonArr2, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture3 = writer.append(updatedJsonArr2); assertEquals(2L, appendFuture3.get().getAppendResult().getOffset().getValue()); assertEquals( @@ -614,8 +608,7 @@ public void testAppendOutOfRangeException() throws Exception { foo.put("foo", "allen"); JSONArray jsonArr = new JSONArray(); jsonArr.put(foo); - ApiFuture appendFuture = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture = writer.append(jsonArr); try { appendFuture.get(); Assert.fail("expected ExecutionException"); @@ -644,8 +637,7 @@ public void testAppendOutOfRangeAndUpdateSchema() throws Exception { foo.put("foo", "allen"); JSONArray jsonArr = new JSONArray(); jsonArr.put(foo); - ApiFuture appendFuture = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture = writer.append(jsonArr); try { appendFuture.get(); Assert.fail("expected ExecutionException"); @@ -668,8 +660,7 @@ public void testAppendOutOfRangeAndUpdateSchema() throws Exception { JSONArray updatedJsonArr = new JSONArray(); updatedJsonArr.put(updatedFoo); - ApiFuture appendFuture2 = - writer.append(updatedJsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture2 = writer.append(updatedJsonArr); assertEquals(0L, appendFuture2.get().getAppendResult().getOffset().getValue()); appendFuture2.get(); assertEquals( @@ -727,12 +718,9 @@ public void testSchemaUpdateWithNonemptyBatch() throws Exception { JSONArray jsonArr = new JSONArray(); jsonArr.put(foo); - ApiFuture appendFuture1 = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); - ApiFuture appendFuture2 = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); - ApiFuture appendFuture3 = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture1 = writer.append(jsonArr); + ApiFuture appendFuture2 = writer.append(jsonArr); + ApiFuture appendFuture3 = writer.append(jsonArr); assertEquals(0L, appendFuture1.get().getAppendResult().getOffset().getValue()); assertEquals(1L, appendFuture2.get().getAppendResult().getOffset().getValue()); @@ -796,8 +784,7 @@ public void testSchemaUpdateWithNonemptyBatch() throws Exception { JSONArray updatedJsonArr = new JSONArray(); updatedJsonArr.put(updatedFoo); - ApiFuture appendFuture4 = - writer.append(updatedJsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture4 = writer.append(updatedJsonArr); assertEquals(3L, appendFuture4.get().getAppendResult().getOffset().getValue()); assertEquals( @@ -857,8 +844,7 @@ public void testMultiThreadAppendNoSchemaUpdate() throws Exception { new Runnable() { public void run() { try { - ApiFuture appendFuture = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture = writer.append(jsonArr); AppendRowsResponse response = appendFuture.get(); offsetSets.remove(response.getAppendResult().getOffset().getValue()); } catch (Exception e) { @@ -940,8 +926,7 @@ public void testMultiThreadAppendWithSchemaUpdate() throws Exception { new Runnable() { public void run() { try { - ApiFuture appendFuture = - writer.append(jsonArr, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture = writer.append(jsonArr); AppendRowsResponse response = appendFuture.get(); offsetSets.remove(response.getAppendResult().getOffset().getValue()); } catch (Exception e) { @@ -1004,8 +989,7 @@ public void run() { new Runnable() { public void run() { try { - ApiFuture appendFuture = - writer.append(jsonArr2, -1, /* allowUnknownFields */ false); + ApiFuture appendFuture = writer.append(jsonArr2); AppendRowsResponse response = appendFuture.get(); offsetSets.remove(response.getAppendResult().getOffset().getValue()); } catch (Exception e) { diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessageTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessageTest.java index ec5a7490ba..16cf5be087 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessageTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessageTest.java @@ -28,6 +28,7 @@ import java.util.Map; import org.json.JSONArray; import org.json.JSONObject; +import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -264,8 +265,8 @@ public void testDifferentNameCasing() throws Exception { json.put("inT", 1); json.put("lONg", 1L); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); - assertEquals(protoMsg, expectedProto); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + assertEquals(expectedProto, protoMsg); } @Test @@ -278,8 +279,8 @@ public void testInt64() throws Exception { json.put("int", 1); json.put("long", 1L); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); - assertEquals(protoMsg, expectedProto); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + assertEquals(expectedProto, protoMsg); } @Test @@ -290,8 +291,8 @@ public void testInt32() throws Exception { json.put("short", (short) 1); json.put("int", 1); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json, false); - assertEquals(protoMsg, expectedProto); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json); + assertEquals(expectedProto, protoMsg); } @Test @@ -302,9 +303,10 @@ public void testInt32NotMatchInt64() throws Exception { json.put("int", 1L); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt32.getDescriptor(), json); + Assert.fail("should fail"); } catch (IllegalArgumentException e) { - assertEquals(e.getMessage(), "JSONObject does not have a int32 field at root.int."); + assertEquals("JSONObject does not have a int32 field at root.int.", e.getMessage()); } } @@ -315,8 +317,8 @@ public void testDouble() throws Exception { json.put("double", 1.2); json.put("float", 3.4f); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestDouble.getDescriptor(), json, false); - assertEquals(protoMsg, expectedProto); + JsonToProtoMessage.convertJsonToProtoMessage(TestDouble.getDescriptor(), json); + assertEquals(expectedProto, protoMsg); } @Test @@ -326,13 +328,13 @@ public void testAllTypes() throws Exception { for (JSONObject json : simpleJSONObjects) { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json); assertEquals(protoMsg, AllTypesToCorrectProto.get(entry.getKey())[success]); success += 1; } catch (IllegalArgumentException e) { assertEquals( - e.getMessage(), - "JSONObject does not have a " + entry.getValue() + " field at root.test_field_type."); + "JSONObject does not have a " + entry.getValue() + " field at root.test_field_type.", + e.getMessage()); } } if (entry.getKey() == Int64Type.getDescriptor()) { @@ -350,15 +352,13 @@ public void testAllRepeatedTypesWithLimits() throws Exception { for (JSONObject json : simpleJSONArrays) { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(entry.getKey(), json); assertEquals(protoMsg, AllRepeatedTypesToCorrectProto.get(entry.getKey())[success]); success += 1; } catch (IllegalArgumentException e) { assertEquals( - e.getMessage(), - "JSONObject does not have a " - + entry.getValue() - + " field at root.test_repeated[0]."); + "JSONObject does not have a " + entry.getValue() + " field at root.test_repeated[0].", + e.getMessage()); } } if (entry.getKey() == RepeatedInt64.getDescriptor() @@ -377,8 +377,8 @@ public void testOptional() throws Exception { json.put("byte", 1); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); - assertEquals(protoMsg, expectedProto); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + assertEquals(expectedProto, protoMsg); } @Test @@ -389,9 +389,8 @@ public void testRepeatedIsOptional() throws Exception { json.put("required_double", 1.1); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage( - TestRepeatedIsOptional.getDescriptor(), json, false); - assertEquals(protoMsg, expectedProto); + JsonToProtoMessage.convertJsonToProtoMessage(TestRepeatedIsOptional.getDescriptor(), json); + assertEquals(expectedProto, protoMsg); } @Test @@ -400,10 +399,11 @@ public void testRequired() throws Exception { json.put("optional_double", 1.1); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestRequired.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(TestRequired.getDescriptor(), json); + Assert.fail("should fail"); } catch (IllegalArgumentException e) { assertEquals( - e.getMessage(), "JSONObject does not have the required field root.required_double."); + "JSONObject does not have the required field root.required_double.", e.getMessage()); } } @@ -419,8 +419,8 @@ public void testStructSimple() throws Exception { json.put("test_field_type", stringType); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json, false); - assertEquals(protoMsg, expectedProto); + JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json); + assertEquals(expectedProto, protoMsg); } @Test @@ -431,11 +431,12 @@ public void testStructSimpleFail() throws Exception { json.put("test_field_type", stringType); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(MessageType.getDescriptor(), json); + Assert.fail("should fail"); } catch (IllegalArgumentException e) { assertEquals( - e.getMessage(), - "JSONObject does not have a string field at root.test_field_type.test_field_type."); + "JSONObject does not have a string field at root.test_field_type.test_field_type.", + e.getMessage()); } } @@ -479,8 +480,8 @@ public void testStructComplex() throws Exception { json.put("complex_lvl2", complex_lvl2); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json, false); - assertEquals(protoMsg, expectedProto); + JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json); + assertEquals(expectedProto, protoMsg); } @Test @@ -504,10 +505,11 @@ public void testStructComplexFail() throws Exception { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(ComplexRoot.getDescriptor(), json); + Assert.fail("should fail"); } catch (IllegalArgumentException e) { assertEquals( - e.getMessage(), "JSONObject does not have a int64 field at root.complex_lvl1.test_int."); + "JSONObject does not have a int64 field at root.complex_lvl1.test_int.", e.getMessage()); } } @@ -517,10 +519,11 @@ public void testRepeatedWithMixedTypes() throws Exception { json.put("test_repeated", new JSONArray("[1.1, 2.2, true]")); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(RepeatedDouble.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(RepeatedDouble.getDescriptor(), json); + Assert.fail("should fail"); } catch (IllegalArgumentException e) { assertEquals( - e.getMessage(), "JSONObject does not have a double field at root.test_repeated[2]."); + "JSONObject does not have a double field at root.test_repeated[2].", e.getMessage()); } } @@ -559,7 +562,7 @@ public void testNestedRepeatedComplex() throws Exception { json.put("repeated_string", jsonRepeatedString); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json); assertEquals(protoMsg, expectedProto); } @@ -578,33 +581,15 @@ public void testNestedRepeatedComplexFail() throws Exception { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(NestedRepeated.getDescriptor(), json); + Assert.fail("should fail"); } catch (IllegalArgumentException e) { assertEquals( - e.getMessage(), - "JSONObject does not have a string field at root.repeated_string.test_repeated[0]."); + "JSONObject does not have a string field at root.repeated_string.test_repeated[0].", + e.getMessage()); } } - @Test - public void testAllowUnknownFields() throws Exception { - RepeatedInt64 expectedProto = - 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); - assertEquals(protoMsg, expectedProto); - } - @Test public void testEmptySecondLevelObject() throws Exception { ComplexLvl1 expectedProto = @@ -618,8 +603,8 @@ public void testEmptySecondLevelObject() throws Exception { json.put("complex_lvl2", complexLvl2); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, true); - assertEquals(protoMsg, expectedProto); + JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json); + assertEquals(expectedProto, protoMsg); } @Test @@ -630,11 +615,10 @@ public void testAllowUnknownFieldsError() throws Exception { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json); + Assert.fail("Should fail"); } catch (IllegalArgumentException e) { - assertEquals( - e.getMessage(), - "JSONObject has fields unknown to BigQuery: root.string. Set allowUnknownFields to True to allow unknown fields."); + assertEquals("JSONObject has fields unknown to BigQuery: root.string.", e.getMessage()); } } @@ -642,13 +626,13 @@ public void testAllowUnknownFieldsError() throws Exception { 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); + JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json); + Assert.fail("Should fail"); } catch (IllegalArgumentException e) { - assertEquals(e.getMessage(), "The created protobuf message is empty."); + assertEquals("The created protobuf message is empty.", e.getMessage()); } } @@ -657,9 +641,10 @@ public void testEmptyJSONObject() throws Exception { JSONObject json = new JSONObject(); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), json); + Assert.fail("Should fail"); } catch (IllegalStateException e) { - assertEquals(e.getMessage(), "JSONObject is empty."); + assertEquals("JSONObject is empty.", e.getMessage()); } } @@ -667,9 +652,10 @@ public void testEmptyJSONObject() throws Exception { public void testNullJson() throws Exception { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), null, false); + JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), null); + Assert.fail("Should fail"); } catch (NullPointerException e) { - assertEquals(e.getMessage(), "JSONObject is null."); + assertEquals("JSONObject is null.", e.getMessage()); } } @@ -677,9 +663,10 @@ public void testNullJson() throws Exception { public void testNullDescriptor() throws Exception { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(null, new JSONObject(), false); + JsonToProtoMessage.convertJsonToProtoMessage(null, new JSONObject()); + Assert.fail("Should fail"); } catch (NullPointerException e) { - assertEquals(e.getMessage(), "Protobuf descriptor is null."); + assertEquals("Protobuf descriptor is null.", e.getMessage()); } } @@ -693,27 +680,11 @@ public void testAllowUnknownFieldsSecondLevel() throws Exception { try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, false); - } catch (IllegalArgumentException e) { - assertEquals( - e.getMessage(), - "JSONObject has fields unknown to BigQuery: root.complex_lvl2.no_match. Set allowUnknownFields to True to allow unknown fields."); - } - } - - @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); + JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json); + Assert.fail("Should fail"); } catch (IllegalArgumentException e) { assertEquals( - e.getMessage(), - "There are no matching fields found for the JSONObject and the protocol buffer descriptor."); + "JSONObject has fields unknown to BigQuery: root.complex_lvl2.no_match.", e.getMessage()); } } @@ -725,14 +696,13 @@ public void testTopLevelMatchSecondLevelMismatch() throws Exception { .setComplexLvl2(ComplexLvl2.newBuilder().build()) .build(); JSONObject complex_lvl2 = new JSONObject(); - complex_lvl2.put("no_match", 1); JSONObject json = new JSONObject(); json.put("test_int", 1); json.put("complex_lvl2", complex_lvl2); DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, true); - assertEquals(protoMsg, expectedProto); + JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json); + assertEquals(expectedProto, protoMsg); } @Test @@ -742,7 +712,8 @@ public void testJsonNullValue() throws Exception { json.put("int", 1); try { DynamicMessage protoMsg = - JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json, false); + JsonToProtoMessage.convertJsonToProtoMessage(TestInt64.getDescriptor(), json); + Assert.fail("Should fail"); } catch (IllegalArgumentException e) { assertEquals(e.getMessage(), "JSONObject does not have a int64 field at root.long."); } diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/StreamWriterTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/StreamWriterTest.java index 6e89ef5dd8..b93bdcfbcc 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/StreamWriterTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/StreamWriterTest.java @@ -958,6 +958,8 @@ public void testFlushAllFailed() throws Exception { .build(); testBigQueryWrite.addException(Status.DATA_LOSS.asException()); + testBigQueryWrite.addException(Status.DATA_LOSS.asException()); + testBigQueryWrite.addException(Status.DATA_LOSS.asException()); ApiFuture appendFuture1 = sendTestMessage(writer, new String[] {"A"}); ApiFuture appendFuture2 = sendTestMessage(writer, new String[] {"B"}); diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/it/ITBigQueryWriteManualClientTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/it/ITBigQueryWriteManualClientTest.java index 17fce83fa4..6986c6f981 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/it/ITBigQueryWriteManualClientTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/it/ITBigQueryWriteManualClientTest.java @@ -251,8 +251,7 @@ public void testJsonStreamWriterBatchWriteWithCommittedStream() row1.put("test_datetime", "2020-10-1 12:00:00"); JSONArray jsonArr1 = new JSONArray(new JSONObject[] {row1}); - ApiFuture response1 = - jsonStreamWriter.append(jsonArr1, -1, /* allowUnknownFields */ false); + ApiFuture response1 = jsonStreamWriter.append(jsonArr1, -1); assertEquals(0, response1.get().getAppendResult().getOffset().getValue()); @@ -270,11 +269,9 @@ public void testJsonStreamWriterBatchWriteWithCommittedStream() jsonArr3.put(row4); LOG.info("Sending two more messages"); - ApiFuture response2 = - jsonStreamWriter.append(jsonArr2, -1, /* allowUnknownFields */ false); + ApiFuture response2 = jsonStreamWriter.append(jsonArr2, -1); LOG.info("Sending one more message"); - ApiFuture response3 = - jsonStreamWriter.append(jsonArr3, -1, /* allowUnknownFields */ false); + ApiFuture response3 = jsonStreamWriter.append(jsonArr3, -1); assertEquals(1, response2.get().getAppendResult().getOffset().getValue()); assertEquals(3, response3.get().getAppendResult().getOffset().getValue()); @@ -331,8 +328,7 @@ public void testJsonStreamWriterSchemaUpdate() JSONArray jsonArr = new JSONArray(); jsonArr.put(foo); - ApiFuture response = - jsonStreamWriter.append(jsonArr, -1, /* allowUnknownFields */ false); + ApiFuture response = jsonStreamWriter.append(jsonArr, -1); assertEquals(0, response.get().getAppendResult().getOffset().getValue()); // 2). Schema update and wait until querying it returns a new schema. try { @@ -375,8 +371,7 @@ public void testJsonStreamWriterSchemaUpdate() int next = 0; for (int i = 1; i < 100; i++) { - ApiFuture response2 = - jsonStreamWriter.append(jsonArr2, -1, /* allowUnknownFields */ false); + ApiFuture response2 = jsonStreamWriter.append(jsonArr2, -1); assertEquals(i, response2.get().getAppendResult().getOffset().getValue()); if (response2.get().hasUpdatedSchema()) { next = i; @@ -403,8 +398,7 @@ public void testJsonStreamWriterSchemaUpdate() JSONArray updatedJsonArr = new JSONArray(); updatedJsonArr.put(updatedFoo); for (int i = 0; i < 10; i++) { - ApiFuture response3 = - jsonStreamWriter.append(updatedJsonArr, -1, /* allowUnknownFields */ false); + ApiFuture response3 = jsonStreamWriter.append(updatedJsonArr, -1); assertEquals(next + 1 + i, response3.get().getAppendResult().getOffset().getValue()); response3.get(); }