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 index ecbac4fdc..6259f77e1 100644 --- 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)); @@ -579,45 +594,43 @@ public void copy(Object pojo, GenericHollowObject rec) { break; case INT: int intValue = rec.getInt(fieldName); - if (intValue != Integer.MIN_VALUE) { - unsafe.putInt(pojo, fieldOffset, intValue); - } + unsafe.putInt(pojo, fieldOffset, intValue); break; case SHORT: int shortValue = rec.getInt(fieldName); - if (shortValue != Integer.MIN_VALUE) { + 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); - if (byteValue != Integer.MIN_VALUE) { + 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); - if (charValue != Integer.MIN_VALUE) { + 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); - if (longValue != Long.MIN_VALUE) { - unsafe.putLong(pojo, fieldOffset, longValue); - } + unsafe.putLong(pojo, fieldOffset, longValue); break; case DOUBLE: double doubleValue = rec.getDouble(fieldName); - if (!Double.isNaN(doubleValue)) { - unsafe.putDouble(pojo, fieldOffset, doubleValue); - } + unsafe.putDouble(pojo, fieldOffset, doubleValue); break; case FLOAT: float floatValue = rec.getFloat(fieldName); - if (!Float.isNaN(floatValue)) { - unsafe.putFloat(pojo, fieldOffset, floatValue); - } + unsafe.putFloat(pojo, fieldOffset, floatValue); break; case STRING: unsafe.putObject(pojo, fieldOffset, rec.getString(fieldName)); @@ -829,51 +842,49 @@ private void copy(Object obj, FlatRecordTraversalObjectNode node) { } case INT: { int value = node.getFieldValueInt(fieldName); - if (value != Integer.MIN_VALUE) { - unsafe.putInt(obj, fieldOffset, value); - } + unsafe.putInt(obj, fieldOffset, value); break; } case SHORT: { int value = node.getFieldValueInt(fieldName); - if (value != Integer.MIN_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); - if (value != Integer.MIN_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); - if (value != Integer.MIN_VALUE) { + if (value == Integer.MIN_VALUE) { + unsafe.putChar(obj, fieldOffset, Character.MIN_VALUE); + } else { unsafe.putChar(obj, fieldOffset, (char) value); } break; } case LONG: { long value = node.getFieldValueLong(fieldName); - if (value != Long.MIN_VALUE) { - unsafe.putLong(obj, fieldOffset, value); - } + unsafe.putLong(obj, fieldOffset, value); break; } case FLOAT: { float value = node.getFieldValueFloat(fieldName); - if (!Float.isNaN(value)) { - unsafe.putFloat(obj, fieldOffset, value); - } + unsafe.putFloat(obj, fieldOffset, value); break; } case DOUBLE: { double value = node.getFieldValueDouble(fieldName); - if (!Double.isNaN(value)) { - unsafe.putDouble(obj, fieldOffset, value); - } + unsafe.putDouble(obj, fieldOffset, value); break; } case STRING: { diff --git a/hollow/src/test/java/com/netflix/hollow/core/write/objectmapper/HollowObjectMapperFlatRecordParserTest.java b/hollow/src/test/java/com/netflix/hollow/core/write/objectmapper/HollowObjectMapperFlatRecordParserTest.java index 19c16f3e7..a7564c8a0 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/write/objectmapper/HollowObjectMapperFlatRecordParserTest.java +++ b/hollow/src/test/java/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/hollow/src/test/java/com/netflix/hollow/core/write/objectmapper/HollowObjectMapperHollowRecordParserTest.java b/hollow/src/test/java/com/netflix/hollow/core/write/objectmapper/HollowObjectMapperHollowRecordParserTest.java index c364716cd..c829eaa0c 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/write/objectmapper/HollowObjectMapperHollowRecordParserTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/write/objectmapper/HollowObjectMapperHollowRecordParserTest.java @@ -1,15 +1,23 @@ package com.netflix.hollow.core.write.objectmapper; +import com.netflix.hollow.api.consumer.HollowConsumer; +import com.netflix.hollow.api.consumer.fs.HollowFilesystemBlobRetriever; import com.netflix.hollow.api.objects.generic.GenericHollowObject; +import com.netflix.hollow.api.producer.HollowProducer; +import com.netflix.hollow.api.producer.fs.HollowFilesystemPublisher; import com.netflix.hollow.core.read.engine.HollowReadStateEngine; import com.netflix.hollow.core.util.StateEngineRoundTripper; import com.netflix.hollow.core.write.HollowWriteStateEngine; +import com.netflix.hollow.core.write.objectmapper.flatrecords.FakeHollowSchemaIdentifierMapper; +import com.netflix.hollow.core.write.objectmapper.flatrecords.FlatRecord; +import com.netflix.hollow.core.write.objectmapper.flatrecords.FlatRecordExtractor; import com.netflix.hollow.test.HollowWriteStateEngineBuilder; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import java.io.IOException; +import java.nio.file.Paths; import java.util.Arrays; import java.util.Base64; import java.util.Date; @@ -21,6 +29,8 @@ import java.util.Set; import java.util.stream.Collectors; +import static org.assertj.core.api.Assertions.assertThat; + public class HollowObjectMapperHollowRecordParserTest { private HollowObjectMapper mapper; @@ -116,6 +126,29 @@ 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 testNullablesSimpleTypes() { TypeWithAllSimpleTypes typeWithAllSimpleTypes = new TypeWithAllSimpleTypes(); @@ -137,9 +170,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); @@ -278,6 +311,106 @@ public void shouldMapPrimitiveWrapperToNonPrimitiveWrapperIfCommonFieldIsTheSame Assert.assertEquals("value", result.subValue.value); } + @Test + public void testPrimitiveNullFieldsReceiveTheirSentinelValues() { + HollowProducer producer = + new HollowProducer.Builder<>() + .withPublisher(new HollowFilesystemPublisher(Paths.get("/tmp/blob-cache/"))) + .build(); + + producer.runCycle( + state -> { + state.add(new PojoV1(1, "The Matrix")); + }); + + // A new producer with a different schema + HollowProducer producerWithSchemaChange = + new HollowProducer.Builder<>() + .withPublisher(new HollowFilesystemPublisher(Paths.get("/tmp/blob-cache/"))) + .build(); + producerWithSchemaChange.initializeDataModel(PojoV2.class); + producerWithSchemaChange.restore( + Long.MAX_VALUE, new HollowFilesystemBlobRetriever(Paths.get("/tmp/blob-cache/"))); + + producerWithSchemaChange.runCycle( + state -> { + state.getStateEngine().addAllObjectsFromPreviousCycle(); + state.add(new PojoV2(2, "Speed Racer", 2008, 0.0d, 0.0f, (short) 10, (byte) 0x8, 'a')); + }); + + HollowConsumer consumer = + HollowConsumer.withBlobRetriever( + new HollowFilesystemBlobRetriever(Paths.get("/tmp/blob-cache/"))) + .build(); + consumer.triggerRefresh(); + + HollowObjectMapper typeMapper = new HollowObjectMapper(new HollowWriteStateEngine()); + typeMapper.initializeTypeState(PojoV2.class); + + ////////////////////////// + // Test ReadState parsing + GenericHollowObject theMatrixObj = new GenericHollowObject(consumer.getStateEngine(), "Pojo", 0); + PojoV2 theMatrixObjPojo = typeMapper.readHollowRecord(theMatrixObj); + assertThat(theMatrixObjPojo.intField).isEqualTo(Integer.MIN_VALUE); + // check other primitive fields... + + GenericHollowObject speedRacerObj = new GenericHollowObject(consumer.getStateEngine(), "Pojo", 1); + PojoV2 speedRacerObjPojo = typeMapper.readHollowRecord(speedRacerObj); + assertThat(speedRacerObjPojo.intField).isEqualTo(2008); + // check other primitive fields... + + ////////////////////////// + // Test FlatRecord parsing + FlatRecordExtractor extractor = new FlatRecordExtractor(consumer.getStateEngine(), new FakeHollowSchemaIdentifierMapper(consumer.getStateEngine())); + + FlatRecord theMatrixFr = extractor.extract("Pojo", 0); + PojoV2 theMatrixFrPojo = typeMapper.readFlat(theMatrixFr); + assertThat(theMatrixFrPojo.intField).isEqualTo(Integer.MIN_VALUE); + // check other primitive fields... + + FlatRecord speedRacerFr = extractor.extract("Pojo", 1); + PojoV2 speedRacerFrPojo = typeMapper.readFlat(speedRacerFr); + assertThat(speedRacerFrPojo.intField).isEqualTo(2008); + // check other primitive fields... + } + + @HollowTypeName(name = "Pojo") + @HollowPrimaryKey(fields = {"id"}) + static class PojoV1 { + final int id; + final String strField; + + public PojoV1(int id, String strField) { + this.id = id; + this.strField = strField; + } + } + + @HollowTypeName(name = "Pojo") + @HollowPrimaryKey(fields = {"id"}) + static class PojoV2 { + final int id; + final String strField; + final int intField; + final double doubleField; + final float floatField; + final short shortField; + final byte byteField; + final char charField; + // all other primitives + + public PojoV2(int id, String strField, int intField, double doubleField, float floatField, short shortField, byte byteField, char charField) { + this.id = id; + this.strField = strField; + this.intField = intField; + this.doubleField = doubleField; + this.floatField = floatField; + this.shortField = shortField; + this.byteField = byteField; + this.charField = charField; + } + } + private HollowReadStateEngine createReadStateEngine(Object... recs) { HollowWriteStateEngine writeStateEngine = new HollowWriteStateEngineBuilder() .add(recs)