Skip to content

Commit

Permalink
Fix according to PR, used case insensitive treeset for allowUnknownFi…
Browse files Browse the repository at this point in the history 
…elds check.
  • Loading branch information
@allenc3
allenc3 committed Jul 14, 2020
1 parent efef042 commit b6316ec
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 52 deletions.
65 changes: 28 additions & 37 deletions ...ystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@
import com.google.protobuf.DynamicMessage;
import com.google.protobuf.Message;
import java.util.HashMap;
import java.util.Map;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
Expand All @@ -40,9 +41,9 @@ public class JsonToProtoMessage {
public static DynamicMessage convertJsonToProtoMessage(
Descriptor protoSchema, JSONObject json, boolean allowUnknownFields)
throws IllegalArgumentException {
if (json.length() == 0) {
throw new IllegalArgumentException("JSONObject is empty.");
}
if (json.length() == 0) {
throw new IllegalArgumentException("JSONObject is empty.");
}
return convertJsonToProtoMessageImpl(protoSchema, json, "root", true, allowUnknownFields);
}

Expand All @@ -66,32 +67,37 @@ private static DynamicMessage convertJsonToProtoMessageImpl(

DynamicMessage.Builder protoMsg = DynamicMessage.newBuilder(protoSchema);
List<FieldDescriptor> protoFields = protoSchema.getFields();
HashMap<String, FieldDescriptor> protoLowercaseNameToString = new HashMap<String, FieldDescriptor>();

Set<String> protoFieldNames = new TreeSet<String>(String.CASE_INSENSITIVE_ORDER);
for (FieldDescriptor field : protoFields) {
protoLowercaseNameToString.put(field.getName().toLowerCase(), field);
protoFieldNames.add(field.getName());
}

HashMap<String, String> jsonLowercaseNameToActualName = new HashMap<String, String>();
String[] actualNames = JSONObject.getNames(json);
for (int i = 0; i < actualNames.length; i++) {
jsonLowercaseNameToActualName.put(actualNames[i].toLowerCase(), actualNames[i]);
HashMap<String, String> jsonLowercaseNameToName = new HashMap<String, String>();
String[] jsonNames = JSONObject.getNames(json);
for (int i = 0; i < jsonNames.length; i++) {
jsonLowercaseNameToName.put(jsonNames[i].toLowerCase(), jsonNames[i]);
}

if (!allowUnknownFields) {
for (String jsonLowercaseField : jsonLowercaseNameToActualName.keySet()) {
if (!protoLowercaseNameToString.containsKey(jsonLowercaseField)) {
for (int i = 0; i < jsonNames.length; i++) {
if (!protoFieldNames.contains(jsonNames[i])) {
throw new IllegalArgumentException(
"JSONObject has fields unknown to BigQuery: " + jsonScope + "." + jsonLowercaseNameToActualName.get(jsonLowercaseField) + ". Set allowUnknownFields to True to allow unknown fields.");
"JSONObject has fields unknown to BigQuery: "
+ jsonScope
+ "."
+ jsonNames[i]
+ ". Set allowUnknownFields to True to allow unknown fields.");
}
}
}

int matchedFields = 0;
for (Map.Entry<String, FieldDescriptor> protoEntry : protoLowercaseNameToString.entrySet()) {
String lowercaseFieldName = protoEntry.getKey();
FieldDescriptor field = protoEntry.getValue();
for (FieldDescriptor field : protoFields) {
String lowercaseFieldName = field.getName().toLowerCase();
String currentScope = jsonScope + "." + field.getName();

if (!jsonLowercaseNameToActualName.containsKey(lowercaseFieldName)) {
if (!jsonLowercaseNameToName.containsKey(lowercaseFieldName)) {
if (field.isRequired()) {
throw new IllegalArgumentException(
"JSONObject does not have the required field " + currentScope + ".");
Expand All @@ -105,15 +111,15 @@ private static DynamicMessage convertJsonToProtoMessageImpl(
protoMsg,
field,
json,
jsonLowercaseNameToActualName.get(lowercaseFieldName),
jsonLowercaseNameToName.get(lowercaseFieldName),
currentScope,
allowUnknownFields);
} else {
fillRepeatedField(
protoMsg,
field,
json,
jsonLowercaseNameToActualName.get(lowercaseFieldName),
jsonLowercaseNameToName.get(lowercaseFieldName),
currentScope,
allowUnknownFields);
}
Expand Down Expand Up @@ -283,12 +289,7 @@ private static void fillRepeatedField(
protoMsg.addRepeatedField(fieldDescriptor, new Long((Long) val));
} else {
throw new IllegalArgumentException(
"JSONObject does not have a int64 field at "
+ currentScope
+ "["
+ i
+ "]"
+ ".");
"JSONObject does not have a int64 field at " + currentScope + "[" + i + "]" + ".");
}
}
break;
Expand All @@ -299,12 +300,7 @@ private static void fillRepeatedField(
protoMsg.addRepeatedField(fieldDescriptor, new Integer((Integer) val));
} else {
throw new IllegalArgumentException(
"JSONObject does not have a int32 field at "
+ currentScope
+ "["
+ i
+ "]"
+ ".");
"JSONObject does not have a int32 field at " + currentScope + "[" + i + "]" + ".");
}
}
break;
Expand All @@ -327,12 +323,7 @@ private static void fillRepeatedField(
protoMsg.addRepeatedField(fieldDescriptor, new Double((float) val));
} else {
throw new IllegalArgumentException(
"JSONObject does not have a double field at "
+ currentScope
+ "["
+ i
+ "]"
+ ".");
"JSONObject does not have a double field at " + currentScope + "[" + i + "]" + ".");
}
}
break;
Expand Down
54 changes: 39 additions & 15 deletions ...rage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonToProtoMessageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,7 @@ public class JsonToProtoMessageTest {
(double) Float.MIN_VALUE
})),
new JSONObject()
.put(
"test_repeated",
new JSONArray(
new Float[] {
Float.MAX_VALUE,
Float.MIN_VALUE
})),
.put("test_repeated", new JSONArray(new Float[] {Float.MAX_VALUE, Float.MIN_VALUE})),
new JSONObject().put("test_repeated", new JSONArray(new Boolean[] {true, false})),
new JSONObject().put("test_repeated", new JSONArray(new String[] {"hello", "test"})),
new JSONObject()
Expand Down Expand Up @@ -329,7 +323,8 @@ public void testAllRepeatedTypesWithLimits() throws Exception {
+ " field at root.test_repeated[0].");
}
}
if (entry.getKey() == RepeatedInt64.getDescriptor() || entry.getKey() == RepeatedDouble.getDescriptor()) {
if (entry.getKey() == RepeatedInt64.getDescriptor()
|| entry.getKey() == RepeatedDouble.getDescriptor()) {
assertEquals(2, success);
} else {
assertEquals(1, success);
Expand All @@ -353,7 +348,8 @@ public void testRepeatedIsOptional() throws Exception {
json.put("required_double", 1.1);

DynamicMessage protoMsg =
JsonToProtoMessage.convertJsonToProtoMessage(TestRepeatedIsOptional.getDescriptor(), json, false);
JsonToProtoMessage.convertJsonToProtoMessage(
TestRepeatedIsOptional.getDescriptor(), json, false);
AreMatchingFieldsFilledIn(protoMsg, json);
}

Expand Down Expand Up @@ -516,15 +512,16 @@ public void testAllowUnknownFields() throws Exception {
@Test
public void testAllowUnknownFieldsError() throws Exception {
JSONObject json = new JSONObject();
json.put("double", 1.1);
json.put("test_repeated", new JSONArray(new int[] {1, 2, 3, 4, 5}));
json.put("string", "hello");

try {
DynamicMessage protoMsg =
JsonToProtoMessage.convertJsonToProtoMessage(RepeatedInt64.getDescriptor(), json, false);
} catch (IllegalArgumentException e) {
assertEquals(
e.getMessage(),
"JSONObject has fields unknown to BigQuery: root.double. Set allowUnknownFields to True to allow unknown fields.");
"JSONObject has fields unknown to BigQuery: root.string. Set allowUnknownFields to True to allow unknown fields.");
}
}

Expand All @@ -535,14 +532,12 @@ public void testEmptyJSONObject() throws Exception {
DynamicMessage protoMsg =
JsonToProtoMessage.convertJsonToProtoMessage(Int64Type.getDescriptor(), json, false);
} catch (IllegalArgumentException e) {
assertEquals(
e.getMessage(),
"JSONObject is empty.");
assertEquals(e.getMessage(), "JSONObject is empty.");
}
}

@Test
public void testTopLevelMatchSecondLevelNoMatch() throws Exception {
public void testAllowUnknownFieldsSecondLevel() throws Exception {
JSONObject complexLvl2 = new JSONObject();
complexLvl2.put("no_match", 1);
JSONObject json = new JSONObject();
Expand All @@ -559,6 +554,35 @@ public void testTopLevelMatchSecondLevelNoMatch() throws Exception {
}
}

@Test
public void testTopLevelMismatch() throws Exception {
JSONObject json = new JSONObject();
json.put("no_match", 1.1);

try {
DynamicMessage protoMsg =
JsonToProtoMessage.convertJsonToProtoMessage(
TopLevelMismatch.getDescriptor(), json, true);
} catch (IllegalArgumentException e) {
assertEquals(
e.getMessage(),
"There are no matching fields found for the JSONObject and the protocol buffer descriptor.");
}
}

@Test
public void testTopLevelMatchSecondLevelMismatch() throws Exception {
JSONObject complexLvl2 = new JSONObject();
complexLvl2.put("no_match", 1);
JSONObject json = new JSONObject();
json.put("test_int", 1);
json.put("complexLvl2", complexLvl2);

DynamicMessage protoMsg =
JsonToProtoMessage.convertJsonToProtoMessage(ComplexLvl1.getDescriptor(), json, true);
AreMatchingFieldsFilledIn(protoMsg, json);
}

@Test
public void testJsonNullValue() throws Exception {
JSONObject json = new JSONObject();
Expand Down
4 changes: 4 additions & 0 deletions google-cloud-bigquerystorage/src/test/proto/jsonTest.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,7 @@ message TestRepeatedIsOptional {
optional double required_double = 1;
repeated double repeated_double = 2;
}

message TopLevelMismatch {
optional double mismatch_double = 1;
}

0 comments on commit b6316ec

Please sign in to comment.