From 3583f119870ce06c43d49beb6eaecb76c4b21fc4 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Wed, 1 Jul 2020 17:17:12 -0500 Subject: [PATCH 01/13] feat: Added BQSchemaToProtoSchema functionality along with test cases; checked for linting --- google-cloud-bigquerystorage/pom.xml | 6 + .../v1alpha2/JsonToProtoConverter.java | 150 ++++++++++++++++++ .../v1alpha2/JsonToProtoConverterTest.java | 105 ++++++++++++ 3 files changed, 261 insertions(+) create mode 100644 google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java create mode 100644 google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java diff --git a/google-cloud-bigquerystorage/pom.xml b/google-cloud-bigquerystorage/pom.xml index ce7261c6ad..6b97167bfa 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/JsonToProtoConverter.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java new file mode 100644 index 0000000000..9e1c9f618c --- /dev/null +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java @@ -0,0 +1,150 @@ +/* + * 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.common.collect.ImmutableMap; +import com.google.protobuf.DescriptorProtos.DescriptorProto; +import com.google.protobuf.DescriptorProtos.FieldDescriptorProto; +import com.google.protobuf.DescriptorProtos.FileDescriptorProto; +import com.google.protobuf.Descriptors; +import com.google.protobuf.Descriptors.Descriptor; +import com.google.protobuf.Descriptors.FileDescriptor; +import java.util.ArrayList; +import java.util.List; + +/** + * A class that checks the schema compatibility between user schema in proto descriptor and Bigquery + * table schema. If this check is passed, then user can write to BigQuery table using the user + * schema, otherwise the write will fail. + * + *

The implementation as of now is not complete, which measn, if this check passed, there is + * still a possbility of writing will fail. + */ +public class JsonToProtoConverter { + private static ImmutableMap + BQTableSchemaModeMap = + ImmutableMap.of( + Table.TableFieldSchema.Mode.NULLABLE, FieldDescriptorProto.Label.LABEL_OPTIONAL, + Table.TableFieldSchema.Mode.REPEATED, FieldDescriptorProto.Label.LABEL_REPEATED, + Table.TableFieldSchema.Mode.REQUIRED, FieldDescriptorProto.Label.LABEL_REQUIRED); + + private static ImmutableMap + BQTableSchemaTypeMap = + new ImmutableMap.Builder() + .put(Table.TableFieldSchema.Type.BOOL, FieldDescriptorProto.Type.TYPE_BOOL) + .put(Table.TableFieldSchema.Type.BYTES, FieldDescriptorProto.Type.TYPE_BYTES) + .put(Table.TableFieldSchema.Type.DATE, FieldDescriptorProto.Type.TYPE_INT64) + .put(Table.TableFieldSchema.Type.DATETIME, FieldDescriptorProto.Type.TYPE_INT64) + .put(Table.TableFieldSchema.Type.DOUBLE, FieldDescriptorProto.Type.TYPE_DOUBLE) + .put(Table.TableFieldSchema.Type.GEOGRAPHY, FieldDescriptorProto.Type.TYPE_BYTES) + .put(Table.TableFieldSchema.Type.INT64, FieldDescriptorProto.Type.TYPE_INT64) + .put(Table.TableFieldSchema.Type.NUMERIC, FieldDescriptorProto.Type.TYPE_DOUBLE) + .put(Table.TableFieldSchema.Type.STRING, FieldDescriptorProto.Type.TYPE_STRING) + .put(Table.TableFieldSchema.Type.STRUCT, FieldDescriptorProto.Type.TYPE_MESSAGE) + .put(Table.TableFieldSchema.Type.TIME, FieldDescriptorProto.Type.TYPE_INT64) + .put(Table.TableFieldSchema.Type.TIMESTAMP, FieldDescriptorProto.Type.TYPE_INT64) + .build(); + + /** + * Converts Table.TableSchema to a Descriptors.Descriptor object. + * + * @param BQTableSchema + * @throws Descriptors.DescriptorValidationException + */ + public static Descriptor BQTableSchemaToProtoSchema(Table.TableSchema BQTableSchema) + throws Descriptors.DescriptorValidationException { + Descriptor descriptor = BQTableSchemaToProtoSchemaImpl(BQTableSchema, "root"); + return descriptor; + } + + /** + * Implementation that converts a Table.TableSchema to a Descriptors.Descriptor object. + * + * @param BQTableSchema + * @param scope Keeps track of current scope to prevent repeated naming while constructing + * descriptor. + * @throws Descriptors.DescriptorValidationException + */ + private static Descriptor BQTableSchemaToProtoSchemaImpl( + Table.TableSchema BQTableSchema, String scope) + throws Descriptors.DescriptorValidationException { + List dependenciesList = new ArrayList(); + List fields = new ArrayList(); + int index = 1; + for (Table.TableFieldSchema BQTableField : BQTableSchema.getFieldsList()) { + if (BQTableField.getType() == Table.TableFieldSchema.Type.STRUCT) { + String currentScope = scope + BQTableField.getName(); + dependenciesList.add( + BQTableSchemaToProtoSchemaImpl( + Table.TableSchema.newBuilder() + .addAllFields(BQTableField.getFieldsList()) + .build(), + currentScope) + .getFile()); + fields.add(BQStructToProtoMessage(BQTableField, index++, currentScope)); + } else { + fields.add(BQTableFieldToProtoField(BQTableField, index++)); + } + } + FileDescriptor[] dependenciesArray = new FileDescriptor[dependenciesList.size()]; + dependenciesArray = dependenciesList.toArray(dependenciesArray); + DescriptorProto descriptorProto = + DescriptorProto.newBuilder().setName(scope).addAllField(fields).build(); + FileDescriptorProto fileDescriptorProto = + FileDescriptorProto.newBuilder().addMessageType(descriptorProto).build(); + FileDescriptor fileDescriptor = + FileDescriptor.buildFrom(fileDescriptorProto, dependenciesArray); + Descriptor descriptor = fileDescriptor.findMessageTypeByName(scope); + return descriptor; + } + + /** + * Constructs a FieldDescriptorProto for non-struct BQ fields. + * + * @param BQTableField BQ Field used to construct a FieldDescriptorProto + * @param index Index for protobuf fields. + */ + private static FieldDescriptorProto BQTableFieldToProtoField( + Table.TableFieldSchema BQTableField, int index) { + String fieldName = BQTableField.getName(); + Table.TableFieldSchema.Mode mode = BQTableField.getMode(); + return FieldDescriptorProto.newBuilder() + .setName(fieldName) + .setType((FieldDescriptorProto.Type) BQTableSchemaTypeMap.get(BQTableField.getType())) + .setLabel((FieldDescriptorProto.Label) BQTableSchemaModeMap.get(mode)) + .setNumber(index) + .build(); + } + + /** + * Constructs a FieldDescriptorProto for a Struct type BQ field. + * + * @param BQTableField BQ Field used to construct a FieldDescriptorProto + * @param index Index for protobuf fields. + * @param scope Need scope to prevent naming issues + */ + private static FieldDescriptorProto BQStructToProtoMessage( + Table.TableFieldSchema BQTableField, int index, String scope) { + String fieldName = BQTableField.getName(); + Table.TableFieldSchema.Mode mode = BQTableField.getMode(); + return FieldDescriptorProto.newBuilder() + .setName(fieldName) + .setTypeName(scope) + .setLabel((FieldDescriptorProto.Label) BQTableSchemaModeMap.get(mode)) + .setNumber(index) + .build(); + } +} diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java new file mode 100644 index 0000000000..35552202b4 --- /dev/null +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java @@ -0,0 +1,105 @@ +/* + * 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 java.util.Map; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class JsonToProtoConverterTest { + private static ImmutableMap + BQTableTypeToProtoDescriptor = + new ImmutableMap.Builder() + .put(Table.TableFieldSchema.Type.BOOL, BoolType.getDescriptor()) + .put(Table.TableFieldSchema.Type.BYTES, BytesType.getDescriptor()) + .put(Table.TableFieldSchema.Type.DATE, Int64Type.getDescriptor()) + .put(Table.TableFieldSchema.Type.DATETIME, Int64Type.getDescriptor()) + .put(Table.TableFieldSchema.Type.DOUBLE, DoubleType.getDescriptor()) + .put(Table.TableFieldSchema.Type.GEOGRAPHY, BytesType.getDescriptor()) + .put(Table.TableFieldSchema.Type.INT64, Int64Type.getDescriptor()) + .put(Table.TableFieldSchema.Type.NUMERIC, DoubleType.getDescriptor()) + .put(Table.TableFieldSchema.Type.STRING, StringType.getDescriptor()) + .put(Table.TableFieldSchema.Type.TIME, Int64Type.getDescriptor()) + .put(Table.TableFieldSchema.Type.TIMESTAMP, Int64Type.getDescriptor()) + .build(); + + private boolean isDescriptorEqual(Descriptor convertedProto, Descriptor originalProto) { + for (FieldDescriptor convertedField : convertedProto.getFields()) { + FieldDescriptor originalField = originalProto.findFieldByName(convertedField.getName()); + if (originalField == null) { + return false; + } + FieldDescriptor.Type convertedType = convertedField.getType(); + FieldDescriptor.Type originalType = originalField.getType(); + if (convertedType != originalType) { + return false; + } + if (convertedType == FieldDescriptor.Type.MESSAGE) { + if (!isDescriptorEqual(convertedField.getMessageType(), originalField.getMessageType())) { + return false; + } + } + } + return true; + } + + @Test + public void testBQTableSchemaToProtoDescriptorSimpleTypes() throws Exception { + for (Map.Entry entry : + BQTableTypeToProtoDescriptor.entrySet()) { + Table.TableFieldSchema tableFieldSchema = + Table.TableFieldSchema.newBuilder() + .setType(entry.getKey()) + .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .setName("test_field_type") + .build(); + Table.TableSchema tableSchema = + Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); + Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + assertTrue(isDescriptorEqual(descriptor, entry.getValue())); + } + } + + @Test + public void testBQTableSchemaToProtoDescriptorComplex() 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.TableSchema tableSchema = + Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); + Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); + assertTrue(isDescriptorEqual(descriptor, MessageType.getDescriptor())); + } +} From 57cb6cfcd381a35ca9e1b36f914550edfb418905 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Wed, 1 Jul 2020 17:44:44 -0500 Subject: [PATCH 02/13] Updated BQ Numeric mapping from proto double to proto string to prevent data loss --- .../cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java | 2 +- .../bigquery/storage/v1alpha2/JsonToProtoConverterTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java index 9e1c9f618c..c146eaeaec 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java @@ -51,7 +51,7 @@ public class JsonToProtoConverter { .put(Table.TableFieldSchema.Type.DOUBLE, FieldDescriptorProto.Type.TYPE_DOUBLE) .put(Table.TableFieldSchema.Type.GEOGRAPHY, FieldDescriptorProto.Type.TYPE_BYTES) .put(Table.TableFieldSchema.Type.INT64, FieldDescriptorProto.Type.TYPE_INT64) - .put(Table.TableFieldSchema.Type.NUMERIC, FieldDescriptorProto.Type.TYPE_DOUBLE) + .put(Table.TableFieldSchema.Type.NUMERIC, FieldDescriptorProto.Type.TYPE_STRING) .put(Table.TableFieldSchema.Type.STRING, FieldDescriptorProto.Type.TYPE_STRING) .put(Table.TableFieldSchema.Type.STRUCT, FieldDescriptorProto.Type.TYPE_MESSAGE) .put(Table.TableFieldSchema.Type.TIME, FieldDescriptorProto.Type.TYPE_INT64) diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java index 35552202b4..3e67ca135f 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java @@ -39,7 +39,7 @@ public class JsonToProtoConverterTest { .put(Table.TableFieldSchema.Type.DOUBLE, DoubleType.getDescriptor()) .put(Table.TableFieldSchema.Type.GEOGRAPHY, BytesType.getDescriptor()) .put(Table.TableFieldSchema.Type.INT64, Int64Type.getDescriptor()) - .put(Table.TableFieldSchema.Type.NUMERIC, DoubleType.getDescriptor()) + .put(Table.TableFieldSchema.Type.NUMERIC, StringType.getDescriptor()) .put(Table.TableFieldSchema.Type.STRING, StringType.getDescriptor()) .put(Table.TableFieldSchema.Type.TIME, Int64Type.getDescriptor()) .put(Table.TableFieldSchema.Type.TIMESTAMP, Int64Type.getDescriptor()) From cbe007fad552fce3e7c2527f2d45393cd576340f Mon Sep 17 00:00:00 2001 From: allenc3 Date: Wed, 1 Jul 2020 17:48:06 -0500 Subject: [PATCH 03/13] Updated BQ Numeric mapping from proto string to proto bytes to match backend mapping --- .../cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java | 2 +- .../bigquery/storage/v1alpha2/JsonToProtoConverterTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java index c146eaeaec..7a31421245 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java @@ -51,7 +51,7 @@ public class JsonToProtoConverter { .put(Table.TableFieldSchema.Type.DOUBLE, FieldDescriptorProto.Type.TYPE_DOUBLE) .put(Table.TableFieldSchema.Type.GEOGRAPHY, FieldDescriptorProto.Type.TYPE_BYTES) .put(Table.TableFieldSchema.Type.INT64, FieldDescriptorProto.Type.TYPE_INT64) - .put(Table.TableFieldSchema.Type.NUMERIC, FieldDescriptorProto.Type.TYPE_STRING) + .put(Table.TableFieldSchema.Type.NUMERIC, FieldDescriptorProto.Type.TYPE_BYTES) .put(Table.TableFieldSchema.Type.STRING, FieldDescriptorProto.Type.TYPE_STRING) .put(Table.TableFieldSchema.Type.STRUCT, FieldDescriptorProto.Type.TYPE_MESSAGE) .put(Table.TableFieldSchema.Type.TIME, FieldDescriptorProto.Type.TYPE_INT64) diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java index 3e67ca135f..c857a0e4b4 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java @@ -39,7 +39,7 @@ public class JsonToProtoConverterTest { .put(Table.TableFieldSchema.Type.DOUBLE, DoubleType.getDescriptor()) .put(Table.TableFieldSchema.Type.GEOGRAPHY, BytesType.getDescriptor()) .put(Table.TableFieldSchema.Type.INT64, Int64Type.getDescriptor()) - .put(Table.TableFieldSchema.Type.NUMERIC, StringType.getDescriptor()) + .put(Table.TableFieldSchema.Type.NUMERIC, BytesType.getDescriptor()) .put(Table.TableFieldSchema.Type.STRING, StringType.getDescriptor()) .put(Table.TableFieldSchema.Type.TIME, Int64Type.getDescriptor()) .put(Table.TableFieldSchema.Type.TIMESTAMP, Int64Type.getDescriptor()) From 1685876ed4ca34c2aac4ed15fc3111e1a7f773c0 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Thu, 2 Jul 2020 15:35:43 -0500 Subject: [PATCH 04/13] Revert pom.xml --- google-cloud-bigquerystorage/pom.xml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/google-cloud-bigquerystorage/pom.xml b/google-cloud-bigquerystorage/pom.xml index 6b97167bfa..ce7261c6ad 100644 --- a/google-cloud-bigquerystorage/pom.xml +++ b/google-cloud-bigquerystorage/pom.xml @@ -108,12 +108,6 @@ org.apache.commons commons-lang3 - - org.json - json - 20200518 - - From d3a621c26d40f9cae5ff426eeda3f953548686dc Mon Sep 17 00:00:00 2001 From: allenc3 Date: Mon, 6 Jul 2020 16:37:23 -0500 Subject: [PATCH 05/13] Fix according to PR conversations --- .../v1alpha2/JsonToProtoConverter.java | 45 ++++++--- .../v1alpha2/JsonToProtoConverterTest.java | 95 +++++++++++++++---- .../src/test/proto/jsonTest.proto | 24 +++++ 3 files changed, 132 insertions(+), 32 deletions(-) create mode 100644 google-cloud-bigquerystorage/src/test/proto/jsonTest.proto diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java index 7a31421245..3a82ed9a79 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java @@ -26,12 +26,30 @@ import java.util.List; /** - * A class that checks the schema compatibility between user schema in proto descriptor and Bigquery - * table schema. If this check is passed, then user can write to BigQuery table using the user - * schema, otherwise the write will fail. + * This class can convert Json data to protobuf messages given a protobuf descriptor. + * The data types will be mapped as shown in the table below. + * Some rules to follow: + * - If field is required in protobuf, then it must be present in the json data + * - If field is optional in protobuf, then it is optional in the json data. + * - The casing must match between protobuf field names and json key names. + * - If there are more json fields than protobuf fields, the allowUnknownFields flag must be set to true. + * - There can be more fields in protobuf fields as long as they are optional. * - *

The implementation as of now is not complete, which measn, if this check passed, there is - * still a possbility of writing will fail. + * This class also provides a converter from a BQ table schema to protobuf descriptor. + * It will follow the following mapping: + * BQ Type -> Protobuf Type -> Json Data Type + * BOOL TYPE_BOOL Boolean + * BYTES TYPE_BYTES String + * DATE TYPE_INT64 Number [byte, short, int, long] + * DATETIME TYPE_INT64 Number [byte, short, int, long] + * DOUBLE TYPE_DOUBLE Number [float, double] + * GEOGRAPHY TYPE_BYTES String + * INT64 TYPE_INT64 Number [byte, short, int, long] + * NUMERIC TYPE_BYTES String + * STRING TYPE_STRING String + * STRUCT TYPE_MESSAGE JSONObject + * TIME TYPE_INT64 Number [byte, short, int, long] + * TIMESTAMP TYPE_INT64 Number [byte, short, int, long] */ public class JsonToProtoConverter { private static ImmutableMap @@ -64,10 +82,9 @@ public class JsonToProtoConverter { * @param BQTableSchema * @throws Descriptors.DescriptorValidationException */ - public static Descriptor BQTableSchemaToProtoSchema(Table.TableSchema BQTableSchema) + public static Descriptor ConvertBQTableSchemaToProtoSchema(Table.TableSchema BQTableSchema) throws Descriptors.DescriptorValidationException { - Descriptor descriptor = BQTableSchemaToProtoSchemaImpl(BQTableSchema, "root"); - return descriptor; + return ConvertBQTableSchemaToProtoSchemaImpl(BQTableSchema, "root"); } /** @@ -78,7 +95,7 @@ public static Descriptor BQTableSchemaToProtoSchema(Table.TableSchema BQTableSch * descriptor. * @throws Descriptors.DescriptorValidationException */ - private static Descriptor BQTableSchemaToProtoSchemaImpl( + private static Descriptor ConvertBQTableSchemaToProtoSchemaImpl( Table.TableSchema BQTableSchema, String scope) throws Descriptors.DescriptorValidationException { List dependenciesList = new ArrayList(); @@ -88,15 +105,15 @@ private static Descriptor BQTableSchemaToProtoSchemaImpl( if (BQTableField.getType() == Table.TableFieldSchema.Type.STRUCT) { String currentScope = scope + BQTableField.getName(); dependenciesList.add( - BQTableSchemaToProtoSchemaImpl( + ConvertBQTableSchemaToProtoSchemaImpl( Table.TableSchema.newBuilder() .addAllFields(BQTableField.getFieldsList()) .build(), currentScope) .getFile()); - fields.add(BQStructToProtoMessage(BQTableField, index++, currentScope)); + fields.add(ConvertBQStructToProtoMessage(BQTableField, index++, currentScope)); } else { - fields.add(BQTableFieldToProtoField(BQTableField, index++)); + fields.add(ConvertBQTableFieldToProtoField(BQTableField, index++)); } } FileDescriptor[] dependenciesArray = new FileDescriptor[dependenciesList.size()]; @@ -117,7 +134,7 @@ private static Descriptor BQTableSchemaToProtoSchemaImpl( * @param BQTableField BQ Field used to construct a FieldDescriptorProto * @param index Index for protobuf fields. */ - private static FieldDescriptorProto BQTableFieldToProtoField( + private static FieldDescriptorProto ConvertBQTableFieldToProtoField( Table.TableFieldSchema BQTableField, int index) { String fieldName = BQTableField.getName(); Table.TableFieldSchema.Mode mode = BQTableField.getMode(); @@ -136,7 +153,7 @@ private static FieldDescriptorProto BQTableFieldToProtoField( * @param index Index for protobuf fields. * @param scope Need scope to prevent naming issues */ - private static FieldDescriptorProto BQStructToProtoMessage( + private static FieldDescriptorProto ConvertBQStructToProtoMessage( Table.TableFieldSchema BQTableField, int index, String scope) { String fieldName = BQTableField.getName(); Table.TableFieldSchema.Mode mode = BQTableField.getMode(); diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java index c857a0e4b4..e5dfad2589 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java @@ -19,6 +19,7 @@ import static org.mockito.Mockito.*; import com.google.cloud.bigquery.storage.test.SchemaTest.*; +import com.google.cloud.bigquery.storage.test.JsonTest.*; import com.google.common.collect.ImmutableMap; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; @@ -29,8 +30,9 @@ @RunWith(JUnit4.class) public class JsonToProtoConverterTest { + // This is a map between the Table.TableFieldSchema.Type and the descriptor it is supposed to produce. The produced descriptor will be used to check against the entry values here. private static ImmutableMap - BQTableTypeToProtoDescriptor = + BQTableTypeToCorrectProtoDescriptorTest = new ImmutableMap.Builder() .put(Table.TableFieldSchema.Type.BOOL, BoolType.getDescriptor()) .put(Table.TableFieldSchema.Type.BYTES, BytesType.getDescriptor()) @@ -45,30 +47,32 @@ public class JsonToProtoConverterTest { .put(Table.TableFieldSchema.Type.TIMESTAMP, Int64Type.getDescriptor()) .build(); - private boolean isDescriptorEqual(Descriptor convertedProto, Descriptor originalProto) { + private void isDescriptorEqual(Descriptor convertedProto, Descriptor originalProto) { + // Check same number of fields + assertEquals(convertedProto.getFields().size(), originalProto.getFields().size()); for (FieldDescriptor convertedField : convertedProto.getFields()) { FieldDescriptor originalField = originalProto.findFieldByName(convertedField.getName()); - if (originalField == null) { - return false; - } + // Check name + assertNotNull(originalField); FieldDescriptor.Type convertedType = convertedField.getType(); FieldDescriptor.Type originalType = originalField.getType(); - if (convertedType != originalType) { - return false; - } + // Check type + assertEquals(convertedType, originalType); + // Check mode + assertEquals(originalField.isRepeated(), convertedField.isRepeated()); + assertEquals(originalField.isRequired(), convertedField.isRequired()); + assertEquals(originalField.isOptional(), convertedField.isOptional()); if (convertedType == FieldDescriptor.Type.MESSAGE) { - if (!isDescriptorEqual(convertedField.getMessageType(), originalField.getMessageType())) { - return false; - } + // Recursively check nested messages + isDescriptorEqual(convertedField.getMessageType(), originalField.getMessageType()); } } - return true; } @Test public void testBQTableSchemaToProtoDescriptorSimpleTypes() throws Exception { for (Map.Entry entry : - BQTableTypeToProtoDescriptor.entrySet()) { + BQTableTypeToCorrectProtoDescriptorTest.entrySet()) { Table.TableFieldSchema tableFieldSchema = Table.TableFieldSchema.newBuilder() .setType(entry.getKey()) @@ -77,13 +81,13 @@ public void testBQTableSchemaToProtoDescriptorSimpleTypes() throws Exception { .build(); Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); - Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - assertTrue(isDescriptorEqual(descriptor, entry.getValue())); + Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); + isDescriptorEqual(descriptor, entry.getValue()); } } @Test - public void testBQTableSchemaToProtoDescriptorComplex() throws Exception { + public void testBQTableSchemaToProtoDescriptorStructSimple() throws Exception { Table.TableFieldSchema StringType = Table.TableFieldSchema.newBuilder() .setType(Table.TableFieldSchema.Type.STRING) @@ -99,7 +103,62 @@ public void testBQTableSchemaToProtoDescriptorComplex() throws Exception { .build(); Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); - Descriptor descriptor = JsonToProtoConverter.BQTableSchemaToProtoSchema(tableSchema); - assertTrue(isDescriptorEqual(descriptor, MessageType.getDescriptor())); + Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); + isDescriptorEqual(descriptor, MessageType.getDescriptor()); + } + + @Test + public void testBQTableSchemaToProtoDescriptorStructComplex() throws Exception { + Table.TableFieldSchema test_int = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.INT64) + .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .setName("test_int") + .build(); + Table.TableFieldSchema NestingLvl2 = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.STRUCT) + .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .addFields(0, test_int) + .setName("nesting_value") + .build(); + Table.TableFieldSchema NestingLvl1 = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.STRUCT) + .setMode(Table.TableFieldSchema.Mode.REPEATED) + .addFields(0, test_int) + .addFields(1, NestingLvl2) + .setName("nesting_value1") + .build(); + Table.TableSchema tableSchema = + Table.TableSchema.newBuilder().addFields(0, test_int).addFields(1, NestingLvl1).addFields(2, NestingLvl2).build(); + Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); + isDescriptorEqual(descriptor, NestingStackedLvl0.getDescriptor()); + } + + @Test + public void testBQTableSchemaToProtoDescriptorOptions() throws Exception { + Table.TableFieldSchema required = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.INT64) + .setMode(Table.TableFieldSchema.Mode.REQUIRED) + .setName("test_required") + .build(); + Table.TableFieldSchema repeated = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.INT64) + .setMode(Table.TableFieldSchema.Mode.REPEATED) + .setName("test_repeated") + .build(); + Table.TableFieldSchema optional = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.INT64) + .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .setName("test_optional") + .build(); + Table.TableSchema tableSchema = + Table.TableSchema.newBuilder().addFields(0, required).addFields(1, repeated).addFields(2, optional).build(); + Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); + isDescriptorEqual(descriptor, OptionTest.getDescriptor()); } } diff --git a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto new file mode 100644 index 0000000000..ee75de0305 --- /dev/null +++ b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto @@ -0,0 +1,24 @@ +syntax = "proto2"; + +package com.google.cloud.bigquery.storage.test; + +message NestingLvl1 { + optional int64 test_int = 1; + optional NestingLvl2 nesting_value = 2; +} + +message NestingLvl2 { + optional int64 test_int = 1; +} + +message NestingStackedLvl0 { + optional int64 test_int = 1; + repeated NestingLvl1 nesting_value1 = 2; + optional NestingLvl2 nesting_value = 3; +} + +message OptionTest { + optional int64 test_optional = 1; + required int64 test_required = 2; + repeated int64 test_repeated = 3; +} From eea28b429c9b3fbd1ef8a2effe2a691f23edb1d4 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Mon, 6 Jul 2020 16:38:09 -0500 Subject: [PATCH 06/13] Fix according to PR conversations and checked linting --- .../v1alpha2/JsonToProtoConverter.java | 36 ++++------- .../v1alpha2/JsonToProtoConverterTest.java | 59 +++++++++++-------- 2 files changed, 47 insertions(+), 48 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java index 3a82ed9a79..fd14638264 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java @@ -26,30 +26,20 @@ import java.util.List; /** - * This class can convert Json data to protobuf messages given a protobuf descriptor. - * The data types will be mapped as shown in the table below. - * Some rules to follow: - * - If field is required in protobuf, then it must be present in the json data - * - If field is optional in protobuf, then it is optional in the json data. - * - The casing must match between protobuf field names and json key names. - * - If there are more json fields than protobuf fields, the allowUnknownFields flag must be set to true. - * - There can be more fields in protobuf fields as long as they are optional. + * This class can convert Json data to protobuf messages given a protobuf descriptor. The data types + * will be mapped as shown in the table below. Some rules to follow: - If field is required in + * protobuf, then it must be present in the json data - If field is optional in protobuf, then it is + * optional in the json data. - The casing must match between protobuf field names and json key + * names. - If there are more json fields than protobuf fields, the allowUnknownFields flag must be + * set to true. - There can be more fields in protobuf fields as long as they are optional. * - * This class also provides a converter from a BQ table schema to protobuf descriptor. - * It will follow the following mapping: - * BQ Type -> Protobuf Type -> Json Data Type - * BOOL TYPE_BOOL Boolean - * BYTES TYPE_BYTES String - * DATE TYPE_INT64 Number [byte, short, int, long] - * DATETIME TYPE_INT64 Number [byte, short, int, long] - * DOUBLE TYPE_DOUBLE Number [float, double] - * GEOGRAPHY TYPE_BYTES String - * INT64 TYPE_INT64 Number [byte, short, int, long] - * NUMERIC TYPE_BYTES String - * STRING TYPE_STRING String - * STRUCT TYPE_MESSAGE JSONObject - * TIME TYPE_INT64 Number [byte, short, int, long] - * TIMESTAMP TYPE_INT64 Number [byte, short, int, long] + *

This class also provides a converter from a BQ table schema to protobuf descriptor. It will + * follow the following mapping: BQ Type -> Protobuf Type -> Json Data Type BOOL TYPE_BOOL Boolean + * BYTES TYPE_BYTES String DATE TYPE_INT64 Number [byte, short, int, long] DATETIME TYPE_INT64 + * Number [byte, short, int, long] DOUBLE TYPE_DOUBLE Number [float, double] GEOGRAPHY TYPE_BYTES + * String INT64 TYPE_INT64 Number [byte, short, int, long] NUMERIC TYPE_BYTES String STRING + * TYPE_STRING String STRUCT TYPE_MESSAGE JSONObject TIME TYPE_INT64 Number [byte, short, int, long] + * TIMESTAMP TYPE_INT64 Number [byte, short, int, long] */ public class JsonToProtoConverter { private static ImmutableMap diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java index e5dfad2589..1a17332d9b 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java @@ -18,8 +18,8 @@ import static org.junit.Assert.*; import static org.mockito.Mockito.*; -import com.google.cloud.bigquery.storage.test.SchemaTest.*; 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; import com.google.protobuf.Descriptors.FieldDescriptor; @@ -30,7 +30,8 @@ @RunWith(JUnit4.class) public class JsonToProtoConverterTest { - // This is a map between the Table.TableFieldSchema.Type and the descriptor it is supposed to produce. The produced descriptor will be used to check against the entry values here. + // This is a map between the Table.TableFieldSchema.Type and the descriptor it is supposed to + // produce. The produced descriptor will be used to check against the entry values here. private static ImmutableMap BQTableTypeToCorrectProtoDescriptorTest = new ImmutableMap.Builder() @@ -131,34 +132,42 @@ public void testBQTableSchemaToProtoDescriptorStructComplex() throws Exception { .setName("nesting_value1") .build(); Table.TableSchema tableSchema = - Table.TableSchema.newBuilder().addFields(0, test_int).addFields(1, NestingLvl1).addFields(2, NestingLvl2).build(); + Table.TableSchema.newBuilder() + .addFields(0, test_int) + .addFields(1, NestingLvl1) + .addFields(2, NestingLvl2) + .build(); Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); isDescriptorEqual(descriptor, NestingStackedLvl0.getDescriptor()); } @Test public void testBQTableSchemaToProtoDescriptorOptions() throws Exception { - Table.TableFieldSchema required = - Table.TableFieldSchema.newBuilder() - .setType(Table.TableFieldSchema.Type.INT64) - .setMode(Table.TableFieldSchema.Mode.REQUIRED) - .setName("test_required") - .build(); - Table.TableFieldSchema repeated = - Table.TableFieldSchema.newBuilder() - .setType(Table.TableFieldSchema.Type.INT64) - .setMode(Table.TableFieldSchema.Mode.REPEATED) - .setName("test_repeated") - .build(); - Table.TableFieldSchema optional = - Table.TableFieldSchema.newBuilder() - .setType(Table.TableFieldSchema.Type.INT64) - .setMode(Table.TableFieldSchema.Mode.NULLABLE) - .setName("test_optional") - .build(); - Table.TableSchema tableSchema = - Table.TableSchema.newBuilder().addFields(0, required).addFields(1, repeated).addFields(2, optional).build(); - Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); - isDescriptorEqual(descriptor, OptionTest.getDescriptor()); + Table.TableFieldSchema required = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.INT64) + .setMode(Table.TableFieldSchema.Mode.REQUIRED) + .setName("test_required") + .build(); + Table.TableFieldSchema repeated = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.INT64) + .setMode(Table.TableFieldSchema.Mode.REPEATED) + .setName("test_repeated") + .build(); + Table.TableFieldSchema optional = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.INT64) + .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .setName("test_optional") + .build(); + Table.TableSchema tableSchema = + Table.TableSchema.newBuilder() + .addFields(0, required) + .addFields(1, repeated) + .addFields(2, optional) + .build(); + Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); + isDescriptorEqual(descriptor, OptionTest.getDescriptor()); } } From 5ed3b9bdd63ffe4579196282141090fd91641b40 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Mon, 6 Jul 2020 16:43:27 -0500 Subject: [PATCH 07/13] Renamed from JsonToProtoConvereter to BQTableSchemaToProtoDescriptor. This is to split it up to two classes. --- ...java => BQTableSchemaToProtoDescriptor.java} | 17 ++--------------- ... => BQTableSchemaToProtoDescriptorTest.java} | 2 +- 2 files changed, 3 insertions(+), 16 deletions(-) rename google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/{JsonToProtoConverter.java => BQTableSchemaToProtoDescriptor.java} (84%) rename google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/{JsonToProtoConverterTest.java => BQTableSchemaToProtoDescriptorTest.java} (99%) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java similarity index 84% rename from google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java rename to google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java index fd14638264..4550dd2a7f 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverter.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java @@ -26,22 +26,9 @@ import java.util.List; /** - * This class can convert Json data to protobuf messages given a protobuf descriptor. The data types - * will be mapped as shown in the table below. Some rules to follow: - If field is required in - * protobuf, then it must be present in the json data - If field is optional in protobuf, then it is - * optional in the json data. - The casing must match between protobuf field names and json key - * names. - If there are more json fields than protobuf fields, the allowUnknownFields flag must be - * set to true. - There can be more fields in protobuf fields as long as they are optional. - * - *

This class also provides a converter from a BQ table schema to protobuf descriptor. It will - * follow the following mapping: BQ Type -> Protobuf Type -> Json Data Type BOOL TYPE_BOOL Boolean - * BYTES TYPE_BYTES String DATE TYPE_INT64 Number [byte, short, int, long] DATETIME TYPE_INT64 - * Number [byte, short, int, long] DOUBLE TYPE_DOUBLE Number [float, double] GEOGRAPHY TYPE_BYTES - * String INT64 TYPE_INT64 Number [byte, short, int, long] NUMERIC TYPE_BYTES String STRING - * TYPE_STRING String STRUCT TYPE_MESSAGE JSONObject TIME TYPE_INT64 Number [byte, short, int, long] - * TIMESTAMP TYPE_INT64 Number [byte, short, int, long] + * This class converts a BQ table schema to protobuf descriptor. The mapping between field types and field modes are shown in the ImmutableMaps below. */ -public class JsonToProtoConverter { +public class BQTableSchemaToProtoSchema { private static ImmutableMap BQTableSchemaModeMap = ImmutableMap.of( diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java similarity index 99% rename from google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java rename to google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java index 1a17332d9b..bc7a62f2a9 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoConverterTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java @@ -29,7 +29,7 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class JsonToProtoConverterTest { +public class BQTableSchemaToProtoDescriptorTest { // This is a map between the Table.TableFieldSchema.Type and the descriptor it is supposed to // produce. The produced descriptor will be used to check against the entry values here. private static ImmutableMap From c0e8685704bf11954499965ca00e5c97f2baa107 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Mon, 6 Jul 2020 16:44:29 -0500 Subject: [PATCH 08/13] Linting --- .../storage/v1alpha2/BQTableSchemaToProtoDescriptor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java index 4550dd2a7f..9d3fa19d71 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java @@ -26,7 +26,8 @@ import java.util.List; /** - * This class converts a BQ table schema to protobuf descriptor. The mapping between field types and field modes are shown in the ImmutableMaps below. + * This class converts a BQ table schema to protobuf descriptor. The mapping between field types and + * field modes are shown in the ImmutableMaps below. */ public class BQTableSchemaToProtoSchema { private static ImmutableMap From a6d786c35c7c87d75e36dc3c2707c740e66e001a Mon Sep 17 00:00:00 2001 From: allenc3 Date: Mon, 6 Jul 2020 16:49:34 -0500 Subject: [PATCH 09/13] make sure tests work and rename is good --- .../BQTableSchemaToProtoDescriptor.java | 2 +- .../BQTableSchemaToProtoDescriptorTest.java | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java index 9d3fa19d71..f094230525 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java @@ -29,7 +29,7 @@ * This class converts a BQ table schema to protobuf descriptor. The mapping between field types and * field modes are shown in the ImmutableMaps below. */ -public class BQTableSchemaToProtoSchema { +public class BQTableSchemaToProtoDescriptor { private static ImmutableMap BQTableSchemaModeMap = ImmutableMap.of( diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java index bc7a62f2a9..06a845d151 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java @@ -60,9 +60,10 @@ private void isDescriptorEqual(Descriptor convertedProto, Descriptor originalPro // Check type assertEquals(convertedType, originalType); // Check mode - assertEquals(originalField.isRepeated(), convertedField.isRepeated()); - assertEquals(originalField.isRequired(), convertedField.isRequired()); - assertEquals(originalField.isOptional(), convertedField.isOptional()); + assertTrue( + (originalField.isRepeated() == convertedField.isRepeated()) + || (originalField.isRequired() == convertedField.isRequired()) + || (originalField.isOptional() == convertedField.isOptional())); if (convertedType == FieldDescriptor.Type.MESSAGE) { // Recursively check nested messages isDescriptorEqual(convertedField.getMessageType(), originalField.getMessageType()); @@ -82,7 +83,8 @@ public void testBQTableSchemaToProtoDescriptorSimpleTypes() throws Exception { .build(); Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); - Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); + Descriptor descriptor = + BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoSchema(tableSchema); isDescriptorEqual(descriptor, entry.getValue()); } } @@ -104,7 +106,8 @@ public void testBQTableSchemaToProtoDescriptorStructSimple() throws Exception { .build(); Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); - Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); + Descriptor descriptor = + BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoSchema(tableSchema); isDescriptorEqual(descriptor, MessageType.getDescriptor()); } @@ -137,7 +140,8 @@ public void testBQTableSchemaToProtoDescriptorStructComplex() throws Exception { .addFields(1, NestingLvl1) .addFields(2, NestingLvl2) .build(); - Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); + Descriptor descriptor = + BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoSchema(tableSchema); isDescriptorEqual(descriptor, NestingStackedLvl0.getDescriptor()); } @@ -167,7 +171,8 @@ public void testBQTableSchemaToProtoDescriptorOptions() throws Exception { .addFields(1, repeated) .addFields(2, optional) .build(); - Descriptor descriptor = JsonToProtoConverter.ConvertBQTableSchemaToProtoSchema(tableSchema); + Descriptor descriptor = + BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoSchema(tableSchema); isDescriptorEqual(descriptor, OptionTest.getDescriptor()); } } From 8dcb1c29f4d518f8ee33815ae72fef0043a8f91e Mon Sep 17 00:00:00 2001 From: allenc3 Date: Mon, 6 Jul 2020 17:10:06 -0500 Subject: [PATCH 10/13] Renamed functions --- .../storage/v1alpha2/BQTableSchemaToProtoDescriptor.java | 8 ++++---- .../v1alpha2/BQTableSchemaToProtoDescriptorTest.java | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java index f094230525..a88fb6bc55 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java @@ -60,9 +60,9 @@ public class BQTableSchemaToProtoDescriptor { * @param BQTableSchema * @throws Descriptors.DescriptorValidationException */ - public static Descriptor ConvertBQTableSchemaToProtoSchema(Table.TableSchema BQTableSchema) + public static Descriptor ConvertBQTableSchemaToProtoDescriptor(Table.TableSchema BQTableSchema) throws Descriptors.DescriptorValidationException { - return ConvertBQTableSchemaToProtoSchemaImpl(BQTableSchema, "root"); + return ConvertBQTableSchemaToProtoDescriptorImpl(BQTableSchema, "root"); } /** @@ -73,7 +73,7 @@ public static Descriptor ConvertBQTableSchemaToProtoSchema(Table.TableSchema BQT * descriptor. * @throws Descriptors.DescriptorValidationException */ - private static Descriptor ConvertBQTableSchemaToProtoSchemaImpl( + private static Descriptor ConvertBQTableSchemaToProtoDescriptorImpl( Table.TableSchema BQTableSchema, String scope) throws Descriptors.DescriptorValidationException { List dependenciesList = new ArrayList(); @@ -83,7 +83,7 @@ private static Descriptor ConvertBQTableSchemaToProtoSchemaImpl( if (BQTableField.getType() == Table.TableFieldSchema.Type.STRUCT) { String currentScope = scope + BQTableField.getName(); dependenciesList.add( - ConvertBQTableSchemaToProtoSchemaImpl( + ConvertBQTableSchemaToProtoDescriptorImpl( Table.TableSchema.newBuilder() .addAllFields(BQTableField.getFieldsList()) .build(), diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java index 06a845d151..c2e0ddf4cf 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java @@ -84,7 +84,7 @@ public void testBQTableSchemaToProtoDescriptorSimpleTypes() throws Exception { Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); Descriptor descriptor = - BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoSchema(tableSchema); + BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoDescriptor(tableSchema); isDescriptorEqual(descriptor, entry.getValue()); } } @@ -107,7 +107,7 @@ public void testBQTableSchemaToProtoDescriptorStructSimple() throws Exception { Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); Descriptor descriptor = - BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoSchema(tableSchema); + BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoDescriptor(tableSchema); isDescriptorEqual(descriptor, MessageType.getDescriptor()); } @@ -141,7 +141,7 @@ public void testBQTableSchemaToProtoDescriptorStructComplex() throws Exception { .addFields(2, NestingLvl2) .build(); Descriptor descriptor = - BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoSchema(tableSchema); + BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoDescriptor(tableSchema); isDescriptorEqual(descriptor, NestingStackedLvl0.getDescriptor()); } @@ -172,7 +172,7 @@ public void testBQTableSchemaToProtoDescriptorOptions() throws Exception { .addFields(2, optional) .build(); Descriptor descriptor = - BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoSchema(tableSchema); + BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoDescriptor(tableSchema); isDescriptorEqual(descriptor, OptionTest.getDescriptor()); } } From 8f6ee9b6709c0c414cb35a8a2e46af83d2fb8bc4 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 7 Jul 2020 11:15:59 -0500 Subject: [PATCH 11/13] Fixed bugs according to PR, added descriptor reuse (instead of creating the same descriptor again) and a test case that checks for this. --- .../BQTableSchemaToProtoDescriptor.java | 41 +++-- .../BQTableSchemaToProtoDescriptorTest.java | 170 ++++++++++++++---- .../src/test/proto/jsonTest.proto | 31 +++- 3 files changed, 188 insertions(+), 54 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java index a88fb6bc55..29a63abde7 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java @@ -15,6 +15,7 @@ */ package com.google.cloud.bigquery.storage.v1alpha2; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.protobuf.DescriptorProtos.DescriptorProto; import com.google.protobuf.DescriptorProtos.FieldDescriptorProto; @@ -23,11 +24,12 @@ import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FileDescriptor; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; /** - * This class converts a BQ table schema to protobuf descriptor. The mapping between field types and - * field modes are shown in the ImmutableMaps below. + * Converts a BQ table schema to protobuf descriptor. The mapping between field types and field + * modes are shown in the ImmutableMaps below. */ public class BQTableSchemaToProtoDescriptor { private static ImmutableMap @@ -62,11 +64,12 @@ public class BQTableSchemaToProtoDescriptor { */ public static Descriptor ConvertBQTableSchemaToProtoDescriptor(Table.TableSchema BQTableSchema) throws Descriptors.DescriptorValidationException { - return ConvertBQTableSchemaToProtoDescriptorImpl(BQTableSchema, "root"); + return ConvertBQTableSchemaToProtoDescriptorImpl( + BQTableSchema, "root", new HashMap, Descriptor>()); } /** - * Implementation that converts a Table.TableSchema to a Descriptors.Descriptor object. + * Converts a Table.TableSchema to a Descriptors.Descriptor object. * * @param BQTableSchema * @param scope Keeps track of current scope to prevent repeated naming while constructing @@ -74,22 +77,32 @@ public static Descriptor ConvertBQTableSchemaToProtoDescriptor(Table.TableSchema * @throws Descriptors.DescriptorValidationException */ private static Descriptor ConvertBQTableSchemaToProtoDescriptorImpl( - Table.TableSchema BQTableSchema, String scope) + Table.TableSchema BQTableSchema, + String scope, + HashMap, Descriptor> dependencyMap) throws Descriptors.DescriptorValidationException { List dependenciesList = new ArrayList(); List fields = new ArrayList(); int index = 1; for (Table.TableFieldSchema BQTableField : BQTableSchema.getFieldsList()) { if (BQTableField.getType() == Table.TableFieldSchema.Type.STRUCT) { - String currentScope = scope + BQTableField.getName(); - dependenciesList.add( - ConvertBQTableSchemaToProtoDescriptorImpl( - Table.TableSchema.newBuilder() - .addAllFields(BQTableField.getFieldsList()) - .build(), - currentScope) - .getFile()); - fields.add(ConvertBQStructToProtoMessage(BQTableField, index++, currentScope)); + ImmutableList fieldList = + ImmutableList.copyOf(BQTableField.getFieldsList()); + String currentScope = scope + "__" + BQTableField.getName(); + if (dependencyMap.containsKey(fieldList)) { + Descriptor descriptor = dependencyMap.get(fieldList); + dependenciesList.add(descriptor.getFile()); + fields.add(ConvertBQStructToProtoMessage(BQTableField, index++, descriptor.getName())); + } else { + Descriptor descriptor = + ConvertBQTableSchemaToProtoDescriptorImpl( + Table.TableSchema.newBuilder().addAllFields(fieldList).build(), + currentScope, + dependencyMap); + dependenciesList.add(descriptor.getFile()); + dependencyMap.put(fieldList, descriptor); + fields.add(ConvertBQStructToProtoMessage(BQTableField, index++, currentScope)); + } } else { fields.add(ConvertBQTableFieldToProtoField(BQTableField, index++)); } diff --git a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java index c2e0ddf4cf..e2cb04d1f4 100644 --- a/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java +++ b/google-cloud-bigquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; +import java.util.HashMap; import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; @@ -48,131 +49,232 @@ public class BQTableSchemaToProtoDescriptorTest { .put(Table.TableFieldSchema.Type.TIMESTAMP, Int64Type.getDescriptor()) .build(); + // Creates mapping from descriptor to how many times it was reused. + private void mapDescriptorToCount(Descriptor descriptor, HashMap map) { + for (FieldDescriptor field : descriptor.getFields()) { + if (field.getType() == FieldDescriptor.Type.MESSAGE) { + Descriptor subDescriptor = field.getMessageType(); + String messageName = subDescriptor.getName(); + if (map.containsKey(messageName)) { + map.put(messageName, map.get(messageName) + 1); + } else { + map.put(messageName, 1); + } + mapDescriptorToCount(subDescriptor, map); + } + } + } + private void isDescriptorEqual(Descriptor convertedProto, Descriptor originalProto) { // Check same number of fields assertEquals(convertedProto.getFields().size(), originalProto.getFields().size()); for (FieldDescriptor convertedField : convertedProto.getFields()) { + // Check field name FieldDescriptor originalField = originalProto.findFieldByName(convertedField.getName()); - // Check name assertNotNull(originalField); + // Check type FieldDescriptor.Type convertedType = convertedField.getType(); FieldDescriptor.Type originalType = originalField.getType(); - // Check type assertEquals(convertedType, originalType); // Check mode assertTrue( (originalField.isRepeated() == convertedField.isRepeated()) - || (originalField.isRequired() == convertedField.isRequired()) - || (originalField.isOptional() == convertedField.isOptional())); + && (originalField.isRequired() == convertedField.isRequired()) + && (originalField.isOptional() == convertedField.isOptional())); + // Recursively check nested messages if (convertedType == FieldDescriptor.Type.MESSAGE) { - // Recursively check nested messages isDescriptorEqual(convertedField.getMessageType(), originalField.getMessageType()); } } } @Test - public void testBQTableSchemaToProtoDescriptorSimpleTypes() throws Exception { + public void testSimpleTypes() throws Exception { for (Map.Entry entry : BQTableTypeToCorrectProtoDescriptorTest.entrySet()) { - Table.TableFieldSchema tableFieldSchema = + final Table.TableFieldSchema tableFieldSchema = Table.TableFieldSchema.newBuilder() .setType(entry.getKey()) .setMode(Table.TableFieldSchema.Mode.NULLABLE) .setName("test_field_type") .build(); - Table.TableSchema tableSchema = + final Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); - Descriptor descriptor = + final Descriptor descriptor = BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoDescriptor(tableSchema); isDescriptorEqual(descriptor, entry.getValue()); } } @Test - public void testBQTableSchemaToProtoDescriptorStructSimple() throws Exception { - Table.TableFieldSchema StringType = + public void testStructSimple() throws Exception { + final Table.TableFieldSchema StringType = Table.TableFieldSchema.newBuilder() .setType(Table.TableFieldSchema.Type.STRING) .setMode(Table.TableFieldSchema.Mode.NULLABLE) .setName("test_field_type") .build(); - Table.TableFieldSchema tableFieldSchema = + final 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.TableSchema tableSchema = + final Table.TableSchema tableSchema = Table.TableSchema.newBuilder().addFields(0, tableFieldSchema).build(); - Descriptor descriptor = + final Descriptor descriptor = BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoDescriptor(tableSchema); isDescriptorEqual(descriptor, MessageType.getDescriptor()); } @Test - public void testBQTableSchemaToProtoDescriptorStructComplex() throws Exception { - Table.TableFieldSchema test_int = + public void testStructComplex() throws Exception { + final Table.TableFieldSchema test_int = Table.TableFieldSchema.newBuilder() .setType(Table.TableFieldSchema.Type.INT64) .setMode(Table.TableFieldSchema.Mode.NULLABLE) .setName("test_int") .build(); - Table.TableFieldSchema NestingLvl2 = + final Table.TableFieldSchema test_string = Table.TableFieldSchema.newBuilder() - .setType(Table.TableFieldSchema.Type.STRUCT) + .setType(Table.TableFieldSchema.Type.STRING) + .setMode(Table.TableFieldSchema.Mode.REPEATED) + .setName("test_string") + .build(); + final Table.TableFieldSchema test_bytes = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.BYTES) + .setMode(Table.TableFieldSchema.Mode.REQUIRED) + .setName("test_bytes") + .build(); + final Table.TableFieldSchema test_bool = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.BOOL) .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .setName("test_bool") + .build(); + final Table.TableFieldSchema test_double = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.DOUBLE) + .setMode(Table.TableFieldSchema.Mode.REPEATED) + .setName("test_double") + .build(); + final Table.TableFieldSchema ComplexLvl2 = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.STRUCT) + .setMode(Table.TableFieldSchema.Mode.REQUIRED) .addFields(0, test_int) - .setName("nesting_value") + .setName("complexLvl2") .build(); - Table.TableFieldSchema NestingLvl1 = + final Table.TableFieldSchema ComplexLvl1 = Table.TableFieldSchema.newBuilder() .setType(Table.TableFieldSchema.Type.STRUCT) - .setMode(Table.TableFieldSchema.Mode.REPEATED) + .setMode(Table.TableFieldSchema.Mode.REQUIRED) .addFields(0, test_int) - .addFields(1, NestingLvl2) - .setName("nesting_value1") + .addFields(1, ComplexLvl2) + .setName("complexLvl1") .build(); - Table.TableSchema tableSchema = + final Table.TableSchema tableSchema = Table.TableSchema.newBuilder() .addFields(0, test_int) - .addFields(1, NestingLvl1) - .addFields(2, NestingLvl2) + .addFields(1, test_string) + .addFields(2, test_bytes) + .addFields(3, test_bool) + .addFields(4, test_double) + .addFields(5, ComplexLvl1) + .addFields(6, ComplexLvl2) .build(); - Descriptor descriptor = + final Descriptor descriptor = BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoDescriptor(tableSchema); - isDescriptorEqual(descriptor, NestingStackedLvl0.getDescriptor()); + isDescriptorEqual(descriptor, ComplexRoot.getDescriptor()); } @Test - public void testBQTableSchemaToProtoDescriptorOptions() throws Exception { - Table.TableFieldSchema required = + public void testOptions() throws Exception { + final Table.TableFieldSchema required = Table.TableFieldSchema.newBuilder() .setType(Table.TableFieldSchema.Type.INT64) .setMode(Table.TableFieldSchema.Mode.REQUIRED) .setName("test_required") .build(); - Table.TableFieldSchema repeated = + final Table.TableFieldSchema repeated = Table.TableFieldSchema.newBuilder() .setType(Table.TableFieldSchema.Type.INT64) .setMode(Table.TableFieldSchema.Mode.REPEATED) .setName("test_repeated") .build(); - Table.TableFieldSchema optional = + final Table.TableFieldSchema optional = Table.TableFieldSchema.newBuilder() .setType(Table.TableFieldSchema.Type.INT64) .setMode(Table.TableFieldSchema.Mode.NULLABLE) .setName("test_optional") .build(); - Table.TableSchema tableSchema = + final Table.TableSchema tableSchema = Table.TableSchema.newBuilder() .addFields(0, required) .addFields(1, repeated) .addFields(2, optional) .build(); - Descriptor descriptor = + final Descriptor descriptor = BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoDescriptor(tableSchema); isDescriptorEqual(descriptor, OptionTest.getDescriptor()); } + + @Test + public void testDescriptorReuseDuringCreation() throws Exception { + final Table.TableFieldSchema test_int = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.INT64) + .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .setName("test_int") + .build(); + final Table.TableFieldSchema reuse_lvl2 = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.STRUCT) + .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .setName("reuse_lvl2") + .addFields(0, test_int) + .build(); + final Table.TableFieldSchema reuse_lvl1 = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.STRUCT) + .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .setName("reuse_lvl1") + .addFields(0, test_int) + .addFields(0, reuse_lvl2) + .build(); + final Table.TableFieldSchema reuse_lvl1_1 = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.STRUCT) + .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .setName("reuse_lvl1_1") + .addFields(0, test_int) + .addFields(0, reuse_lvl2) + .build(); + final Table.TableFieldSchema reuse_lvl1_2 = + Table.TableFieldSchema.newBuilder() + .setType(Table.TableFieldSchema.Type.STRUCT) + .setMode(Table.TableFieldSchema.Mode.NULLABLE) + .setName("reuse_lvl1_2") + .addFields(0, test_int) + .addFields(0, reuse_lvl2) + .build(); + final Table.TableSchema tableSchema = + Table.TableSchema.newBuilder() + .addFields(0, reuse_lvl1) + .addFields(1, reuse_lvl1_1) + .addFields(2, reuse_lvl1_2) + .build(); + final Descriptor descriptor = + BQTableSchemaToProtoDescriptor.ConvertBQTableSchemaToProtoDescriptor(tableSchema); + HashMap descriptorToCount = new HashMap(); + mapDescriptorToCount(descriptor, descriptorToCount); + assertEquals(descriptorToCount.size(), 2); + assertTrue(descriptorToCount.containsKey("root__reuse_lvl1")); + assertEquals(descriptorToCount.get("root__reuse_lvl1").intValue(), 3); + assertTrue(descriptorToCount.containsKey("root__reuse_lvl1__reuse_lvl2")); + assertEquals(descriptorToCount.get("root__reuse_lvl1__reuse_lvl2").intValue(), 3); + isDescriptorEqual(descriptor, ReuseRoot.getDescriptor()); + } } diff --git a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto index ee75de0305..c531e09096 100644 --- a/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto +++ b/google-cloud-bigquerystorage/src/test/proto/jsonTest.proto @@ -2,19 +2,23 @@ syntax = "proto2"; package com.google.cloud.bigquery.storage.test; -message NestingLvl1 { +message ComplexRoot { optional int64 test_int = 1; - optional NestingLvl2 nesting_value = 2; + repeated string test_string = 2; + required bytes test_bytes = 3; + optional bool test_bool = 4; + repeated double test_double = 5; + required ComplexLvl1 complexLvl1 = 6; + required ComplexLvl2 complexLvl2 = 7; } -message NestingLvl2 { +message ComplexLvl1 { optional int64 test_int = 1; + required ComplexLvl2 complexLvl2 = 2; } -message NestingStackedLvl0 { +message ComplexLvl2 { optional int64 test_int = 1; - repeated NestingLvl1 nesting_value1 = 2; - optional NestingLvl2 nesting_value = 3; } message OptionTest { @@ -22,3 +26,18 @@ message OptionTest { required int64 test_required = 2; repeated int64 test_repeated = 3; } + +message ReuseRoot { + optional ReuseLvl1 reuse_lvl1 = 1; + optional ReuseLvl1 reuse_lvl1_1 = 2; + optional ReuseLvl1 reuse_lvl1_2 = 3; +} + +message ReuseLvl1 { + optional int64 test_int = 1; + optional ReuseLvl2 reuse_lvl2 = 2; +} + +message ReuseLvl2 { + optional int64 test_int = 1; +} From bdcaa22b9a105f9dbb602b6fcf1a9942d79ae5c0 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 7 Jul 2020 12:59:45 -0500 Subject: [PATCH 12/13] Update documentation --- .../storage/v1alpha2/BQTableSchemaToProtoDescriptor.java | 1 + 1 file changed, 1 insertion(+) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java index 29a63abde7..a5b063f0f2 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java @@ -74,6 +74,7 @@ public static Descriptor ConvertBQTableSchemaToProtoDescriptor(Table.TableSchema * @param BQTableSchema * @param scope Keeps track of current scope to prevent repeated naming while constructing * descriptor. + * @param dependencyMap Stores already constructed descriptors to prevent reconstruction * @throws Descriptors.DescriptorValidationException */ private static Descriptor ConvertBQTableSchemaToProtoDescriptorImpl( From c8569dca10e41f0046c3275808afdb9f8630e9e1 Mon Sep 17 00:00:00 2001 From: allenc3 Date: Tue, 7 Jul 2020 13:55:48 -0500 Subject: [PATCH 13/13] Merge ConvertBQTableFieldToProtoField and ConvertBQStructToProtoMsg --- .../BQTableSchemaToProtoDescriptor.java | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java index a5b063f0f2..946d2bc7c8 100644 --- a/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java +++ b/google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptor.java @@ -86,14 +86,14 @@ private static Descriptor ConvertBQTableSchemaToProtoDescriptorImpl( List fields = new ArrayList(); int index = 1; for (Table.TableFieldSchema BQTableField : BQTableSchema.getFieldsList()) { + String currentScope = scope + "__" + BQTableField.getName(); if (BQTableField.getType() == Table.TableFieldSchema.Type.STRUCT) { ImmutableList fieldList = ImmutableList.copyOf(BQTableField.getFieldsList()); - String currentScope = scope + "__" + BQTableField.getName(); if (dependencyMap.containsKey(fieldList)) { Descriptor descriptor = dependencyMap.get(fieldList); dependenciesList.add(descriptor.getFile()); - fields.add(ConvertBQStructToProtoMessage(BQTableField, index++, descriptor.getName())); + fields.add(ConvertBQTableFieldToProtoField(BQTableField, index++, descriptor.getName())); } else { Descriptor descriptor = ConvertBQTableSchemaToProtoDescriptorImpl( @@ -102,10 +102,10 @@ private static Descriptor ConvertBQTableSchemaToProtoDescriptorImpl( dependencyMap); dependenciesList.add(descriptor.getFile()); dependencyMap.put(fieldList, descriptor); - fields.add(ConvertBQStructToProtoMessage(BQTableField, index++, currentScope)); + fields.add(ConvertBQTableFieldToProtoField(BQTableField, index++, currentScope)); } } else { - fields.add(ConvertBQTableFieldToProtoField(BQTableField, index++)); + fields.add(ConvertBQTableFieldToProtoField(BQTableField, index++, currentScope)); } } FileDescriptor[] dependenciesArray = new FileDescriptor[dependenciesList.size()]; @@ -121,37 +121,27 @@ private static Descriptor ConvertBQTableSchemaToProtoDescriptorImpl( } /** - * Constructs a FieldDescriptorProto for non-struct BQ fields. + * Converts a BQTableField to ProtoField * * @param BQTableField BQ Field used to construct a FieldDescriptorProto * @param index Index for protobuf fields. + * @param scope used to name descriptors */ private static FieldDescriptorProto ConvertBQTableFieldToProtoField( - Table.TableFieldSchema BQTableField, int index) { - String fieldName = BQTableField.getName(); - Table.TableFieldSchema.Mode mode = BQTableField.getMode(); - return FieldDescriptorProto.newBuilder() - .setName(fieldName) - .setType((FieldDescriptorProto.Type) BQTableSchemaTypeMap.get(BQTableField.getType())) - .setLabel((FieldDescriptorProto.Label) BQTableSchemaModeMap.get(mode)) - .setNumber(index) - .build(); - } - - /** - * Constructs a FieldDescriptorProto for a Struct type BQ field. - * - * @param BQTableField BQ Field used to construct a FieldDescriptorProto - * @param index Index for protobuf fields. - * @param scope Need scope to prevent naming issues - */ - private static FieldDescriptorProto ConvertBQStructToProtoMessage( Table.TableFieldSchema BQTableField, int index, String scope) { - String fieldName = BQTableField.getName(); Table.TableFieldSchema.Mode mode = BQTableField.getMode(); + String fieldName = BQTableField.getName(); + if (BQTableField.getType() == Table.TableFieldSchema.Type.STRUCT) { + return FieldDescriptorProto.newBuilder() + .setName(fieldName) + .setTypeName(scope) + .setLabel((FieldDescriptorProto.Label) BQTableSchemaModeMap.get(mode)) + .setNumber(index) + .build(); + } return FieldDescriptorProto.newBuilder() .setName(fieldName) - .setTypeName(scope) + .setType((FieldDescriptorProto.Type) BQTableSchemaTypeMap.get(BQTableField.getType())) .setLabel((FieldDescriptorProto.Label) BQTableSchemaModeMap.get(mode)) .setNumber(index) .build();