DonalEvans commented on a change in pull request #6053:
URL: https://github.com/apache/geode/pull/6053#discussion_r583301238
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/TXCommitMessage.java
##########
@@ -754,8 +754,14 @@ void firePendingCallbacks(List<EntryEventImpl> callbacks) {
ee.getRegion().invokeTXCallbacks(EnumListenerEvent.AFTER_CREATE, ee,
true,
isLastTransactionEvent);
} else {
- ee.getRegion().invokeTXCallbacks(EnumListenerEvent.AFTER_UPDATE, ee,
true,
- isLastTransactionEvent);
+ if (ee.getNewValue() == null) { // GEODE-8964, fixes GII and TX
create conflict that
Review comment:
Small nitpick, but could the comment here be put on its own line?
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/cache/TXCommitMessageTest.java
##########
@@ -80,4 +81,48 @@ public void
firePendingCallbacksSetsChangeAppliedToCacheInEventLocalFilterInfo()
verify(filterInfo2).setChangeAppliedToCache(true);
}
+ @Test
+ public void
firePendingCallbacksSendsAFTER_CREATECallbackIfUpdateEntryEventHasNullNewValue()
{
+ TXCommitMessage message = spy(new TXCommitMessage());
+ LocalRegion region = mock(LocalRegion.class, RETURNS_DEEP_STUBS);
+ EntryEventImpl updateEvent = mock(EntryEventImpl.class,
RETURNS_DEEP_STUBS);
+ EntryEventImpl lastTxEvent = mock(EntryEventImpl.class,
RETURNS_DEEP_STUBS);
+
+ List<EntryEventImpl> callbacks = new ArrayList<>();
+ callbacks.add(updateEvent);
+ callbacks.add(lastTxEvent);
+
+ when(updateEvent.getLocalFilterInfo()).thenReturn(null);
+ when(updateEvent.getNewValue()).thenReturn(null);
+ when(updateEvent.getRegion()).thenReturn(region);
+
+ doReturn(lastTxEvent).when(message).getLastTransactionEvent(callbacks);
+
+ message.firePendingCallbacks(callbacks);
+
+ verify(region).invokeTXCallbacks(EnumListenerEvent.AFTER_CREATE,
updateEvent, true, false);
Review comment:
The two test cases here can be condensed to one if this line is changed
to `verify(region, only()).invokeTXCallbacks(EnumListenerEvent.AFTER_CREATE,
updateEvent, true, false);` which will validate both that `invokeTXCallbacks()`
is called with the correct arguments and that it is *only* called with those
arguments, removing the need for a separate test to confirm that it's not
called with the `AFTER_UPDATE` argument.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
