Skip to content

Commit

Permalink
Primitive objectmapper nulls (#712)
Browse files Browse the repository at this point in the history 
* objectmapper's conversion from hollow/flat record to pojo should keep the sentinal values, instead of not setting them

* handle shorts,chars,byte sentinal values and write tests

* remove empty test

* test to simulate the empty values for primitives that triggers default sentinal value conversions
  • Loading branch information
@vvstej
vvstej authored Dec 16, 2024
1 parent ff6fa9e commit 3f11f46
Show file tree
Hide file tree
Showing 3 changed files with 205 additions and 35 deletions.
77 changes: 44 additions & 33 deletions hollow/src/main/java/com/netflix/hollow/core/write/objectmapper/HollowObjectTypeMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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: {
Expand Down
26 changes: 26 additions & 0 deletions ...va/com/netflix/hollow/core/write/objectmapper/HollowObjectMapperFlatRecordParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
137 changes: 135 additions & 2 deletions .../com/netflix/hollow/core/write/objectmapper/HollowObjectMapperHollowRecordParserTest.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 3f11f46

Please sign in to comment.