JAVA-952 Tests for validating equals/hashCode implementations and include MaterializedViewMetadata#whereClause in equals/hashCode#471
JAVA-952 Tests for validating equals/hashCode implementations and include MaterializedViewMetadata#whereClause in equals/hashCode#471
Conversation
There was a problem hiding this comment.
I noticed that 'tables' is included in equals/hashCode, but not views/userTypes/functions/aggregates. Should that be considered a bug?
|
Just realized that some of the updates i've made to equals/hashCode should also be applied on older branches, so would be good to mark this as a 'conceptual' PR (work in progress). Would like feedback on the changes / approach and then can open PRs for equals/hashcode fixes as applicable on older branches first. |
c387251 to
2a0f57a
Compare
There was a problem hiding this comment.
simple names are derived from other fields so don't need to be included, codec doesn't seem relevant either.
|
rebased on 3.0 the only change that wasn't already in 3.0 was MaterializedViewMetadata equals/hashCode missing whereClause. |
2a0f57a to
a127d44
Compare
|
Closing this as it needs to be rebased/revisited, will open up a new PR later. |
There have been a few occasions now where we've added fields to classes and missed updating equals/hashCode implementations. I thought it would be good to use equalsverifier to validate that equals and hashCode implementations are correct where applicable. Having tests for classes that depend on equals/hashCode will 'future-proof' any changes to these classes by detecting missing field use in these methods.
I didn't implement this comprehensively for all classes overriding equals/hashCode in the driver, just the ones that were related to schema.
I chose to make this change on 3.0 since it adds some API restrictions (marking methods final that previously weren't). We could make this worker on older branches but not sure if it's worth it since schema changes seem less likely.
I found a variety of issues which were since fixed. The only remaining issue I've identifier that should probably be definitively fixed is:
There are a few cases where its debatable where certain fields need to be included in equals/hashCode as they may not factor in what we use these methods for (i.e. keyspace/table parentage of the type). I've added comments in the test classes where this is applicable.
Source-Level Changes: