Skip to content

Commit c2920b7

Browse files
dvehardvehar
committed
Prevent NPE when Incremental producer attempts to remove records from types that no longer exist
1 parent d0e88ce commit c2920b7

File tree

2 files changed

+86
-68
lines changed

2 files changed

+86
-68
lines changed

‎hollow/src/main/java/com/netflix/hollow/api/producer/HollowIncrementalCyclePopulator.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,9 @@ private BitSet markTypeRecordsToRemove(HollowReadStateEngine priorStateEngine, f
139139
private void removeRecordsFromNewState(HollowProducer.WriteState newState, Map<String, BitSet> recordsToRemove) {
140140
for(Map.Entry<String, BitSet> removalEntry : recordsToRemove.entrySet()) {
141141
HollowTypeWriteState writeState = newState.getStateEngine().getTypeState(removalEntry.getKey());
142-
BitSet typeRecordsToRemove = removalEntry.getValue();
142+
if (writeState == null) continue;
143143

144+
BitSet typeRecordsToRemove = removalEntry.getValue();
144145
int ordinalToRemove = typeRecordsToRemove.nextSetBit(0);
145146
while(ordinalToRemove != -1) {
146147
writeState.removeOrdinalFromThisCycle(ordinalToRemove);

‎hollow/src/test/java/com/netflix/hollow/api/producer/HollowIncrementalProducerTest.java

+84-67
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,11 @@ public void publishAndLoadASnapshot() {
115115
idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeB", "id");
116116
Assert.assertFalse(idx.containsDuplicates());
117117

118-
assertTypeB(idx, 1, "1");
119-
assertTypeB(idx, 2, null);
120-
assertTypeB(idx, 3, null);
121-
assertTypeB(idx, 4, "4");
122-
assertTypeB(idx, 5, "6");
118+
assertTypeBOrE(idx, 1, "1");
119+
assertTypeBOrE(idx, 2, null);
120+
assertTypeBOrE(idx, 3, null);
121+
assertTypeBOrE(idx, 4, "4");
122+
assertTypeBOrE(idx, 5, "6");
123123

124124
consumer.triggerRefreshTo(finalVersion);
125125

@@ -271,11 +271,11 @@ public void publishAndLoadASnapshotDirectly() {
271271
Assert.assertFalse(idx.containsDuplicates());
272272

273273
// backing producer was never initialized, so only records added to the incremental producer are here
274-
assertTypeB(idx, 1, null);
275-
assertTypeB(idx, 2, null);
276-
assertTypeB(idx, 3, null);
277-
assertTypeB(idx, 4, null);
278-
assertTypeB(idx, 5, "6");
274+
assertTypeBOrE(idx, 1, null);
275+
assertTypeBOrE(idx, 2, null);
276+
assertTypeBOrE(idx, 3, null);
277+
assertTypeBOrE(idx, 4, null);
278+
assertTypeBOrE(idx, 5, "6");
279279

280280
consumer.triggerRefreshTo(finalVersion);
281281

@@ -330,12 +330,12 @@ public void publishDirectlyAndRestore() {
330330
Assert.assertFalse(idx.containsDuplicates());
331331

332332
// backing producer was never initialized, so only records added to the incremental producer are here
333-
assertTypeB(idx, 1, null);
334-
assertTypeB(idx, 2, null);
335-
assertTypeB(idx, 3, null);
336-
assertTypeB(idx, 4, null);
337-
assertTypeB(idx, 5, "5");
338-
assertTypeB(idx, 6, "6");
333+
assertTypeBOrE(idx, 1, null);
334+
assertTypeBOrE(idx, 2, null);
335+
assertTypeBOrE(idx, 3, null);
336+
assertTypeBOrE(idx, 4, null);
337+
assertTypeBOrE(idx, 5, "5");
338+
assertTypeBOrE(idx, 6, "6");
339339

340340
// Create NEW incremental producer which will restore from the state left by the previous incremental producer
341341
/// adding a new type this time (TypeB).
@@ -375,12 +375,12 @@ public void publishDirectlyAndRestore() {
375375
Assert.assertFalse(idx.containsDuplicates());
376376

377377
// backing producer was never initialized, so only records added to the incremental producer are here
378-
assertTypeB(idx, 1, "1");
379-
assertTypeB(idx, 2, "2");
380-
assertTypeB(idx, 3, "3");
381-
assertTypeB(idx, 4, "4");
382-
assertTypeB(idx, 5, null);
383-
assertTypeB(idx, 6, "6");
378+
assertTypeBOrE(idx, 1, "1");
379+
assertTypeBOrE(idx, 2, "2");
380+
assertTypeBOrE(idx, 3, "3");
381+
assertTypeBOrE(idx, 4, "4");
382+
assertTypeBOrE(idx, 5, null);
383+
assertTypeBOrE(idx, 6, "6");
384384

385385
}
386386

@@ -431,7 +431,7 @@ public void populate(WriteState state) throws Exception {
431431
idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeB", "id");
432432
Assert.assertFalse(idx.containsDuplicates());
433433

434-
assertTypeB(idx, 3, "three");
434+
assertTypeBOrE(idx, 3, "three");
435435
}
436436

437437
@Test
@@ -442,6 +442,7 @@ public void canRemoveAndModifyNewTypesFromRestoredState() {
442442
long originalVersion = genesisProducer.runCycle(new Populator() {
443443
public void populate(WriteState state) throws Exception {
444444
state.add(new TypeA(1, "one", 1));
445+
state.add(new TypeB(2, "two"));
445446
}
446447
});
447448

@@ -452,22 +453,22 @@ public void populate(WriteState state) throws Exception {
452453
.noIntegrityCheck()
453454
.build();
454455

455-
/// adding a new type this time (TypeB).
456-
backingProducer.initializeDataModel(TypeA.class, TypeB.class);
456+
/// adding a new type this time (TypeE) and removing TypeB
457+
backingProducer.initializeDataModel(TypeA.class, TypeE.class);
457458

458459
/// now create our HollowIncrementalProducer
459460
HollowIncrementalProducer incrementalProducer = new HollowIncrementalProducer(backingProducer);
460461
incrementalProducer.restore(originalVersion, blobStore);
461462

462463
incrementalProducer.addOrModify(new TypeA(1, "one", 2));
463-
incrementalProducer.addOrModify(new TypeA(2, "two", 2));
464-
incrementalProducer.addOrModify(new TypeB(3, "three"));
465-
incrementalProducer.addOrModify(new TypeB(4, "four"));
464+
incrementalProducer.addOrModify(new TypeA(3, "three", 3));
465+
incrementalProducer.addOrModify(new TypeE(4, "four"));
466+
incrementalProducer.addOrModify(new TypeE(5, "five"));
466467

467468
incrementalProducer.runCycle();
468469

469-
incrementalProducer.delete(new RecordPrimaryKey("TypeB", new Object[] { 3 }));
470-
incrementalProducer.addOrModify(new TypeB(4, "four!"));
470+
incrementalProducer.delete(new RecordPrimaryKey("TypeE", new Object[] { 4 }));
471+
incrementalProducer.addOrModify(new TypeE(5, "five!"));
471472

472473
long version2 = incrementalProducer.runCycle();
473474

@@ -479,17 +480,21 @@ public void populate(WriteState state) throws Exception {
479480
Assert.assertFalse(idx.containsDuplicates());
480481

481482
assertTypeA(idx, 1, "one", 2L);
482-
assertTypeA(idx, 2, "two", 2L);
483+
assertTypeA(idx, 3, "three", 3L);
483484

484485
/// consumers with established data models don't have visibility into new types.
485486
consumer = HollowConsumer.withBlobRetriever(blobStore).build();
486487
consumer.triggerRefreshTo(version2);
487488

488-
idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeB", "id");
489+
Assert.assertNotNull(consumer.getStateEngine().getTypeState("TypeA"));
490+
Assert.assertNull(consumer.getStateEngine().getTypeState("TypeB"));
491+
Assert.assertNotNull(consumer.getStateEngine().getTypeState("TypeE"));
492+
493+
idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeE", "id");
489494
Assert.assertFalse(idx.containsDuplicates());
490495

491-
assertEquals(-1, idx.getMatchingOrdinal(3));
492-
assertTypeB(idx, 4, "four!");
496+
assertEquals(-1, idx.getMatchingOrdinal(4));
497+
assertTypeBOrE(idx, 5, "five!");
493498
}
494499

495500
@Test
@@ -541,7 +546,7 @@ public void populate(WriteState state) throws Exception {
541546
idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeB", "id");
542547
Assert.assertFalse(idx.containsDuplicates());
543548

544-
assertTypeB(idx, 3, "three");
549+
assertTypeBOrE(idx, 3, "three");
545550
}
546551

547552
@Test(expected = RuntimeException.class)
@@ -627,11 +632,11 @@ public void publishUsingThreadConfig() {
627632
idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeB", "id");
628633
Assert.assertFalse(idx.containsDuplicates());
629634

630-
assertTypeB(idx, 1, "1");
631-
assertTypeB(idx, 2, null);
632-
assertTypeB(idx, 3, null);
633-
assertTypeB(idx, 4, "4");
634-
assertTypeB(idx, 5, "6");
635+
assertTypeBOrE(idx, 1, "1");
636+
assertTypeBOrE(idx, 2, null);
637+
assertTypeBOrE(idx, 3, null);
638+
assertTypeBOrE(idx, 4, "4");
639+
assertTypeBOrE(idx, 5, "6");
635640

636641
consumer.triggerRefreshTo(finalVersion);
637642

@@ -664,7 +669,7 @@ public void discardChanges() {
664669

665670
HollowPrimaryKeyIndex idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeB", "id");
666671

667-
assertTypeB(idx, 1, "one");
672+
assertTypeBOrE(idx, 1, "one");
668673

669674
incrementalProducer.delete(new TypeB(1, "one"));
670675

@@ -680,7 +685,7 @@ public void discardChanges() {
680685
consumer = HollowConsumer.withBlobRetriever(blobStore).build();
681686
consumer.triggerRefreshTo(version);
682687

683-
assertTypeB(idx, 1, "one");
688+
assertTypeBOrE(idx, 1, "one");
684689

685690
incrementalProducer.delete(new TypeB(1, "one"));
686691

@@ -696,7 +701,7 @@ public void discardChanges() {
696701
consumer = HollowConsumer.withBlobRetriever(blobStore).build();
697702
consumer.triggerRefreshTo(finalVersion);
698703

699-
assertTypeB(idx, 1, "one");
704+
assertTypeBOrE(idx, 1, "one");
700705
}
701706

702707
@Test
@@ -780,11 +785,11 @@ public void addAndDeleteCollections() {
780785
idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeB", "id");
781786
Assert.assertFalse(idx.containsDuplicates());
782787

783-
assertTypeB(idx, 1, "1");
784-
assertTypeB(idx, 2, null);
785-
assertTypeB(idx, 3, "3");
786-
assertTypeB(idx, 4, "4");
787-
assertTypeB(idx, 5, null);
788+
assertTypeBOrE(idx, 1, "1");
789+
assertTypeBOrE(idx, 2, null);
790+
assertTypeBOrE(idx, 3, "3");
791+
assertTypeBOrE(idx, 4, "4");
792+
assertTypeBOrE(idx, 5, null);
788793

789794
consumer.triggerRefreshTo(finalVersion);
790795

@@ -849,11 +854,11 @@ public void addAndDeleteCollectionsInParallel() {
849854
idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeB", "id");
850855
Assert.assertFalse(idx.containsDuplicates());
851856

852-
assertTypeB(idx, 1, "1");
853-
assertTypeB(idx, 2, null);
854-
assertTypeB(idx, 3, "3");
855-
assertTypeB(idx, 4, "4");
856-
assertTypeB(idx, 5, null);
857+
assertTypeBOrE(idx, 1, "1");
858+
assertTypeBOrE(idx, 2, null);
859+
assertTypeBOrE(idx, 3, "3");
860+
assertTypeBOrE(idx, 4, "4");
861+
assertTypeBOrE(idx, 5, null);
857862

858863
consumer.triggerRefreshTo(finalVersion);
859864

@@ -919,11 +924,11 @@ public void discardCollections() {
919924
idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeB", "id");
920925
Assert.assertFalse(idx.containsDuplicates());
921926

922-
assertTypeB(idx, 1, "1");
923-
assertTypeB(idx, 2, "2");
924-
assertTypeB(idx, 3, "3");
925-
assertTypeB(idx, 4, "4");
926-
assertTypeB(idx, 5, null);
927+
assertTypeBOrE(idx, 1, "1");
928+
assertTypeBOrE(idx, 2, "2");
929+
assertTypeBOrE(idx, 3, "3");
930+
assertTypeBOrE(idx, 4, "4");
931+
assertTypeBOrE(idx, 5, null);
927932
}
928933

929934
@Test
@@ -977,11 +982,11 @@ public void discardCollectionsInParallel() {
977982
idx = new HollowPrimaryKeyIndex(consumer.getStateEngine(), "TypeB", "id");
978983
Assert.assertFalse(idx.containsDuplicates());
979984

980-
assertTypeB(idx, 1, "1");
981-
assertTypeB(idx, 2, "2");
982-
assertTypeB(idx, 3, "3");
983-
assertTypeB(idx, 4, "4");
984-
assertTypeB(idx, 5, null);
985+
assertTypeBOrE(idx, 1, "1");
986+
assertTypeBOrE(idx, 2, "2");
987+
assertTypeBOrE(idx, 3, "3");
988+
assertTypeBOrE(idx, 4, "4");
989+
assertTypeBOrE(idx, 5, null);
985990
}
986991

987992
@Test
@@ -1478,16 +1483,16 @@ private void assertTypeA(HollowPrimaryKeyIndex typeAIdx, int id1,
14781483
}
14791484
}
14801485

1481-
private void assertTypeB(HollowPrimaryKeyIndex typeBIdx, int id1,
1482-
String expectedValue) {
1483-
int ordinal = typeBIdx.getMatchingOrdinal(id1);
1486+
private void assertTypeBOrE(HollowPrimaryKeyIndex typeIdx, int id1,
1487+
String expectedValue) {
1488+
int ordinal = typeIdx.getMatchingOrdinal(id1);
14841489

14851490
if (expectedValue == null) {
14861491
Assert.assertEquals(-1, ordinal);
14871492
} else {
14881493
Assert.assertNotEquals(-1, ordinal);
14891494
GenericHollowObject obj = new GenericHollowObject(
1490-
typeBIdx.getTypeState(), ordinal);
1495+
typeIdx.getTypeState(), ordinal);
14911496
Assert.assertEquals(expectedValue, obj.getObject("value")
14921497
.getString("value"));
14931498
}
@@ -1543,6 +1548,18 @@ public TypeD(int id, String name) {
15431548
}
15441549
}
15451550

1551+
@HollowPrimaryKey(fields = "id")
1552+
private static class TypeE {
1553+
int id;
1554+
@HollowTypeName(name = "TypeEValue")
1555+
String value;
1556+
1557+
public TypeE(int id, String value) {
1558+
this.id = id;
1559+
this.value = value;
1560+
}
1561+
}
1562+
15461563
private Collection<HollowObject> getAllHollowObjects(HollowConsumer hollowConsumer, final String type) {
15471564
final HollowReadStateEngine readStateEngine = hollowConsumer.getStateEngine();
15481565
final HollowTypeDataAccess typeDataAccess = readStateEngine.getTypeDataAccess(type);

0 commit comments

Comments
 (0)