[#1173] Check the references of every managed attribute type of an add and every modification of a modify, not only the first

[vharseko]

Fixes #1173

Problem

With check-references: true, the Referential Integrity plugin checks references in its two doPreOperation hooks, and both returned after their first check:

if (result.getResultCode() != ResultCode.SUCCESS)
{
  return result;
}

A reference that passes yields PluginResult.PreOperation.continueOperationProcessing(), whose result code is null, not SUCCESS, so the condition also held for a check that passed. As a result:

• add: only the first managed attribute type was checked. An entry that did not hold that attribute had none of its references checked, because an empty attribute list passes too.
• modify: only the first ADD or REPLACE modification of a managed attribute was checked, and the ones after it were not.

Fix

Both hooks now return early only when processing must stop (!result.continueProcessing()). isIntegrityMaintained(List<Attribute>, …) already worked that way: it compares with continueOperationProcessing().

Tests

ReferentialIntegrityPluginTestCase, with check-references on manager and seeAlso below dc=example,dc=com, and the data provider missingReferenceNextToAnother: a missing reference in one attribute, next to a valid reference in the other attribute or alone. Both attributes take both places, because the order in which the plugin checks attribute types is an implementation detail.

• testEnforceIntegrityAddChecksEveryAttributeType: adding an entry with a missing reference in either attribute is refused with CONSTRAINT_VIOLATION.
• testEnforceIntegrityModifyChecksEveryModification: a modify request whose second modification adds a missing reference is refused. Its first modification replaces the other managed attribute with a valid reference, or, in the rows without one, replaces description, which the plugin does not manage.

The plugin configuration these tests share moved into a helper, enableCheckReferences.

Without the fix, 4 of the 69 tests fail with expected [Constraint Violation] but found [Success]:

• add [seeAlso, manager] and [seeAlso, null], because the plugin checks manager first;
• modify [manager, seeAlso] and [seeAlso, manager].

The two modify rows without a valid reference passed even before the fix, since an unmanaged first modification was already skipped; they guard that case. With the fix the class passes 69/69.

Found while working on #1172 (PR #1174), whose test has to check continueProcessing() for the same reason.

[maximthomas]

praise: The fix changes exactly the two conditions that caused #1173, and the new tests fail without it.

• doPreOperation(PreOperationModifyOperation) at :1093 and doPreOperation(PreOperationAddOperation) at :1127 now return only on !result.continueProcessing(). That matches isIntegrityMaintained(List<Attribute>, …), which already compared against continueOperationProcessing(), the DEFAULT_RESULT whose result code is null (PluginResult.java:360).
• missingReferenceNextToAnother puts each of manager and seeAlso in both positions. Without the fix, the add rows whose missing reference sits in the second attribute type checked fail, and so do modify rows 1-2, whichever order the plugin iterates in. CI ran the class on Linux and all 69 tests passed.