Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUMM-2494 Use View hashcode as unique identifier for mapped Wireframe #1037

Conversation

mariusc83
Copy link
Collaborator

What does this PR do?

In this PR we are fixing the collision problem when taking the screen snapshot as a tree of wireframes. We were currently using the following approach:
return if(view.id!=View.NO_ID) view.id else view.hashCode to resolve the view unique identifier to be used in the equivalent mapped Wireframe. Following our tests we cannot rely on the view.id property as this is not unique in a view tree and it can actually be the same for more views in a tree creating collisions.
Instead we are going to use the View reference address returned by the View.hashCode function. In addition to this in order to avoid problems in case subclasses of the View class override the hashCode class we will use the System.identityHashCode function which will always return the default value from the default implementation of the hashCode function.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@mariusc83 mariusc83 self-assigned this Sep 8, 2022
@mariusc83 mariusc83 requested a review from a team as a code owner September 8, 2022 15:06
@codecov-commenter
Copy link

codecov-commenter commented Sep 8, 2022

Codecov Report

Merging #1037 (539775c) into feature/session-replay-vo (539775c) will not change coverage.
The diff coverage is n/a.

❗ Current head 539775c differs from pull request most recent head f860fd5. Consider uploading reports for the commit f860fd5 to get more accurate results

@@                    Coverage Diff                     @@
##           feature/session-replay-vo    #1037   +/-   ##
==========================================================
  Coverage                      83.32%   83.32%           
==========================================================
  Files                            327      327           
  Lines                          10454    10454           
  Branches                        1737     1737           
==========================================================
  Hits                            8710     8710           
  Misses                          1208     1208           
  Partials                         536      536           

Base automatically changed from mconstantin/rumm-2424/add-session-replay-v0-privacy-rules to feature/session-replay-vo September 12, 2022 07:37
@mariusc83 mariusc83 merged commit 7a7508e into feature/session-replay-vo Sep 12, 2022
@mariusc83 mariusc83 deleted the mconstantin/rumm-2494/wireframe-mapper-use-reference-address-as-view-id branch September 12, 2022 08:04
@xgouchet xgouchet added this to the internal milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants