diff --git a/extensions/protobuf/src/main/java/io/deephaven/protobuf/ProtobufDescriptorParserImpl.java b/extensions/protobuf/src/main/java/io/deephaven/protobuf/ProtobufDescriptorParserImpl.java index a2365ea853e..91c4850e976 100644 --- a/extensions/protobuf/src/main/java/io/deephaven/protobuf/ProtobufDescriptorParserImpl.java +++ b/extensions/protobuf/src/main/java/io/deephaven/protobuf/ProtobufDescriptorParserImpl.java @@ -32,7 +32,6 @@ import io.deephaven.function.ToShortFunction; import io.deephaven.function.TypedFunction; import io.deephaven.function.TypedFunction.Visitor; -import io.deephaven.util.QueryConstants; import java.lang.reflect.Array; import java.util.HashMap; @@ -362,40 +361,46 @@ private ProtobufFunctions functions() { } } + private ToObjectFunction maybeBypass(ToObjectFunction f) { + // Ideally, we could be very targetted in our application of null checks; in a lot of contexts, our + // implementation could know it will never be called with a null message to produce an array. + return BypassOnNull.of(f); + } + private ToObjectFunction mapChars(ToCharFunction f) { - return ToObjectFunction.of(m -> toChars(m, fd, f), Type.charType().arrayType()); + return maybeBypass(ToObjectFunction.of(m -> toChars(m, fd, f), Type.charType().arrayType())); } private ToObjectFunction mapBytes(ToByteFunction f) { - return ToObjectFunction.of(m -> toBytes(m, fd, f), Type.byteType().arrayType()); + return maybeBypass(ToObjectFunction.of(m -> toBytes(m, fd, f), Type.byteType().arrayType())); } private ToObjectFunction mapShorts(ToShortFunction f) { - return ToObjectFunction.of(m -> toShorts(m, fd, f), Type.shortType().arrayType()); + return maybeBypass(ToObjectFunction.of(m -> toShorts(m, fd, f), Type.shortType().arrayType())); } private ToObjectFunction mapInts(ToIntFunction f) { - return ToObjectFunction.of(m -> toInts(m, fd, f), Type.intType().arrayType()); + return maybeBypass(ToObjectFunction.of(m -> toInts(m, fd, f), Type.intType().arrayType())); } private ToObjectFunction mapLongs(ToLongFunction f) { - return ToObjectFunction.of(m -> toLongs(m, fd, f), Type.longType().arrayType()); + return maybeBypass(ToObjectFunction.of(m -> toLongs(m, fd, f), Type.longType().arrayType())); } private ToObjectFunction mapFloats(ToFloatFunction f) { - return ToObjectFunction.of(m -> toFloats(m, fd, f), Type.floatType().arrayType()); + return maybeBypass(ToObjectFunction.of(m -> toFloats(m, fd, f), Type.floatType().arrayType())); } private ToObjectFunction mapDoubles(ToDoubleFunction f) { - return ToObjectFunction.of(m -> toDoubles(m, fd, f), Type.doubleType().arrayType()); + return maybeBypass(ToObjectFunction.of(m -> toDoubles(m, fd, f), Type.doubleType().arrayType())); } private ToObjectFunction mapBooleans(ToBooleanFunction f) { - return ToObjectFunction.of(m -> toBooleans(m, fd, f), Type.booleanType().arrayType()); + return maybeBypass(ToObjectFunction.of(m -> toBooleans(m, fd, f), Type.booleanType().arrayType())); } private ToObjectFunction mapGenerics(ToObjectFunction f) { - return ToObjectFunction.of(message -> toArray(message, fd, f), f.returnType().arrayType()); + return maybeBypass(ToObjectFunction.of(message -> toArray(message, fd, f), f.returnType().arrayType())); } private class ToRepeatedType implements diff --git a/extensions/protobuf/src/test/java/io/deephaven/protobuf/ProtobufDescriptorParserTest.java b/extensions/protobuf/src/test/java/io/deephaven/protobuf/ProtobufDescriptorParserTest.java index 975f13fa6c1..fd4e3a4e1e2 100644 --- a/extensions/protobuf/src/test/java/io/deephaven/protobuf/ProtobufDescriptorParserTest.java +++ b/extensions/protobuf/src/test/java/io/deephaven/protobuf/ProtobufDescriptorParserTest.java @@ -45,6 +45,7 @@ import io.deephaven.protobuf.test.ByteWrapperRepeated; import io.deephaven.protobuf.test.FieldMaskWrapper; import io.deephaven.protobuf.test.MultiRepeated; +import io.deephaven.protobuf.test.NestedArrays; import io.deephaven.protobuf.test.NestedByteWrapper; import io.deephaven.protobuf.test.NestedRepeatedTimestamps; import io.deephaven.protobuf.test.NestedRepeatedTimestamps.Timestamps; @@ -1470,6 +1471,111 @@ void twoTimestampsOneAsWellKnown() { assertThat(nf.keySet()).containsExactly(List.of("ts1"), List.of("ts2", "seconds"), List.of("ts2", "nanos")); } + @Test + void nestedArraysADirect() { + checkKey( + NestedArrays.getDescriptor(), + List.of("a_direct", "b", "c"), + Type.stringType().arrayType(), + new HashMap<>() { + { + put(NestedArrays.getDefaultInstance(), null); + + put(NestedArrays.newBuilder() + .setADirect(NestedArrays.A.getDefaultInstance()) + .build(), null); + + // c is only non-null when b has been explicitly set + + put(NestedArrays.newBuilder() + .setADirect(NestedArrays.A.newBuilder() + .setB(NestedArrays.B.getDefaultInstance()) + .build()) + .build(), new String[0]); + + put(NestedArrays.newBuilder() + .setADirect(NestedArrays.A.newBuilder() + .setB(NestedArrays.B.newBuilder() + .addC("Foo") + .addC("Bar") + .build()) + .build()) + .build(), new String[] {"Foo", "Bar"}); + } + }); + } + + @Test + void nestedArraysARepeated() { + checkKey( + NestedArrays.getDescriptor(), + List.of("a_repeated", "b", "c"), + Type.stringType().arrayType().arrayType(), + new HashMap<>() { + { + put(NestedArrays.getDefaultInstance(), new String[0][]); + put(NestedArrays.newBuilder() + .addARepeated(NestedArrays.A.getDefaultInstance()) + .addARepeated(NestedArrays.A.newBuilder() + .setB(NestedArrays.B.getDefaultInstance()) + .build()) + .addARepeated(NestedArrays.A.newBuilder() + .setB(NestedArrays.B.newBuilder() + .addC("Foo") + .addC("Bar") + .build()) + .build()) + .build(), new String[][] {null, new String[0], new String[] {"Foo", "Bar"}}); + } + }); + } + + @Test + void nestedArraysBDirect() { + checkKey( + NestedArrays.getDescriptor(), + List.of("b_direct", "c"), + Type.stringType().arrayType(), + new HashMap<>() { + { + put(NestedArrays.getDefaultInstance(), null); + + put(NestedArrays.newBuilder() + .setBDirect(NestedArrays.B.getDefaultInstance()) + .build(), new String[0]); + + put(NestedArrays.newBuilder() + .setBDirect(NestedArrays.B.newBuilder() + .addC("Foo") + .addC("Bar") + .build()) + .build(), new String[] {"Foo", "Bar"}); + } + }); + } + + @Test + void nestedArraysBRepeated() { + checkKey( + NestedArrays.getDescriptor(), + List.of("b_repeated", "c"), + Type.stringType().arrayType().arrayType(), + new HashMap<>() { + { + put(NestedArrays.getDefaultInstance(), new String[0][]); + + put(NestedArrays.newBuilder() + .addBRepeated(NestedArrays.B.getDefaultInstance()) + .addBRepeated(NestedArrays.B.newBuilder() + .addC("Foo") + .addC("Bar") + .build()) + + .build(), new String[][] {new String[0], new String[] {"Foo", "Bar"}}); + } + }); + } + private static Map, TypedFunction> nf(Descriptor descriptor) { return nf(descriptor, ProtobufDescriptorParserOptions.defaults()); } diff --git a/extensions/protobuf/src/test/proto/mytest.proto b/extensions/protobuf/src/test/proto/mytest.proto index da6a6e169db..46d6e2a9513 100644 --- a/extensions/protobuf/src/test/proto/mytest.proto +++ b/extensions/protobuf/src/test/proto/mytest.proto @@ -131,6 +131,21 @@ message RepeatedObject { repeated XYZ xyz = 1; } +message NestedArrays { + message A { + B b = 1; + } + message B { + repeated string c = 1; + } + + A a_direct = 1; + repeated A a_repeated = 2; + + B b_direct = 3; + repeated B b_repeated = 4; +} + message MultiRepeated { repeated RepeatedBasics my_basics = 1; repeated RepeatedWrappers my_wrappers = 2;