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

fix: Check for equality of value instead of equality of instance. #1101

Merged

Conversation

michael-simons
Copy link
Contributor

isCustomized returned the wrong value for all instances that have been serialized and read back as they obviously does not point to the default instance.

The solution is to check for equality of the values, not for equality of the instances. This is a minor change in behaviour: A setting that is created with the builder matching the exact same defaults is now considered to be not customized.
This solution has been preferred over a custom readResolve returning the DEFAULT when things match as it would otherwise be able to create an instance that identifies itself as customized but changed to not customized when read back, basically the oppositte of what is fixed here: An instances created as not customized must never be read back as customized.

A couple of tests needed to be adapted due to the fact that the previous test did not actually change the default value.

isCustomized returned the wrong value for all instances that have been serialized and read back as they obviously does not point to the default instance.

The solution is to check for equality of the values, not for equality of the instances. This is a minor change in behaviour: A setting that is created with the builder matching the exact same defaults is now considered to be not customized.
This solution has been preferred over a custom readResolve returning the DEFAULT when things match as it would otherwise be able to create an instance that identifies itself as customized but changed to not customized when read back, basically the oppositte of what is fixed here: An instances created as not customized must never be read back as customized.

A couple of tests needed to be adapted due to the fact that the previous test did not actually change the default value.
@michael-simons michael-simons changed the base branch from 5.0 to 4.4 December 13, 2021 11:51
Copy link
Member

@dotStart dotStart left a comment

Choose a reason for hiding this comment

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

📦

@michael-simons michael-simons merged commit 7a81b47 into neo4j:4.4 Dec 14, 2021
@michael-simons michael-simons deleted the issue/secsettings-serialized-4.4 branch December 14, 2021 10:04
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.

2 participants