sequence may not handle empty lists correctly

[ctrychta]

When using sequence to read fields I've found that it sometime does not correctly handle the case where a list is empty, which can lead to subsequent fields holding incorrect data.

For example this json:

{"field1":1234,"field2":456,"field3":[ ], "field4":[ "abcdef","xyz" ]}

may be parsed as:

Foo{field1=1234, field2=456, field3=[], field4=[field4]}

This seems to be due to not consuming the comma after the empty sequence. Below is a code snippet showing this behavior. You can see the difference in behavior when uncommenting ((JSONWire) wire).consumePadding(1);.

class ExampleTest{
    @Test
    void emptySequence() {
        final byte[] json =
                "{\"field1\":1234,\"field2\":456,\"field3\":[ ], \"field4\":[ \"abcdef\",\"xyz\" ]}".getBytes(StandardCharsets.US_ASCII);

        final Bytes<byte[]> data = Bytes.wrapForRead(json);
        final JSONWire wire = new JSONWire(data, true);

        final Foo f = new Foo();
        wire.getValueIn().object(f, Foo.class);

        System.out.println(f);

        assertEquals("Foo{field1=1234, field2=456, field3=[], field4=[abcdef, xyz]}", f.toString());
    }

    private static final class Foo implements Marshallable {
        int field1;

        int field2;

        final List<StringBuilder> field3 = new ArrayList<>();

        final List<StringBuilder> field4 = new ArrayList<>();

        @Override
        public void readMarshallable(@NotNull final WireIn wire) throws IORuntimeException {
            wire.read(() -> "field1").int32(this, (self, n) -> self.field1 = n);
            wire.read(() -> "field2").int32(this, (self, n) -> self.field2 = n);
            readSequenceHandleEmpty(wire, () -> "field3",
                    this, field3, Foo::readList);
            readSequenceHandleEmpty(wire, () -> "field4",
                    this, field4, Foo::readList);
        }

        private static WireIn readSequenceHandleEmpty(final WireIn wire, final WireKey name,
                                                      final Foo record, final List<StringBuilder> data,
                                                      final TriConsumer<Foo, List<StringBuilder>, ValueIn> tReader) {
            final ValueIn vin = wire.read(name);
            vin.sequence(record, data, tReader);
            // Uncomment to consume a comma after the sequence
            //((JSONWire) wire).consumePadding(1);
            return wire;
        }

        private static void readList(final Foo record, final List<StringBuilder> data,
                                     final ValueIn reader) {
            while (reader.hasNextSequenceItem()) {
                data.add(new StringBuilder());
                reader.text(data.get(data.size() - 1));
            }
        }

        @Override
        public String toString() {
            return "Foo{" +
                    "field1=" + field1 +
                    ", field2=" + field2 +
                    ", field3=" + field3 +
                    ", field4=" + field4 +
                    '}';
        }
    }
}

[RobAustin]

I made a few changes but it looks ok to me:

package net.openhft.chronicle.wire;

import net.openhft.chronicle.bytes.Bytes;
import net.openhft.chronicle.core.pool.ClassAliasPool;
import org.junit.Assert;
import org.junit.Test;
import java.util.ArrayList;
import java.util.List;

public class JSONEmptySequences {
    @Test
    public void emptySequence() {
        ClassAliasPool.CLASS_ALIASES.addAlias(Foo.class);

        final Bytes data = Bytes.elasticByteBuffer();
        data.append("!Foo {\n" +
                "  field1: 1234,\n" +
                "  field2: 456,\n" +
                "  field3: [ ],\n" +
                "  field4: [\n" +
                "    abc,\n" +
                "    xyz\n" +
                "  ]\n" +
                "}");

        final JSONWire wire = new JSONWire(data);
        final Foo f = new Foo();
        wire.getValueIn().object(f, Foo.class);

        Assert.assertEquals("!Foo {\n" +
                "  field1: 1234,\n" +
                "  field2: 456,\n" +
                "  field3: [ ],\n" +
                "  field4: [\n" +
                "    abc,\n" +
                "    xyz\n" +
                "  ]\n" +
                "}\n", f.toString());
    }

    private static final class Foo extends SelfDescribingMarshallable {
        int field1;
        int field2;
        final List<String> field3 = new ArrayList<>();
        final List<String> field4 = new ArrayList<>();
    }
}

[RobAustin]

or if you want to put quotes around the field name, like this

package net.openhft.chronicle.wire;

import net.openhft.chronicle.bytes.Bytes;
import net.openhft.chronicle.core.pool.ClassAliasPool;
import org.junit.Assert;
import org.junit.Test;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class JSONEmptySequences {
    @Test
    public void emptySequence() {
        ClassAliasPool.CLASS_ALIASES.addAlias(Foo.class);

        final Bytes data = Bytes.elasticByteBuffer();
        data.append("!Foo {\"field1\":1234,\"field2\":456,\"field3\":[ ],\"field4\":[\"abc\",\"xyz\" ]}");

        final Wire wire = WireType.JSON.apply(data);
        Foo f = (Foo) wire.getValueIn().object();

        Assert.assertEquals("!Foo {\n" +
                "  field1: 1234,\n" +
                "  field2: 456,\n" +
                "  field3: [ ],\n" +
                "  field4: [\n" +
                "    abc,\n" +
                "    xyz\n" +
                "  ]\n" +
                "}\n", f.toString());
    }

    private static final class Foo extends SelfDescribingMarshallable {
        int field1;
        int field2;
        final List<String> field3 = new ArrayList<>();
        final List<String> field4 = new ArrayList<>();
    }
}

[alamar]

I can reproduce the issue - it appears to be trickier and needing a wire.read().sequence() with handler