Dhairya-007 opened a new pull request, #29383:
URL: https://github.com/apache/camel/pull/29383
Three related defects in `EipAttribute`, all in the equality/ordering
contract.
### 1. `equals` without `hashCode`
`EipAttribute` is the only class under `src/main` in the whole repository
that overrides `equals` and not `hashCode` (I scanned for it). Two attributes
that compare equal therefore get different hash codes, so both survive in a
`HashSet` and act as separate `HashMap` keys.
### 2. `equals` throws on an attribute with no id
```java
return id.equals(((EipAttribute) o).id);
```
`id` is only populated when the deserialized key happens to be `id`.
`Components.setAttribute` builds an attribute from `{"<key>":"<value>"}` for
whatever key it is given, so an attribute built from any other key has a null
`id`, and comparing it throws `NullPointerException`. `Objects.equals` handles
it.
### 3. `compareTo` overflows
```java
return index - o.index;
```
Subtracting indexes overflows. With `index = Integer.MAX_VALUE` compared
against `-1`, this returns `Integer.MIN_VALUE`, so the larger index sorts
first. `Integer.compare` is total.
### How far this reaches
The production path in `CoverageResultsProcessor` only iterates these lists,
so as far as I can tell none of this misbehaves in the plugin as it stands
today. These are latent contract violations on a public model class rather than
a reproducible failure in the report output, and I did not want to overstate
that.
### Verification
`catalog/camel-report-maven-plugin` depends on `camel-core`, `camel-catalog`
and `camel-allcomponents` at the current SNAPSHOT, so running just this module
locally would mean building most of Camel. I checked the three behaviours
against the old and new code in a standalone harness instead:
```
before after
equals with a null id -> NPE -> false
compareTo(MAX_VALUE, -1) -> -2147483648 -> 1
HashSet of two equal attributes -> 2 -> 1
```
The new tests in `EipAttributeTest` cover the same three cases, and
`testEqualsWithoutId` fails with a `NullPointerException` against the current
code.
`compareTo` orders by `index` while `equals` compares `id`, so the ordering
stays inconsistent with equals. I left that alone, since the index ordering
looks deliberate for report output and changing it would alter the generated
report.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]