[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 <derekbailey@google.com>

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"

src/idl_gen_kotlin.cpp

@@ -709,10 +709,7 @@ class KotlinGenerator : public BaseGenerator {
     auto secondArg =
         ConvertCase(Esc(field.name), Case::kLowerCamel) + ": " + field_type;
 
-    GenerateFunOneLine(
+    auto content = [&]() {
-        writer, "add" + ConvertCase(Esc(field.name), Case::kUpperCamel),
-        "builder: FlatBufferBuilder, " + secondArg, "",
-        [&]() {
           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));
-
+          if (field.key) {
-          writer += "builder.add{{method_name}}({{pos}}, \\";
+              // field has key attribute, so always need to exist
-          writer += "{{field_name}}{{cast}}, {{default}})";
+              // even if its value is equal to default.
-        },
+              // Generated code will bypass default checking
-        options.gen_jvmstatic);
+              // 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) {

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
+        }
+    }
+}

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)
+    }
+}

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")

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)

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

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