handle shorts,chars,byte sentinal values and write tests

Netflix · Dec 12, 2024 · 68d8144 · 68d8144
1 parent 948a3ac
commit 68d8144
Show file tree

Hide file tree

Showing 3 changed files with 104 additions and 11 deletions.
diff --git a/hollow/src/main/java/com/netflix/hollow/core/write/objectmapper/HollowObjectTypeMapper.java b/hollow/src/main/java/com/netflix/hollow/core/write/objectmapper/HollowObjectTypeMapper.java
@@ -473,13 +473,28 @@ public void copy(Object obj, HollowObjectWriteRecord rec, FlatRecordWriter flatR
                     rec.setInt(fieldName, unsafe.getInt(obj, fieldOffset));
                     break;
                 case SHORT:
-                    rec.setInt(fieldName, unsafe.getShort(obj, fieldOffset));
+                    short shortValue = unsafe.getShort(obj, fieldOffset);
+                    if (shortValue == Short.MIN_VALUE) {
+                        rec.setInt(fieldName, Integer.MIN_VALUE);
+                    } else {
+                        rec.setInt(fieldName, shortValue);
+                    }
                     break;
                 case BYTE:
-                    rec.setInt(fieldName, unsafe.getByte(obj, fieldOffset));
+                    byte byteValue = unsafe.getByte(obj, fieldOffset);
+                    if (byteValue == Byte.MIN_VALUE) {
+                        rec.setInt(fieldName, Integer.MIN_VALUE);
+                    } else {
+                        rec.setInt(fieldName, byteValue);
+                    }
                     break;
                 case CHAR:
-                    rec.setInt(fieldName, unsafe.getChar(obj, fieldOffset));
+                    char charValue = unsafe.getChar(obj, fieldOffset);
+                    if (charValue == Character.MIN_VALUE) {
+                        rec.setInt(fieldName, Integer.MIN_VALUE);
+                    } else {
+                        rec.setInt(fieldName, charValue);
+                    }
                     break;
                 case LONG:
                     rec.setLong(fieldName, unsafe.getLong(obj, fieldOffset));
@@ -583,15 +598,27 @@ public void copy(Object pojo, GenericHollowObject rec) {
                     break;
                 case SHORT:
                     int shortValue = rec.getInt(fieldName);
-                    unsafe.putShort(pojo, fieldOffset, (short) shortValue);
+                    if (shortValue == Integer.MIN_VALUE) {
+                        unsafe.putShort(pojo, fieldOffset, Short.MIN_VALUE);
+                    } else {
+                        unsafe.putShort(pojo, fieldOffset, (short) shortValue);
+                    }
                     break;
                 case BYTE:
                     int byteValue = rec.getInt(fieldName);
-                    unsafe.putByte(pojo, fieldOffset, (byte) byteValue);
+                    if (byteValue == Integer.MIN_VALUE) {
+                        unsafe.putByte(pojo, fieldOffset, Byte.MIN_VALUE);
+                    } else {
+                        unsafe.putByte(pojo, fieldOffset, (byte) byteValue);
+                    }
                     break;
                 case CHAR:
                     int charValue = rec.getInt(fieldName);
-                    unsafe.putChar(pojo, fieldOffset, (char) charValue);
+                    if (charValue == Integer.MIN_VALUE) {
+                        unsafe.putChar(pojo, fieldOffset, Character.MIN_VALUE);
+                    } else {
+                        unsafe.putChar(pojo, fieldOffset, (char) charValue);
+                    }
                     break;
                 case LONG:
                     long longValue = rec.getLong(fieldName);
@@ -820,17 +847,29 @@ private void copy(Object obj, FlatRecordTraversalObjectNode node) {
                 }
                 case SHORT: {
                     int value = node.getFieldValueInt(fieldName);
-                    unsafe.putShort(obj, fieldOffset, (short) value);
+                    if(value == Integer.MIN_VALUE) {
+                        unsafe.putShort(obj, fieldOffset, Short.MIN_VALUE);
+                    } else {
+                        unsafe.putShort(obj, fieldOffset, (short) value);
+                    }
                     break;
                 }
                 case BYTE: {
                     int value = node.getFieldValueInt(fieldName);
-                    unsafe.putByte(obj, fieldOffset, (byte) value);
+                    if (value == Integer.MIN_VALUE) {
+                        unsafe.putByte(obj, fieldOffset, Byte.MIN_VALUE);
+                    } else {
+                        unsafe.putByte(obj, fieldOffset, (byte) value);
+                    }
                     break;
                 }
                 case CHAR: {
                     int value = node.getFieldValueInt(fieldName);
-                    unsafe.putChar(obj, fieldOffset, (char) value);
+                    if (value == Integer.MIN_VALUE) {
+                        unsafe.putChar(obj, fieldOffset, Character.MIN_VALUE);
+                    } else {
+                        unsafe.putChar(obj, fieldOffset, (char) value);
+                    }
                     break;
                 }
                 case LONG: {

diff --git a/...va/com/netflix/hollow/core/write/objectmapper/HollowObjectMapperFlatRecordParserTest.java b/...va/com/netflix/hollow/core/write/objectmapper/HollowObjectMapperFlatRecordParserTest.java
@@ -203,6 +203,32 @@ public void shouldMapPrimitiveWrapperToNonPrimitiveWrapperIfCommonFieldIsTheSame
     Assert.assertEquals("value", result.subValue.value);
   }
 
+  @Test
+  public void testReadPrimitivesPersistedWithSentinalValues() {
+    TypeWithAllSimpleTypes
+        typeWithAllSimpleTypes = new TypeWithAllSimpleTypes();
+    typeWithAllSimpleTypes.boxedIntegerField = 10;
+    typeWithAllSimpleTypes.stringField = "stringField";
+    typeWithAllSimpleTypes.primitiveIntegerField = Integer.MIN_VALUE; //write sentinal
+    typeWithAllSimpleTypes.primitiveFloatField = Float.NaN;
+    typeWithAllSimpleTypes.primitiveDoubleField = Double.NaN;
+    typeWithAllSimpleTypes.primitiveLongField = Long.MIN_VALUE;
+    typeWithAllSimpleTypes.primitiveShortField = Short.MIN_VALUE;
+    typeWithAllSimpleTypes.primitiveByteField = Byte.MIN_VALUE;
+    typeWithAllSimpleTypes.primitiveCharField = Character.MIN_VALUE;
+    flatRecordWriter.reset();
+    mapper.writeFlat(typeWithAllSimpleTypes, flatRecordWriter);
+    FlatRecord fr = flatRecordWriter.generateFlatRecord();
+    TypeWithAllSimpleTypes result = mapper.readFlat(fr);
+    Assert.assertEquals(Integer.MIN_VALUE, typeWithAllSimpleTypes.primitiveIntegerField);
+    Assert.assertEquals(Long.MIN_VALUE, typeWithAllSimpleTypes.primitiveLongField);
+    Assert.assertEquals(Short.MIN_VALUE, typeWithAllSimpleTypes.primitiveShortField);
+    Assert.assertEquals(Byte.MIN_VALUE, typeWithAllSimpleTypes.primitiveByteField);
+    Assert.assertEquals(Character.MIN_VALUE, typeWithAllSimpleTypes.primitiveCharField);
+    Assert.assertTrue(Float.isNaN(typeWithAllSimpleTypes.primitiveFloatField));
+    Assert.assertTrue(Double.isNaN(typeWithAllSimpleTypes.primitiveDoubleField));
+  }
+
   @HollowPrimaryKey(fields={"boxedIntegerField", "stringField"})
   private static class TypeWithAllSimpleTypes {
     Integer boxedIntegerField;

diff --git a/.../com/netflix/hollow/core/write/objectmapper/HollowObjectMapperHollowRecordParserTest.java b/.../com/netflix/hollow/core/write/objectmapper/HollowObjectMapperHollowRecordParserTest.java
@@ -116,6 +116,34 @@ public void testSimpleTypes() {
     Assert.assertEquals(typeWithAllSimpleTypes, result);
   }
 
+  @Test
+  public void testReadPrimitivesPersistedWithSentinalValues() {
+    TypeWithAllSimpleTypes typeWithAllSimpleTypes = new TypeWithAllSimpleTypes();
+    typeWithAllSimpleTypes.primitiveIntegerField = Integer.MIN_VALUE; //write sentinal
+    typeWithAllSimpleTypes.primitiveFloatField = Float.NaN;
+    typeWithAllSimpleTypes.primitiveDoubleField = Double.NaN;
+    typeWithAllSimpleTypes.primitiveLongField = Long.MIN_VALUE;
+    typeWithAllSimpleTypes.primitiveShortField = Short.MIN_VALUE;
+    typeWithAllSimpleTypes.primitiveByteField = Byte.MIN_VALUE;
+    typeWithAllSimpleTypes.primitiveCharField = Character.MIN_VALUE;
+    HollowReadStateEngine stateEngine = createReadStateEngine(typeWithAllSimpleTypes);
+    GenericHollowObject obj = new GenericHollowObject(stateEngine, "TypeWithAllSimpleTypes", 0);
+    TypeWithAllSimpleTypes result = mapper.readHollowRecord(obj);
+    Assert.assertEquals(result.primitiveByteField, Byte.MIN_VALUE);
+    Assert.assertEquals(result.primitiveCharField, Character.MIN_VALUE);
+    Assert.assertEquals(result.primitiveIntegerField, Integer.MIN_VALUE);
+    Assert.assertEquals(result.primitiveShortField, Short.MIN_VALUE);
+    Assert.assertEquals(0, Double.compare(result.primitiveDoubleField, Double.NaN));
+    Assert.assertEquals(result.primitiveLongField, Long.MIN_VALUE);
+    Assert.assertEquals(0, Float.compare(result.primitiveFloatField, Float.NaN));
+    Assert.assertEquals(result.primitiveIntegerField, Integer.MIN_VALUE);
+  }
+
+  @Test
+  public void testWritePrimitivesPersistedWithSentinalValues() {
+    TypeWithAllSimpleTypes typeWithAllSimpleTypes = new TypeWithAllSimpleTypes();
+  }
+
   @Test
   public void testNullablesSimpleTypes() {
     TypeWithAllSimpleTypes typeWithAllSimpleTypes = new TypeWithAllSimpleTypes();
@@ -137,9 +165,9 @@ public void testNullablesSimpleTypes() {
     Assert.assertNull(result.boxedLongField);
     Assert.assertNull(result.boxedShortField);
     Assert.assertNull(result.boxedByteField);
-    Assert.assertEquals(0, result.primitiveIntegerField);
+    Assert.assertEquals(Integer.MIN_VALUE, result.primitiveIntegerField);
     Assert.assertFalse(result.primitiveBooleanField);
-    Assert.assertEquals(0.0, result.primitiveDoubleField, 0);
+    Assert.assertEquals(Double.NaN, result.primitiveDoubleField, 0);
     Assert.assertEquals(0.0f, result.primitiveFloatField, 0);
     Assert.assertEquals(0L, result.primitiveLongField);
     Assert.assertEquals(0, result.primitiveShortField);