Conversation
…sitor Closes java/chained-type-tests for ObjectNormalizerFacade.normalizeFilter (12 tests) and AbstractFilterTranslator.createLeafExpression (9 tests): both already had a real Filter type hierarchy and a FilterVisitor interface available, so the instanceof chains become proper visitor dispatch instead of a mechanical alternative. Behavior-preserving, including a null-safety edge case the first pass of this refactor briefly broke (caught by the existing test suite, fixed, and locked in with a new regression test). The other two java/chained-type-tests alerts (EqualsHashCodeBuilder, SQLUtil) are dismissed on GitHub as won't-fix: they dispatch on final JDK types (primitive array component types / Integer, Double, Blob, Timestamp, ...) that a visitor pattern cannot be added to.
vharseko
added
java
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the last remaining CodeQL
note-severity category,java/chained-type-tests(4 alerts).ObjectNormalizerFacade.normalizeFilter(12-way instanceof chain) andAbstractFilterTranslator.createLeafExpression(9-way instanceof chain) both dispatch on the framework's ownFiltersubtypes, and the framework already ships aFilterVisitorinterface for exactly this purpose (seeFilteredResultsHandlerVisitorfor a prior-art example). Rewrote both as proper visitor dispatch instead of an instanceof chain.AbstractFilterTranslator's visitor is a private, non-static inner class so itsvisitXxxFiltermethods reach the enclosing instance'sprotected createXxxExpression(...)methods — preserving the class's whole point, which is letting connector subclasses override those methods.instanceofonnullis alwaysfalse, falling through to theelsebranch), butfilter.accept(...)throwsNullPointerExceptionon anullfilter. Fixed in both files (restored the null check at the entry points, and routed the AND/OR/NOT recursion inObjectNormalizerFacadeback through the null-checked public method rather than calling.acceptdirectly on possibly-null sub-filters). Added a regression test (testNullFilterReturnsNull) plus one for the previously-untested presence-filter passthrough branch (testPresenceFilterPassedThroughUnchanged).java/chained-type-testsalerts —EqualsHashCodeBuilder.append(dispatches on primitive array component types) andSQLUtil's JDBC parameter binding (dispatches onInteger/Double/Blob/Timestamp/...) — are dismissed on GitHub as won't-fix: both dispatch on final JDK types we don't own, so a visitor/polymorphism-based fix is not possible.Test plan
ObjectNormalizerFacadeTests.testPresenceFilterPassedThroughUnchanged,testNullFilterReturnsNull.mvn install -DskipITsonconnector-framework,connector-framework-internal,OpenICF-ldap-connector,OpenICF-dbcommon,OpenICF-xml-connector(incl. their existing filter-translator test suites) — all green, 988 tests, 0 failures.