Primitive objectmapper nulls

[vvstej]

No description provided.

[eduardoramirez]

is this test empty on purpose?

[eduardoramirez]

this is good. in addition, we should add a test case for the specific case that raised this concern, where primitive fields are added in a new cycle but only partially populated. the simplest way i can think of doing this is to use a producer/consumer pair. here is an example of that (where in only show the int field but the rest are the same). it also tests both the HollowReadStateEngine and FlatRecord path

class Test {
  @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));
      });

    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;
    // all other primitives

    public PojoV2(int id, String strField, int intField) {
      this.id = id;
      this.strField = strField;
      this.intField = intField;
    }
  }
}

[vvstej]

Thanks for this test, the underlying issue though seems to be covered by the serialization/deserialization tests though isnt it i.e interpretation of "empty" records generally?

[vvstej]

added the test anyway

[eduardoramirez]

as in, the sentinel values you're manually setting on the POJOs get converted into just the "null" value by ser? in that case, it's probably covered, not 100% sure for schema change-based scenarios. generally, i like adding these "reproduce"-like tests that are very close to the prod use case. it's not always possible but nice when it is. thanks for adding it!