From b17d59b18c5a4409bf7c53e9503ebc8904a1e305 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Luis=20Mill=C3=A1n?= Date: Tue, 10 Jan 2023 19:43:17 +0100 Subject: [TS]: builder, Fix requiredField(). Verity that the field is present in the vtable (#7739) (#7752) * [TS]: Fix vtable creation for consecutive required fileds (#7739) * handle feedback * comment the schema * comment change in builder.ts * [TS]: builder, Fix requiredField() Verifty that the field is present in the vtable. * restore monsterdata binary file Co-authored-by: Derek Bailey --- tests/required_strings.fbs | 12 +++++++ tests/ts/JavaScriptRequiredStringTest.js | 32 +++++++++++++++++ tests/ts/TypeScriptTest.py | 3 +- tests/ts/required-strings/foo.js | 49 +++++++++++++++++++++++++ tests/ts/required-strings/foo.ts | 62 ++++++++++++++++++++++++++++++++ tests/ts/required_strings_generated.js | 2 ++ tests/ts/required_strings_generated.ts | 2 ++ ts/builder.ts | 3 +- 8 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 tests/required_strings.fbs create mode 100644 tests/ts/JavaScriptRequiredStringTest.js create mode 100644 tests/ts/required-strings/foo.js create mode 100644 tests/ts/required-strings/foo.ts create mode 100644 tests/ts/required_strings_generated.js create mode 100644 tests/ts/required_strings_generated.ts diff --git a/tests/required_strings.fbs b/tests/required_strings.fbs new file mode 100644 index 00000000..98556d4f --- /dev/null +++ b/tests/required_strings.fbs @@ -0,0 +1,12 @@ +namespace required_strings; + +/** + * Foo defines a type where both fields are mandatory. + * The creation of a Foo buffer must throw if either of the fields is missing. + * + * https://github.com/google/flatbuffers/issues/7739 + */ +table Foo { + str_a:string (required); + str_b:string (required); +} diff --git a/tests/ts/JavaScriptRequiredStringTest.js b/tests/ts/JavaScriptRequiredStringTest.js new file mode 100644 index 00000000..6023ef80 --- /dev/null +++ b/tests/ts/JavaScriptRequiredStringTest.js @@ -0,0 +1,32 @@ +import assert from 'assert' +import * as flatbuffers from 'flatbuffers'; +import { Foo } from './required-strings/foo.js'; + + +var builder = new flatbuffers.Builder(); + +function main() { + testMissingFirstRequiredString(); + builder.clear(); + testMissingSecondRequiredString(); +} + +function testMissingFirstRequiredString() { + const undefined_string = builder.createString(undefined); + const defined_string = builder.createString('cat'); + + assert.throws(() => Foo.createFoo( + builder, undefined_string, defined_string + )); +} + +function testMissingSecondRequiredString() { + const defined_string = builder.createString('cat'); + const undefined_string = builder.createString(undefined); + + assert.throws(() => Foo.createFoo( + builder, defined_string, undefined_string + )); +} + +main(); diff --git a/tests/ts/TypeScriptTest.py b/tests/ts/TypeScriptTest.py index 4fe7ab65..4a4ccb2a 100755 --- a/tests/ts/TypeScriptTest.py +++ b/tests/ts/TypeScriptTest.py @@ -134,4 +134,5 @@ print("Running TypeScript Tests...") check_call(NODE_CMD + ["JavaScriptTest"]) check_call(NODE_CMD + ["JavaScriptUnionVectorTest"]) check_call(NODE_CMD + ["JavaScriptFlexBuffersTest"]) -check_call(NODE_CMD + ["JavaScriptComplexArraysTest"]) \ No newline at end of file +check_call(NODE_CMD + ["JavaScriptComplexArraysTest"]) +check_call(NODE_CMD + ["JavaScriptRequiredStringTest"]) diff --git a/tests/ts/required-strings/foo.js b/tests/ts/required-strings/foo.js new file mode 100644 index 00000000..774fcca1 --- /dev/null +++ b/tests/ts/required-strings/foo.js @@ -0,0 +1,49 @@ +// automatically generated by the FlatBuffers compiler, do not modify +import * as flatbuffers from 'flatbuffers'; +export class Foo { + constructor() { + this.bb = null; + this.bb_pos = 0; + } + __init(i, bb) { + this.bb_pos = i; + this.bb = bb; + return this; + } + static getRootAsFoo(bb, obj) { + return (obj || new Foo()).__init(bb.readInt32(bb.position()) + bb.position(), bb); + } + static getSizePrefixedRootAsFoo(bb, obj) { + bb.setPosition(bb.position() + flatbuffers.SIZE_PREFIX_LENGTH); + return (obj || new Foo()).__init(bb.readInt32(bb.position()) + bb.position(), bb); + } + strA(optionalEncoding) { + const offset = this.bb.__offset(this.bb_pos, 4); + return offset ? this.bb.__string(this.bb_pos + offset, optionalEncoding) : null; + } + strB(optionalEncoding) { + const offset = this.bb.__offset(this.bb_pos, 6); + return offset ? this.bb.__string(this.bb_pos + offset, optionalEncoding) : null; + } + static startFoo(builder) { + builder.startObject(2); + } + static addStrA(builder, strAOffset) { + builder.addFieldOffset(0, strAOffset, 0); + } + static addStrB(builder, strBOffset) { + builder.addFieldOffset(1, strBOffset, 0); + } + static endFoo(builder) { + const offset = builder.endObject(); + builder.requiredField(offset, 4); // str_a + builder.requiredField(offset, 6); // str_b + return offset; + } + static createFoo(builder, strAOffset, strBOffset) { + Foo.startFoo(builder); + Foo.addStrA(builder, strAOffset); + Foo.addStrB(builder, strBOffset); + return Foo.endFoo(builder); + } +} diff --git a/tests/ts/required-strings/foo.ts b/tests/ts/required-strings/foo.ts new file mode 100644 index 00000000..8ae6666c --- /dev/null +++ b/tests/ts/required-strings/foo.ts @@ -0,0 +1,62 @@ +// automatically generated by the FlatBuffers compiler, do not modify + +import * as flatbuffers from 'flatbuffers'; + +export class Foo { + bb: flatbuffers.ByteBuffer|null = null; + bb_pos = 0; + __init(i:number, bb:flatbuffers.ByteBuffer):Foo { + this.bb_pos = i; + this.bb = bb; + return this; +} + +static getRootAsFoo(bb:flatbuffers.ByteBuffer, obj?:Foo):Foo { + return (obj || new Foo()).__init(bb.readInt32(bb.position()) + bb.position(), bb); +} + +static getSizePrefixedRootAsFoo(bb:flatbuffers.ByteBuffer, obj?:Foo):Foo { + bb.setPosition(bb.position() + flatbuffers.SIZE_PREFIX_LENGTH); + return (obj || new Foo()).__init(bb.readInt32(bb.position()) + bb.position(), bb); +} + +strA():string|null +strA(optionalEncoding:flatbuffers.Encoding):string|Uint8Array|null +strA(optionalEncoding?:any):string|Uint8Array|null { + const offset = this.bb!.__offset(this.bb_pos, 4); + return offset ? this.bb!.__string(this.bb_pos + offset, optionalEncoding) : null; +} + +strB():string|null +strB(optionalEncoding:flatbuffers.Encoding):string|Uint8Array|null +strB(optionalEncoding?:any):string|Uint8Array|null { + const offset = this.bb!.__offset(this.bb_pos, 6); + return offset ? this.bb!.__string(this.bb_pos + offset, optionalEncoding) : null; +} + +static startFoo(builder:flatbuffers.Builder) { + builder.startObject(2); +} + +static addStrA(builder:flatbuffers.Builder, strAOffset:flatbuffers.Offset) { + builder.addFieldOffset(0, strAOffset, 0); +} + +static addStrB(builder:flatbuffers.Builder, strBOffset:flatbuffers.Offset) { + builder.addFieldOffset(1, strBOffset, 0); +} + +static endFoo(builder:flatbuffers.Builder):flatbuffers.Offset { + const offset = builder.endObject(); + builder.requiredField(offset, 4) // str_a + builder.requiredField(offset, 6) // str_b + return offset; +} + +static createFoo(builder:flatbuffers.Builder, strAOffset:flatbuffers.Offset, strBOffset:flatbuffers.Offset):flatbuffers.Offset { + Foo.startFoo(builder); + Foo.addStrA(builder, strAOffset); + Foo.addStrB(builder, strBOffset); + return Foo.endFoo(builder); +} +} diff --git a/tests/ts/required_strings_generated.js b/tests/ts/required_strings_generated.js new file mode 100644 index 00000000..9f9cf017 --- /dev/null +++ b/tests/ts/required_strings_generated.js @@ -0,0 +1,2 @@ +"use strict"; +// automatically generated by the FlatBuffers compiler, do not modify diff --git a/tests/ts/required_strings_generated.ts b/tests/ts/required_strings_generated.ts new file mode 100644 index 00000000..b7d545f8 --- /dev/null +++ b/tests/ts/required_strings_generated.ts @@ -0,0 +1,2 @@ +// automatically generated by the FlatBuffers compiler, do not modify + diff --git a/ts/builder.ts b/ts/builder.ts index f1a2b419..4ba34035 100644 --- a/ts/builder.ts +++ b/ts/builder.ts @@ -458,7 +458,8 @@ export class Builder { requiredField(table: Offset, field: number): void { const table_start = this.bb.capacity() - table; const vtable_start = table_start - this.bb.readInt32(table_start); - const ok = this.bb.readInt16(vtable_start + field) != 0; + const ok = field < this.bb.readInt16(vtable_start) && + this.bb.readInt16(vtable_start + field) != 0; // If this fails, the caller will show what field needs to be set. if (!ok) { -- cgit v1.2.3