Skip to content

Handle comparison errors in "sort" filter - #327

Open
kohlschuetter wants to merge 1 commit into
bkiers:masterfrom
kohlschuetter:ck/SortComparable
Open

kohlschuetter wants to merge 1 commit into
bkiers:masterfrom
kohlschuetter:ck/SortComparable

Conversation

@kohlschuetter

Copy link
Copy Markdown
Contributor

It is not guaranteed that all objects in a sortable list are of the same type. We currently assume that all objects are comparable via Comparable. This unfortunately is not the case, as most implementations only allow comparing to other instances of the same type (e.g., String vs. String, not String vs. CharSequence), throwing a ClassCastException upon mismatch.

Use a custom comparator that handles this case, first trying to compare the other way around (assuming that the custom RenderTransformer object implementation can indeed compare against Strings), falling back to comparing the outputs of toString() for the two objects.

An on-demand toString() cache is used to handle duplicate toString() conversions.

It is not guaranteed that all objects in a sortable list are of the same
type. We currently assume that all objects are comparable via
Comparable<Object>. This unfortunately is not the case, as most
implementations only allow comparing to other instances of the same
type (e.g., String vs. String, not String vs. CharSequence), throwing a
ClassCastException upon mismatch.

Use a custom comparator that handles this case, first trying to compare
the other way around (assuming that the custom RenderTransformer object
implementation can indeed compare against Strings), falling back to
comparing the outputs of toString() for the two objects.

An on-demand toString() cache is used to handle duplicate toString()
conversions.
@pmenhart

Copy link
Copy Markdown
Contributor

The stringCache grows unbounded?
Some applications use this library to process huge amount of data, having a cache that is not controlled for size is a problem.
Also, can you add a unit test to illustrate all branches of comparison, please?

@kohlschuetter

Copy link
Copy Markdown
Contributor Author

We create a new ObjectComparator per call. The stringCache is bounded by the number of elements to sort, and it is only instantiated if the inverse comparison fails (which doesn't if you use stringhold's StringHolderRenderTransformer, for example, since StringHolder can compare against String).

@kohlschuetter

kohlschuetter commented Sep 10, 2025 •

Copy link
Copy Markdown
Contributor Author

The comparison is trivial, not sure if we need to add a unit test for this. To be serious, we would also have to replicate the situation triggering the bug, which requires quite some setup (adding a dependency to stringhold, etc.), so not worth imho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants