From 7b5fd2bd0530b38f314b0ef3d04ec76a9703e291 Mon Sep 17 00:00:00 2001 From: Paulo Pinheiro Date: Tue, 12 Apr 2022 02:17:19 +0200 Subject: [PATCH] [Kotlin] Fix key lookup returning null clashing with default value (#7237) * [Java] Fix key lookup returning null clashing with default value A field with key attribute must always be written on the message so it can be looked up by key. There is a edge case where inserting a key field with same value as default would prevent it to be written on the message and later cannot be found when searched by key. * [Kotlin] Fix key lookup returning null clashing with default value A field with key attribute must always be written on the message so it can be looked up by key. There is a edge case where inserting a key field with same value as default would prevent it to be written on the message and later cannot be found when searched by key. Co-authored-by: Derek Bailey --- scripts/generate_code.py | 2 +- src/idl_gen_kotlin.cpp | 29 ++++++--- tests/DictionaryLookup/LongFloatEntry.kt | 80 ++++++++++++++++++++++++ tests/DictionaryLookup/LongFloatMap.kt | 77 +++++++++++++++++++++++ tests/KotlinTest.kt | 21 +++++++ tests/MyGame/Example/Monster.kt | 5 +- tests/MyGame/Example/Referrable.kt | 5 +- tests/MyGame/Example/Stat.kt | 5 +- 8 files changed, 211 insertions(+), 13 deletions(-) create mode 100644 tests/DictionaryLookup/LongFloatEntry.kt create mode 100644 tests/DictionaryLookup/LongFloatMap.kt diff --git a/scripts/generate_code.py b/scripts/generate_code.py index b90dd02be..939b7c0bb 100755 --- a/scripts/generate_code.py +++ b/scripts/generate_code.py @@ -364,7 +364,7 @@ flatc(BASE_OPTS + DART_OPTS, prefix="../dart/test/", schema="keyword_test.fbs") # Field key lookup with default value test dictionary_lookup_schema = "dictionary_lookup.fbs" -flatc(["--java"], schema=dictionary_lookup_schema) +flatc(["--java", "--kotlin"], schema=dictionary_lookup_schema) # Swift Tests swift_prefix = "FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests" diff --git a/src/idl_gen_kotlin.cpp b/src/idl_gen_kotlin.cpp index fc55aebd8..da5aec0c5 100644 --- a/src/idl_gen_kotlin.cpp +++ b/src/idl_gen_kotlin.cpp @@ -709,10 +709,7 @@ class KotlinGenerator : public BaseGenerator { auto secondArg = ConvertCase(Esc(field.name), Case::kLowerCamel) + ": " + field_type; - GenerateFunOneLine( - writer, "add" + ConvertCase(Esc(field.name), Case::kUpperCamel), - "builder: FlatBufferBuilder, " + secondArg, "", - [&]() { + auto content = [&]() { auto method = GenMethod(field.value.type); writer.SetValue("field_name", ConvertCase(Esc(field.name), Case::kLowerCamel)); @@ -720,11 +717,25 @@ class KotlinGenerator : public BaseGenerator { writer.SetValue("pos", field_pos); writer.SetValue("default", GenFBBDefaultValue(field)); writer.SetValue("cast", GenFBBValueCast(field)); - - writer += "builder.add{{method_name}}({{pos}}, \\"; - writer += "{{field_name}}{{cast}}, {{default}})"; - }, - options.gen_jvmstatic); + if (field.key) { + // field has key attribute, so always need to exist + // even if its value is equal to default. + // Generated code will bypass default checking + // resulting in { builder.addShort(name); slot(id); } + writer += "builder.add{{method_name}}({{field_name}}{{cast}})"; + writer += "builder.slot({{pos}})"; + } else { + writer += "builder.add{{method_name}}({{pos}}, \\"; + writer += "{{field_name}}{{cast}}, {{default}})"; + } + }; + auto signature = "add" + ConvertCase(Esc(field.name), Case::kUpperCamel); + auto params = "builder: FlatBufferBuilder, " + secondArg; + if (field.key) { + GenerateFun(writer,signature, params, "", content, options.gen_jvmstatic); + } else { + GenerateFunOneLine(writer, signature, params, "", content, options.gen_jvmstatic); + } } static std::string ToSignedType(const Type &type) { diff --git a/tests/DictionaryLookup/LongFloatEntry.kt b/tests/DictionaryLookup/LongFloatEntry.kt new file mode 100644 index 000000000..7e6046e84 --- /dev/null +++ b/tests/DictionaryLookup/LongFloatEntry.kt @@ -0,0 +1,80 @@ +// automatically generated by the FlatBuffers compiler, do not modify + +package DictionaryLookup + +import java.nio.* +import kotlin.math.sign +import com.google.flatbuffers.* + +@Suppress("unused") +class LongFloatEntry : Table() { + + fun __init(_i: Int, _bb: ByteBuffer) { + __reset(_i, _bb) + } + fun __assign(_i: Int, _bb: ByteBuffer) : LongFloatEntry { + __init(_i, _bb) + return this + } + val key : Long + get() { + val o = __offset(4) + return if(o != 0) bb.getLong(o + bb_pos) else 0L + } + val value : Float + get() { + val o = __offset(6) + return if(o != 0) bb.getFloat(o + bb_pos) else 0.0f + } + override fun keysCompare(o1: Int, o2: Int, _bb: ByteBuffer) : Int { + val val_1 = _bb.getLong(__offset(4, o1, _bb)) + val val_2 = _bb.getLong(__offset(4, o2, _bb)) + return (val_1 - val_2).sign + } + companion object { + fun validateVersion() = Constants.FLATBUFFERS_2_0_0() + fun getRootAsLongFloatEntry(_bb: ByteBuffer): LongFloatEntry = getRootAsLongFloatEntry(_bb, LongFloatEntry()) + fun getRootAsLongFloatEntry(_bb: ByteBuffer, obj: LongFloatEntry): LongFloatEntry { + _bb.order(ByteOrder.LITTLE_ENDIAN) + return (obj.__assign(_bb.getInt(_bb.position()) + _bb.position(), _bb)) + } + fun createLongFloatEntry(builder: FlatBufferBuilder, key: Long, value: Float) : Int { + builder.startTable(2) + addKey(builder, key) + addValue(builder, value) + return endLongFloatEntry(builder) + } + fun startLongFloatEntry(builder: FlatBufferBuilder) = builder.startTable(2) + fun addKey(builder: FlatBufferBuilder, key: Long) { + builder.addLong(key) + builder.slot(0) + } + fun addValue(builder: FlatBufferBuilder, value: Float) = builder.addFloat(1, value, 0.0) + fun endLongFloatEntry(builder: FlatBufferBuilder) : Int { + val o = builder.endTable() + return o + } + fun __lookup_by_key(obj: LongFloatEntry?, vectorLocation: Int, key: Long, bb: ByteBuffer) : LongFloatEntry? { + var span = bb.getInt(vectorLocation - 4) + var start = 0 + while (span != 0) { + var middle = span / 2 + val tableOffset = __indirect(vectorLocation + 4 * (start + middle), bb) + val value = bb.getLong(__offset(4, bb.capacity() - tableOffset, bb)) + val comp = value.compareTo(key) + when { + comp > 0 -> span = middle + comp < 0 -> { + middle++ + start += middle + span -= middle + } + else -> { + return (obj ?: LongFloatEntry()).__assign(tableOffset, bb) + } + } + } + return null + } + } +} diff --git a/tests/DictionaryLookup/LongFloatMap.kt b/tests/DictionaryLookup/LongFloatMap.kt new file mode 100644 index 000000000..0019c5fd8 --- /dev/null +++ b/tests/DictionaryLookup/LongFloatMap.kt @@ -0,0 +1,77 @@ +// automatically generated by the FlatBuffers compiler, do not modify + +package DictionaryLookup + +import java.nio.* +import kotlin.math.sign +import com.google.flatbuffers.* + +@Suppress("unused") +class LongFloatMap : Table() { + + fun __init(_i: Int, _bb: ByteBuffer) { + __reset(_i, _bb) + } + fun __assign(_i: Int, _bb: ByteBuffer) : LongFloatMap { + __init(_i, _bb) + return this + } + fun entries(j: Int) : DictionaryLookup.LongFloatEntry? = entries(DictionaryLookup.LongFloatEntry(), j) + fun entries(obj: DictionaryLookup.LongFloatEntry, j: Int) : DictionaryLookup.LongFloatEntry? { + val o = __offset(4) + return if (o != 0) { + obj.__assign(__indirect(__vector(o) + j * 4), bb) + } else { + null + } + } + val entriesLength : Int + get() { + val o = __offset(4); return if (o != 0) __vector_len(o) else 0 + } + fun entriesByKey(key: Long) : DictionaryLookup.LongFloatEntry? { + val o = __offset(4) + return if (o != 0) { + DictionaryLookup.LongFloatEntry.__lookup_by_key(null, __vector(o), key, bb) + } else { + null + } + } + fun entriesByKey(obj: DictionaryLookup.LongFloatEntry, key: Long) : DictionaryLookup.LongFloatEntry? { + val o = __offset(4) + return if (o != 0) { + DictionaryLookup.LongFloatEntry.__lookup_by_key(obj, __vector(o), key, bb) + } else { + null + } + } + companion object { + fun validateVersion() = Constants.FLATBUFFERS_2_0_0() + fun getRootAsLongFloatMap(_bb: ByteBuffer): LongFloatMap = getRootAsLongFloatMap(_bb, LongFloatMap()) + fun getRootAsLongFloatMap(_bb: ByteBuffer, obj: LongFloatMap): LongFloatMap { + _bb.order(ByteOrder.LITTLE_ENDIAN) + return (obj.__assign(_bb.getInt(_bb.position()) + _bb.position(), _bb)) + } + fun createLongFloatMap(builder: FlatBufferBuilder, entriesOffset: Int) : Int { + builder.startTable(1) + addEntries(builder, entriesOffset) + return endLongFloatMap(builder) + } + fun startLongFloatMap(builder: FlatBufferBuilder) = builder.startTable(1) + fun addEntries(builder: FlatBufferBuilder, entries: Int) = builder.addOffset(0, entries, 0) + fun createEntriesVector(builder: FlatBufferBuilder, data: IntArray) : Int { + builder.startVector(4, data.size, 4) + for (i in data.size - 1 downTo 0) { + builder.addOffset(data[i]) + } + return builder.endVector() + } + fun startEntriesVector(builder: FlatBufferBuilder, numElems: Int) = builder.startVector(4, numElems, 4) + fun endLongFloatMap(builder: FlatBufferBuilder) : Int { + val o = builder.endTable() + return o + } + fun finishLongFloatMapBuffer(builder: FlatBufferBuilder, offset: Int) = builder.finish(offset) + fun finishSizePrefixedLongFloatMapBuffer(builder: FlatBufferBuilder, offset: Int) = builder.finishSizePrefixed(offset) + } +} diff --git a/tests/KotlinTest.kt b/tests/KotlinTest.kt index cfb7056f6..9a8fe5fcd 100644 --- a/tests/KotlinTest.kt +++ b/tests/KotlinTest.kt @@ -14,6 +14,7 @@ * limitations under the License. */ +import DictionaryLookup.*; import MyGame.Example.* import optional_scalars.* import com.google.flatbuffers.ByteBufferUtil @@ -79,9 +80,29 @@ class KotlinTest { TestSharedStringPool() TestScalarOptional() + TestDictionaryLookup() println("FlatBuffers test: completed successfully") } + fun TestDictionaryLookup() { + val fbb = FlatBufferBuilder(16) + val lfIndex = LongFloatEntry.createLongFloatEntry(fbb, 0, 99.0f) + val vectorEntriesIdx = LongFloatMap.createEntriesVector(fbb, intArrayOf(lfIndex)) + val rootIdx = LongFloatMap.createLongFloatMap(fbb, vectorEntriesIdx) + + LongFloatMap.finishLongFloatMapBuffer(fbb, rootIdx) + val map = LongFloatMap.getRootAsLongFloatMap(fbb.dataBuffer()) + assert(map.entriesLength == 1) + + val e = map.entries(0)!! + assert(e.key == 0L) + assert(e.value == 99.0f) + + val e2 = map.entriesByKey(0)!! + assert(e2.key == 0L) + assert(e2.value == 99.0f) + } + fun TestEnums() { assert(Color.name(Color.Red.toInt()) == "Red") assert(Color.name(Color.Blue.toInt()) == "Blue") diff --git a/tests/MyGame/Example/Monster.kt b/tests/MyGame/Example/Monster.kt index 1abe05c05..5c372f91e 100644 --- a/tests/MyGame/Example/Monster.kt +++ b/tests/MyGame/Example/Monster.kt @@ -885,7 +885,10 @@ class Monster : Table() { fun addPos(builder: FlatBufferBuilder, pos: Int) = builder.addStruct(0, pos, 0) fun addMana(builder: FlatBufferBuilder, mana: Short) = builder.addShort(1, mana, 150) fun addHp(builder: FlatBufferBuilder, hp: Short) = builder.addShort(2, hp, 100) - fun addName(builder: FlatBufferBuilder, name: Int) = builder.addOffset(3, name, 0) + fun addName(builder: FlatBufferBuilder, name: Int) { + builder.addOffset(name) + builder.slot(3) + } fun addInventory(builder: FlatBufferBuilder, inventory: Int) = builder.addOffset(5, inventory, 0) fun createInventoryVector(builder: FlatBufferBuilder, data: UByteArray) : Int { builder.startVector(1, data.size, 1) diff --git a/tests/MyGame/Example/Referrable.kt b/tests/MyGame/Example/Referrable.kt index 7f728a7aa..10ccb1f70 100644 --- a/tests/MyGame/Example/Referrable.kt +++ b/tests/MyGame/Example/Referrable.kt @@ -48,7 +48,10 @@ class Referrable : Table() { return endReferrable(builder) } fun startReferrable(builder: FlatBufferBuilder) = builder.startTable(1) - fun addId(builder: FlatBufferBuilder, id: ULong) = builder.addLong(0, id.toLong(), 0) + fun addId(builder: FlatBufferBuilder, id: ULong) { + builder.addLong(id.toLong()) + builder.slot(0) + } fun endReferrable(builder: FlatBufferBuilder) : Int { val o = builder.endTable() return o diff --git a/tests/MyGame/Example/Stat.kt b/tests/MyGame/Example/Stat.kt index e6cc94cf0..83ad8f227 100644 --- a/tests/MyGame/Example/Stat.kt +++ b/tests/MyGame/Example/Stat.kt @@ -73,7 +73,10 @@ class Stat : Table() { fun startStat(builder: FlatBufferBuilder) = builder.startTable(3) fun addId(builder: FlatBufferBuilder, id: Int) = builder.addOffset(0, id, 0) fun addVal_(builder: FlatBufferBuilder, val_: Long) = builder.addLong(1, val_, 0L) - fun addCount(builder: FlatBufferBuilder, count: UShort) = builder.addShort(2, count.toShort(), 0) + fun addCount(builder: FlatBufferBuilder, count: UShort) { + builder.addShort(count.toShort()) + builder.slot(2) + } fun endStat(builder: FlatBufferBuilder) : Int { val o = builder.endTable() return o