[Java] Fix key lookup returning null clashing with default value (#7236)

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.

scripts/generate_code.py

@@ -362,6 +362,10 @@ flatc(
 )
 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)
+
 # Swift Tests
 swift_prefix = "FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests"
 flatc(

src/idl_gen_java.cpp

@@ -1091,7 +1091,7 @@ class JavaGenerator : public BaseGenerator {
            it != struct_def.fields.vec.end(); ++it) {
         auto &field = **it;
         if (field.deprecated) continue;
-        if (field.key) key_field = &field;
+
         code += "  public static void " + namer_.Method("add", field);
         code += "(FlatBufferBuilder builder, ";
         code += GenTypeBasic(DestinationType(field.value.type, false));
@@ -1099,13 +1099,25 @@ class JavaGenerator : public BaseGenerator {
         if (!IsScalar(field.value.type.base_type)) argname += "Offset";
         code += " " + argname + ") { builder.add";
         code += GenMethod(field.value.type) + "(";
-        code += NumToString(it - struct_def.fields.vec.begin()) + ", ";
-        code += SourceCastBasic(field.value.type);
-        code += argname;
-        code += ", ";
-        code += SourceCastBasic(field.value.type);
-        code += GenDefaultValue(field);
-        code += "); }\n";
+
+        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); }
+          key_field = &field;
+          code += SourceCastBasic(field.value.type);
+          code += argname;
+          code += "); builder.slot(" + NumToString(it - struct_def.fields.vec.begin()) + "); }\n";
+        } else {
+          code += NumToString(it - struct_def.fields.vec.begin()) + ", ";
+          code += SourceCastBasic(field.value.type);
+          code += argname;
+          code += ", ";
+          code += SourceCastBasic(field.value.type);
+          code += GenDefaultValue(field);
+          code += "); }\n";
+        }
         if (IsVector(field.value.type)) {
           auto vector_type = field.value.type.VectorType();
           auto alignment = InlineAlignment(vector_type);

tests/DictionaryLookup/LongFloatEntry.java

@@ -0,0 +1,75 @@
+// automatically generated by the FlatBuffers compiler, do not modify
+
+package DictionaryLookup;
+
+import java.nio.*;
+import java.lang.*;
+import java.util.*;
+import com.google.flatbuffers.*;
+
+@SuppressWarnings("unused")
+public final class LongFloatEntry extends Table {
+  public static void ValidateVersion() { Constants.FLATBUFFERS_2_0_0(); }
+  public static LongFloatEntry getRootAsLongFloatEntry(ByteBuffer _bb) { return getRootAsLongFloatEntry(_bb, new LongFloatEntry()); }
+  public static LongFloatEntry getRootAsLongFloatEntry(ByteBuffer _bb, LongFloatEntry obj) { _bb.order(ByteOrder.LITTLE_ENDIAN); return (obj.__assign(_bb.getInt(_bb.position()) + _bb.position(), _bb)); }
+  public void __init(int _i, ByteBuffer _bb) { __reset(_i, _bb); }
+  public LongFloatEntry __assign(int _i, ByteBuffer _bb) { __init(_i, _bb); return this; }
+
+  public long key() { int o = __offset(4); return o != 0 ? bb.getLong(o + bb_pos) : 0L; }
+  public float value() { int o = __offset(6); return o != 0 ? bb.getFloat(o + bb_pos) : 0.0f; }
+
+  public static int createLongFloatEntry(FlatBufferBuilder builder,
+      long key,
+      float value) {
+    builder.startTable(2);
+    LongFloatEntry.addKey(builder, key);
+    LongFloatEntry.addValue(builder, value);
+    return LongFloatEntry.endLongFloatEntry(builder);
+  }
+
+  public static void startLongFloatEntry(FlatBufferBuilder builder) { builder.startTable(2); }
+  public static void addKey(FlatBufferBuilder builder, long key) { builder.addLong(key); builder.slot(0); }
+  public static void addValue(FlatBufferBuilder builder, float value) { builder.addFloat(1, value, 0.0f); }
+  public static int endLongFloatEntry(FlatBufferBuilder builder) {
+    int o = builder.endTable();
+    return o;
+  }
+
+  @Override
+  protected int keysCompare(Integer o1, Integer o2, ByteBuffer _bb) {
+    long val_1 = _bb.getLong(__offset(4, o1, _bb));
+    long val_2 = _bb.getLong(__offset(4, o2, _bb));
+    return val_1 > val_2 ? 1 : val_1 < val_2 ? -1 : 0;
+  }
+
+  public static LongFloatEntry __lookup_by_key(LongFloatEntry obj, int vectorLocation, long key, ByteBuffer bb) {
+    int span = bb.getInt(vectorLocation - 4);
+    int start = 0;
+    while (span != 0) {
+      int middle = span / 2;
+      int tableOffset = __indirect(vectorLocation + 4 * (start + middle), bb);
+      long val = bb.getLong(__offset(4, bb.capacity() - tableOffset, bb));
+      int comp = val > key ? 1 : val < key ? -1 : 0;
+      if (comp > 0) {
+        span = middle;
+      } else if (comp < 0) {
+        middle++;
+        start += middle;
+        span -= middle;
+      } else {
+        return (obj == null ? new LongFloatEntry() : obj).__assign(tableOffset, bb);
+      }
+    }
+    return null;
+  }
+
+  public static final class Vector extends BaseVector {
+    public Vector __assign(int _vector, int _element_size, ByteBuffer _bb) { __reset(_vector, _element_size, _bb); return this; }
+
+    public LongFloatEntry get(int j) { return get(new LongFloatEntry(), j); }
+    public LongFloatEntry get(LongFloatEntry obj, int j) {  return obj.__assign(__indirect(__element(j), bb), bb); }
+    public LongFloatEntry getByKey(long key) {  return __lookup_by_key(null, __vector(), key, bb); }
+    public LongFloatEntry getByKey(LongFloatEntry obj, long key) {  return __lookup_by_key(obj, __vector(), key, bb); }
+  }
+}
+

tests/DictionaryLookup/LongFloatMap.java

@@ -0,0 +1,51 @@
+// automatically generated by the FlatBuffers compiler, do not modify
+
+package DictionaryLookup;
+
+import java.nio.*;
+import java.lang.*;
+import java.util.*;
+import com.google.flatbuffers.*;
+
+@SuppressWarnings("unused")
+public final class LongFloatMap extends Table {
+  public static void ValidateVersion() { Constants.FLATBUFFERS_2_0_0(); }
+  public static LongFloatMap getRootAsLongFloatMap(ByteBuffer _bb) { return getRootAsLongFloatMap(_bb, new LongFloatMap()); }
+  public static LongFloatMap getRootAsLongFloatMap(ByteBuffer _bb, LongFloatMap obj) { _bb.order(ByteOrder.LITTLE_ENDIAN); return (obj.__assign(_bb.getInt(_bb.position()) + _bb.position(), _bb)); }
+  public void __init(int _i, ByteBuffer _bb) { __reset(_i, _bb); }
+  public LongFloatMap __assign(int _i, ByteBuffer _bb) { __init(_i, _bb); return this; }
+
+  public DictionaryLookup.LongFloatEntry entries(int j) { return entries(new DictionaryLookup.LongFloatEntry(), j); }
+  public DictionaryLookup.LongFloatEntry entries(DictionaryLookup.LongFloatEntry obj, int j) { int o = __offset(4); return o != 0 ? obj.__assign(__indirect(__vector(o) + j * 4), bb) : null; }
+  public int entriesLength() { int o = __offset(4); return o != 0 ? __vector_len(o) : 0; }
+  public DictionaryLookup.LongFloatEntry entriesByKey(long key) { int o = __offset(4); return o != 0 ? DictionaryLookup.LongFloatEntry.__lookup_by_key(null, __vector(o), key, bb) : null; }
+  public DictionaryLookup.LongFloatEntry entriesByKey(DictionaryLookup.LongFloatEntry obj, long key) { int o = __offset(4); return o != 0 ? DictionaryLookup.LongFloatEntry.__lookup_by_key(obj, __vector(o), key, bb) : null; }
+  public DictionaryLookup.LongFloatEntry.Vector entriesVector() { return entriesVector(new DictionaryLookup.LongFloatEntry.Vector()); }
+  public DictionaryLookup.LongFloatEntry.Vector entriesVector(DictionaryLookup.LongFloatEntry.Vector obj) { int o = __offset(4); return o != 0 ? obj.__assign(__vector(o), 4, bb) : null; }
+
+  public static int createLongFloatMap(FlatBufferBuilder builder,
+      int entriesOffset) {
+    builder.startTable(1);
+    LongFloatMap.addEntries(builder, entriesOffset);
+    return LongFloatMap.endLongFloatMap(builder);
+  }
+
+  public static void startLongFloatMap(FlatBufferBuilder builder) { builder.startTable(1); }
+  public static void addEntries(FlatBufferBuilder builder, int entriesOffset) { builder.addOffset(0, entriesOffset, 0); }
+  public static int createEntriesVector(FlatBufferBuilder builder, int[] data) { builder.startVector(4, data.length, 4); for (int i = data.length - 1; i >= 0; i--) builder.addOffset(data[i]); return builder.endVector(); }
+  public static void startEntriesVector(FlatBufferBuilder builder, int numElems) { builder.startVector(4, numElems, 4); }
+  public static int endLongFloatMap(FlatBufferBuilder builder) {
+    int o = builder.endTable();
+    return o;
+  }
+  public static void finishLongFloatMapBuffer(FlatBufferBuilder builder, int offset) { builder.finish(offset); }
+  public static void finishSizePrefixedLongFloatMapBuffer(FlatBufferBuilder builder, int offset) { builder.finishSizePrefixed(offset); }
+
+  public static final class Vector extends BaseVector {
+    public Vector __assign(int _vector, int _element_size, ByteBuffer _bb) { __reset(_vector, _element_size, _bb); return this; }
+
+    public LongFloatMap get(int j) { return get(new LongFloatMap(), j); }
+    public LongFloatMap get(LongFloatMap obj, int j) {  return obj.__assign(__indirect(__element(j), bb), bb); }
+  }
+}
+

tests/JavaTest.java

@@ -1,6 +1,7 @@
 
 import static com.google.flatbuffers.Constants.*;
 
+import DictionaryLookup.*;
 import MyGame.Example.*;
 import optional_scalars.ScalarStuff;
 import optional_scalars.OptionalByte;
@@ -111,6 +112,8 @@ class JavaTest {
 
         TestPackUnpack(bb);
 
+        TestDictionaryLookup();
+
         System.out.println("FlatBuffers test: completed successfully");
     }
 
@@ -1148,6 +1151,25 @@ class JavaTest {
         testFlexBuffersMapLookup();
     }
 
+    static void TestDictionaryLookup() {
+        FlatBufferBuilder fbb = new FlatBufferBuilder(16);
+        int lfIndex = LongFloatEntry.createLongFloatEntry(fbb, 0, 99);
+        int vectorEntriesIdx = LongFloatMap.createEntriesVector(fbb, new int[] { lfIndex });
+        int rootIdx = LongFloatMap.createLongFloatMap(fbb, vectorEntriesIdx);
+
+        LongFloatMap.finishLongFloatMapBuffer(fbb, rootIdx);
+        LongFloatMap map = LongFloatMap.getRootAsLongFloatMap(fbb.dataBuffer());
+        TestEq(map.entriesLength(), 1);
+
+        LongFloatEntry e = map.entries(0);
+        TestEq(e.key(), 0L);
+        TestEq(e.value(), 99.0f);
+
+        LongFloatEntry e2 = map.entriesByKey(0);
+        TestEq(e2.key(), 0L);
+        TestEq(e2.value(), 99.0f);
+    }
+
     static void TestVectorOfBytes() {
         FlatBufferBuilder fbb = new FlatBufferBuilder(16);
         int str = fbb.createString("ByteMonster");

tests/MyGame/Example/Monster.java

@@ -221,7 +221,7 @@ public final class Monster extends Table {
   public static void addPos(FlatBufferBuilder builder, int posOffset) { builder.addStruct(0, posOffset, 0); }
   public static void addMana(FlatBufferBuilder builder, short mana) { builder.addShort(1, mana, 150); }
   public static void addHp(FlatBufferBuilder builder, short hp) { builder.addShort(2, hp, 100); }
-  public static void addName(FlatBufferBuilder builder, int nameOffset) { builder.addOffset(3, nameOffset, 0); }
+  public static void addName(FlatBufferBuilder builder, int nameOffset) { builder.addOffset(nameOffset); builder.slot(3); }
   public static void addInventory(FlatBufferBuilder builder, int inventoryOffset) { builder.addOffset(5, inventoryOffset, 0); }
   public static int createInventoryVector(FlatBufferBuilder builder, byte[] data) { return builder.createByteVector(data); }
   public static int createInventoryVector(FlatBufferBuilder builder, ByteBuffer data) { return builder.createByteVector(data); }

tests/MyGame/Example/Referrable.java

@@ -26,7 +26,7 @@ public final class Referrable extends Table {
   }
 
   public static void startReferrable(FlatBufferBuilder builder) { builder.startTable(1); }
-  public static void addId(FlatBufferBuilder builder, long id) { builder.addLong(0, id, 0L); }
+  public static void addId(FlatBufferBuilder builder, long id) { builder.addLong(id); builder.slot(0); }
   public static int endReferrable(FlatBufferBuilder builder) {
     int o = builder.endTable();
     return o;

tests/MyGame/Example/Stat.java

@@ -37,7 +37,7 @@ public final class Stat extends Table {
   public static void startStat(FlatBufferBuilder builder) { builder.startTable(3); }
   public static void addId(FlatBufferBuilder builder, int idOffset) { builder.addOffset(0, idOffset, 0); }
   public static void addVal(FlatBufferBuilder builder, long val) { builder.addLong(1, val, 0L); }
-  public static void addCount(FlatBufferBuilder builder, int count) { builder.addShort(2, (short) count, (short) 0); }
+  public static void addCount(FlatBufferBuilder builder, int count) { builder.addShort((short) count); builder.slot(2); }
   public static int endStat(FlatBufferBuilder builder) {
     int o = builder.endTable();
     return o;

tests/dictionary_lookup.fbs

@@ -0,0 +1,11 @@
+namespace DictionaryLookup;
+
+table LongFloatEntry {
+  key: long (key);
+  value: float;
+}
+
+table LongFloatMap {
+  entries: [LongFloatEntry];
+}
+root_type LongFloatMap;