[#955] Answer a ModifyDN whose entry is gone before the new superior is looked up

[vharseko]

Fixes #955.

solveNamingConflict(ModifyDNOperation) dereferenced currentDN in the branch which handles a missing new parent, seven lines above the check which says currentDN can be null:

DN currentDN = findEntryDN(entryUUID);          // null when the entry is gone
...
if (newSuperior == null)
{
  markConflictEntry(op, currentDN, currentDN.parent().child(newRDN));   // NPE
  ...
}
DN newDN = newSuperior.child(newRDN);
if (currentDN == null)  // the check the author wrote for it, seven lines too late

A replayed ModifyDN carrying a newSuperior, where this replica has since deleted both the entry being moved and the entry it is being moved under, resolves neither entryUUID: findEntryDN() returns null for both, and currentDN.parent() throws.

Nothing upstream of it answers that case: the pre-operation plugin does not run when the target entry is not there - the core returns NO_SUCH_OBJECT before the plugins - and checkDependencies(ModifyDNMsg) only defers a change while an Add or a Delete on the same DNs is still in flight, which deletes applied long ago are not.

What it cost

The NPE did not escape - replay() caught it, logged ERR_EXCEPTION_REPLAYING_OPERATION and left the change out of the ServerState, which is right as far as it goes. But nothing on the replica changes between two deliveries, so every one of them threw in the same place: the change restarted the session each time, burned the whole REPLAY_GIVE_UP_DELAY_IN_MS budget and ended in ERR_REPLAY_SKIPPING_CHANGE and the alert which tells the administrator this replica has diverged and must be reinitialized - for a case the check at the bottom of the method answers in one step.

markConflictEntry() was meaningless in that state anyway: it runs an internal modify against currentDN to put ds-sync-conflict on the entry, and there is no entry at that DN to mark.

The change

The currentDN == null check is answered first, right after the entryUUID is resolved - above the lookup of the new superior as well, so a search which can not change the outcome is not run either. An entry which is not in the database can not be marked as conflicting and does not need to be: the delete has already settled what the ModifyDN was trying to do, which is what the comment on that check says. newDN is only built after the newSuperior == null branch, so nothing between the two checks depended on the order.

This is what the other conflict resolution methods already do: solveNamingConflict(ModifyOperation) and solveNamingConflict(DeleteOperation) both answer "the entry is gone - resolved, nothing to do" before anything else. The ModifyDN one was the only outlier.

The second form of the same branch - newSuperiorID == null with entryDN.parent() null - needs the entry DN to be the root DN, so it is not reachable in practice; the check closes it all the same.

Test

NamingConflictTest.modifyDnOnAnEntryAndANewSuperiorWhichAreBothGone() deletes both the entry and its new superior on this replica and replays a ModifyDN carrying their entryUUIDs. Before the change it failed on the ServerState assertion, with NullPointerException: Cannot invoke "org.forgerock.opendj.ldap.DN.parent()" because "currentDN" is null at LDAPReplicationDomain.java:3530 in the log; now the change is resolved and recorded as replayed on the first delivery.

Verified: NamingConflictTest, DependencyTest and UpdateOperationTest (26 tests), and the whole org.opends.server.replication.plugin package (185 tests) - all pass.