[#939] Pin isServerFailure() directly, so the conflict result codes are guarded

[vharseko]

Fixes #939.

ds-cfg-server-error-result-code is a plain integer which is not validated as a result code, so it can be set to a code conflict resolution owns. #892 (issue #889) kept such a change out of isServerFailure() so that solveNamingConflict() gets it first, and CONFLICT_RESULT_CODES is the list that carve-out reads.

Only one of the six codes in that list is exercised, and only in one direction:

• UpdateOperationTest.changeConflictResolutionCanNotSolveOnTheServerErrorCodeIsRetried ([#889] Keep a change the replay could not apply out of the ServerState #892) sets the code to 53 - and No test for a ModifyDN conflict solved while server-error-result-code is one of the conflict codes #910 already recorded that it passes with 53 removed from the set, because the change is retried either way.
• NamingConflictTest.modifyDnConflictIsSolvedWhileTheServerErrorCodeIsOneOfTheConflictCodes ([#910] Test that a ModifyDN conflict is still solved while server-error-result-code is one of the conflict codes #938, open) watches 53 in the direction that discriminates.

So today, removing any of the six from the set leaves the whole suite green while putting the bug back: a replica whose administrator set the code to 32 would take every NO_SUCH_OBJECT conflict for a failure of the server, retry it in place, hold its ServerState back over it and give up on it once the give-up delay is spent - for a conflict solveNamingConflict() would have solved on the attempt it never got.

The test

IsServerFailureTest is a table of the predicate itself, extends DirectoryServerTestCase with no startServer() in it, so it runs in about a second. Not an end-to-end test per code: each would need a scenario failing with exactly that code which nothing but conflict resolution can apply, and OBJECTCLASS_VIOLATION has none a replayed operation reaches through the server's own paths - the schema checks which raise it are skipped for synchronization operations, which is the half of #910 that #938 could not write.

Two things had to be right for it to guard anything:

• The codes are written out, not read from the set. A data provider fed by CONFLICT_RESULT_CODES agrees with it whatever it holds - which is precisely what leaves the set unguarded today.
• A hand-written list only ever notices a code taken out of the set. So every result code an administrator could configure is swept through the predicate against that list, and a code put into the set is as visible as one taken out. UNDEFINED (-1) is left out of the sweep: the property has lower-limit="0", so it is not a setting. The sweep uses soft assertions, so a drift in both directions at once is reported in one run.

The named tables say the rest of the rule: a code which does not belong in the set - the default 80 among them - still reports the storage failing; a conflict which arrives while a different code is configured is conflict resolution's; and UNAVAILABLE is a failure of the server whatever is configured.

The production change

isServerFailure() becomes package private with a line in its javadoc saying why. @VisibleForTesting is what the three members of this class already exposed for the tests carry, and all three are public.

CONFLICT_RESULT_CODES stays private. An earlier round of this branch opened it to assert set equality directly; sweeping the predicate proves the same thing through observable behaviour, over every configurable code rather than a hand-picked few, and needs no second widening.

Checked against the mutations it exists for

All runs against the committed version.

mutation	result
baseline	23/23
drop NO_SUCH_OBJECT and OBJECTCLASS_VIOLATION from the set	3 failures - both named rows and the sweep
add OTHER (80) to the set	2 - the row for 80 and the sweep
add INVALID_DN_SYNTAX (34) to the set	1 - the sweep alone
add 19 and drop 66 at once	3 - the sweep names both codes in one run
drop the UNAVAILABLE disjunct	66 of the swept codes, in unavailableIsAServerFailureWhateverIsConfigured

The last two are why both halves are kept: the sweep cannot see the UNAVAILABLE disjunct going (with result == configured == UNAVAILABLE the invariant still holds through the second clause), and the named tables cannot see a code they do not name.

UpdateOperationTest.changeConflictResolutionCanNotSolveOnTheServerErrorCodeIsRetried was run against this branch as well: 1/1.

What this does not cover

• The set drifting away from the branches of solveNamingConflict() it is the union of: a code dropped from both at once leaves the table still describing the predicate correctly. Recorded in the class javadoc rather than papered over.
• The call site is not the predicate. replay() answers SUCCESS and NO_OPERATION before consulting it at all, which is A change which never applied is recorded as replayed when server-error-result-code is set to NO_OPERATION or SUCCESS #953; BUSY is answered ahead of it inside the retry loop, but reaches it once the in-place attempts are spent.

Found while writing this

Three issues, none touched here:

• A change which never applied is recorded as replayed when server-error-result-code is set to NO_OPERATION or SUCCESS #953 - a change which never applied is recorded as replayed when server-error-result-code is set to NO_OPERATION or SUCCESS.
• NPE in solveNamingConflict(ModifyDNOperation) when both the moved entry and its new parent are gone #955 - NPE in solveNamingConflict(ModifyDNOperation) when both the moved entry and its new parent are gone.
• A failed entryUUID search reads as a deleted entry, and conflict resolution records the change as replayed #956 - a failed entryUUID search reads as a deleted entry, and conflict resolution records the change as replayed. This one bites at the default configuration too.