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]

Reply via email to