[BEAM-2482] CodedValueMutationDetector should use the coders structural value#3406
[BEAM-2482] CodedValueMutationDetector should use the coders structural value#3406evindj wants to merge 5 commits intoapache:masterfrom
Conversation
…er than encoded- decoded byte arrays
| } | ||
| // ArrayList, which will then be re-encoded in a bounded format. So we get a structural value | ||
| // from a coder and used that to check if the byte array[] are the same. The structuralValue() | ||
| // method of the Coder returns a StructuralByteArray object. |
There was a problem hiding this comment.
This is not always the case; the Coder can understand that it has an object with a good equals method, and return the object directly.
I'm not sure if I agree that this is a desired change. We'll already have compared the final state of the original object to its clone in the original state, and the clone of the original object to the clone of original object in its final state, so we've already done what is appreciably a fast path, and now are checking the most generous possible "equality" in terms of java.
There was a problem hiding this comment.
Hi @lukecwik Please I would like to have your point of view here, do not worry about additional changes I'll revert them
There was a problem hiding this comment.
The jira issue could be worded better but my intent was we should rely only on the structural value comparison and drop all the other checks which are occurring within this method. Contractually the coder is required to return a value with a good equals method (not needing the usage of deep equals or cloning).
|
I had too much trouble trying to revert some of the changes here. I decided to finally just close this PR and open a new one. |
5 participants
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify.<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.