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.

Sign up for GitHub

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

Jump to bottom

Fix/attribute null #750

Merged
merged 2 commits into from
Jul 1, 2020
Merged

Fix/attribute null #750

merged 2 commits into from
Jul 1, 2020

Conversation

diba1013
Copy link
Contributor

Before submission, please check that ...

  • the code follows the Clean Code guidelines.
  • the necessary tests are either created or updated.
  • all status checks (Travis CI, SonarCloud, etc.) pass.
  • your commit history is cleaned up.
  • you updated the changelog.
  • you updated necessary documentation within retest/docs.

Description

TL;DR Fix NPE when attribute is inserted and applied

This is somewhat caused by #748 which destroys the attribute differences.

When performing the autohealing tutorial applying the text attribute difference (which is an identifying attribute) results in an exception reported, because the attribute is missing and cannot be applied.

java.lang.NullPointerException: Cannot apply change to an attribute that is null.

Fix

The changes in #748 causes the attribute difference to be reconstructed. This destroys any special attribute differences (e.g. AdditionalAttributeDifference), which are used to convey additional meaning to the IdentifyingAttributes apply mechanism.

This was used to pass the original inserted attribute along and ignore the missing null attribute to simply create a new attribute (see AdditionalAttributeDifference). However, decided to custom handle the insertion of a (identifying) attribute the following:

  1. If an attribute is present: Use the difference to apply.
  2. If an attribute is missing: Check the actual value for attribute or try to create a new default attribute.

State of this PR

I decided not to revert or adjust the changes made by the causing PR, since most custom AttributeDifferences are related to legacy code (to my understanding) and should not be created. The remaining differences (mostly AdditionalAttributeDifference) should not require special handling (besides the one to be introduced).

Additional Context

This bug has happened a few times in the past and couldn't be reproduced, since it seemed to happen only on edge cases. I hope, that this fix also fixes those cases.

diba1013 added 2 commits June 30, 2020 14:54
Add failing test to reproduce inserted elements
f9dccd4
Rewrite apply to ensure inserted are added
a668195 
which should be represented as AdditionalAttributeDifference so that the original attribute can be restored.
This is important, since attributes have different weights (e.g. TextAttribute).
If for some reason that is not represented, a default attribute is created.
@diba1013 diba1013 added the bug Something isn't working label Jun 30, 2020
@diba1013 diba1013 requested a review from githubert June 30, 2020 14:05
@diba1013 diba1013 self-assigned this Jun 30, 2020
@cla-bot cla-bot bot added the cla-signed label Jun 30, 2020
@diba1013 diba1013 mentioned this pull request Jun 30, 2020
Open
githubert
githubert approved these changes Jul 1, 2020
View reviewed changes
Copy link

@githubert githubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mergify mergify bot merged commit d816556 into hotfix/v1.11.2 Jul 1, 2020
@mergify mergify bot deleted the fix/attribute-null branch July 1, 2020 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@githubert githubert githubert approved these changes

Assignees

@diba1013 diba1013

Labels
bug Something isn't working cla-signed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@diba1013 @githubert