[#150] Keep OrFilter.getRight() an OR for three or more sub-filters

[vharseko]

Fixes #150

Problem

When an OrFilter holds three or more sub-filters, OrFilter.getRight() wraps all of them except the first in an AndFilter. FilterBuilder.or(...) builds exactly such a flat OrFilter. Any code that walks it through getLeft()/getRight() therefore turns or(a, b, c) into OR(a, AND(b, c)), and an object that matches only b or only c stops matching.

The affected callers are ObjectNormalizerFacade.normalizeFilter, AbstractFilterTranslator (and every connector built on it), the FilterHandlers serialization to a remote connector server, and the groovy MapFilterVisitor. OrFilter.accept(ConnectorObject) iterates over the flat list and is not affected.

Change

• OrFilter.getRight() now returns new OrFilter(right). This one line fixes every caller listed above.
• FilterBuilderTests:
  • orFilterRightSideKeepsOrForThreeOrMoreSubFilters checks that getRight() of a three-way OR is a two-way OrFilter. It also checks that rebuilding three- and four-way ORs through getLeft()/getRight() keeps the match. This test failed before the fix.
  • andFilterRightSideKeepsAndForThreeOrMoreSubFilters pins the matching behaviour of AndFilter, which was already correct.

Mixed-version deployments

The remote connector server path is fixed once the side that serializes the filter (the client) runs this framework. CompositeFilterHandler.serialize calls getLeft()/getRight() on the sender, so an upgraded connector server still receives OR(a, AND(b, c)) from an older client and cannot tell that AND from a real one. A new client talking to an older server works correctly: it sends a binary tree, and a binary OrFilter never reaches the broken branch.

Testing

• connector-framework: FilterBuilderTests and FilterTranslatorTests pass (23 tests).
• connector-framework-internal: ObjectNormalizerFacadeTests, ObjectSerializationTests and LocalConnectorInfoManagerTests pass (114 tests).
• One full run of connector-framework-internal hit an unrelated race in LocalConnectorInfoManagerTests.testBatchUseCase3Failure, reported as Flaky LocalConnectorInfoManagerTests.testBatchUseCase3Failure: asserts no batch results right after executeBatch #151. The test passed on the rerun.

[maximthomas]

praise: The fix goes where the bug is, and the test proves it.

• OrFilter.getRight() now returns new OrFilter(right) (OrFilter.java:84). That single line fixes every getLeft()/getRight() walker, so no walker needs its own patch.
• orFilterRightSideKeepsOrForThreeOrMoreSubFilters fails when the line is reverted to new AndFilter(right) (assertion at FilterBuilderTests.java:243) and passes with the fix.

---

suggestion (non-blocking): The description says this line fixes the FilterHandlers remote path. That is only true once the client that sends the filter runs this framework.

OpenICF-java-framework/connector-framework-internal/src/main/java/org/identityconnectors/framework/impl/serializer/FilterHandlers.java:72-73, :63-66

CompositeFilterHandler.serialize calls getLeft()/getRight() on the sending side, and deserialize rebuilds a binary filter. This means an n-ary OR is rewritten before it goes on the wire. Suppose a connector server runs this fix but the client does not. The client sends OrFilter{a, AndFilter{b, c}}, and the server drops objects that match only b or only c. The server cannot repair this, because that AND looks exactly like a real one. The reverse case, a new client with an old server, works correctly. The PR body becomes the squash commit message, so one sentence there will save debugging time in mixed-version deployments:

The remote connector server path is fixed once the side that serializes the filter (the client) runs this framework: `CompositeFilterHandler.serialize` calls `getLeft()`/`getRight()` on the sender, so an upgraded connector server still receives `OR(a, AND(b, c))` from an older client.

[vharseko]

Added a "Mixed-version deployments" section to the description with the client-side caveat and the new-client/old-server case.

Also rebased the branch onto current master (now on top of #145 and #139); no conflicts, and FilterBuilderTests, FilterTranslatorTests, ObjectNormalizerFacadeTests and ObjectSerializationTests pass on the rebased head 11f569ef.