Skip to content

Portlet mapping predicate compareTo is not transitive [SPR-9874] #14507

@spring-projects-issues

Description

@spring-projects-issues

Eric Dalquist opened SPR-9874 and commented

While the obvious part of the problem where the wrong handler is found has been fixed by #13941 and #14239 the compareTo implementation of the ActionMappingPredicate, EventMappingPredicate, RenderMappingPredicate, and ResourceMappingPredicate is still incorrect.

The compareTo javadoc states
The implementor must also ensure that the relation is transitive: (x.compareTo(y)>0 && y.compareTo(z)>0) implies x.compareTo(z)>0

The problem here is any two predicates of different types that both implement SpecialRequestTypePredicate are treated as equal.

For example with three predicates defined as:
A = RenderMappingPredicate(windowState=null, params=[])
B = ResourceMappingPredicate(resourceId="b");
C = RenderMappingPredicate(windowState=null, params=["c=c"])

If they are placed in an array as [A, B, C] then comparing them A.compareTo(B) == 0 and B.compareTo(C) == 0 happens when sorting and the list is "sorted" and implies that A.compareTo(C) == 0 even though A.compareTo(C) == -1

I believe the solution is that when comparing SpecialRequestTypePredicates that are different concrete classes the returned value should be the result of "this.getClass().getName().compareTo(other.getClass().getName())". This will result in predicates of the same types being grouped together and then being sorted by the type specific comparison logic. It should also completely fulfill the compareTo contract.

While there isn't currently a demonstrable bug with the code due to the change in the mapping resolution logic made for #13941 fixing compareTo will result help protect against potential future bugs where someone assumes a correctly implemented compareTo method.


Affects: 3.1.3, 3.2 RC1

Referenced from: commits 8f8e517, a6bcda2

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions