AVRO-3956: [Java] Fix NPE in Protocol#equals #2791
Conversation
mitasov-ra
force-pushed
the
bugfix/npe-in-protocol
branch
from
a3da6fb
to
af974cd
Compare
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return name.hashCode() + namespace.hashCode() + types.hashCode() + messages.hashCode() + propsHashCode(); | ||
| return name.hashCode() + Objects.hash(namespace) + types.hashCode() + messages.hashCode() + propsHashCode(); |
There was a problem hiding this comment.
Why not add the other parameters to Objects.hash? That would read even better.
There was a problem hiding this comment.
New hash would be backward incompatible with the old one in that case. If that's ok, I do not see any problems switching to Objects.hash
There was a problem hiding this comment.
@opwvhk So what do you think?
There was a problem hiding this comment.
Backwards compatibility is not an issue.
The Protocol#hashCode() method has never specified differently, and thus uses the general contract of hashCode(). That states that multiple invocations on the same object, if unchanged, yield the same result. However, it is explicitly stated that the result "need not remain consistent from one execution of an application to another execution of the same application".
See https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Object.html#hashCode()
There was a problem hiding this comment.
Thanks! I've updated Protocol#hashCode
|
@mitasov-ra Please file a Jira issue and update the PR title/description. |
mitasov-ra
changed the title
|
Thank you, @mitasov-ra ! |
* Fix NPE in Protocol#equals * Better Protocol#hashCode
AVRO-3956
What is the purpose of the change
Fix of NPE in Protocol#equals.
As of documentation,
namespaceis optional inProtocoland thus can benull.Protocol#equalscontains direct dereference ofnamespace, which causesNullPointerExceptionwhen comparing Protocols withnullinnamespace.This MR fixes that.
Verifying this change
(Please pick one of the following options)
This change is a trivial rework / code cleanup without any test coverage.
Documentation