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

Make alert conditions resilient against changing number formats. #2182

Merged
merged 6 commits into from
May 9, 2016

Conversation

dennisoelkers
Copy link
Member

In 1.3.x numerical alert condition parameters were stored by MongoDB
(resp. the mongo java driver) in a way, that they are returned as
Doubles to the server, while alert conditions persisted in 2.0.0 return
numerical parameters as Integer. Therefore, 2.0.0 cannot handle alert
conditions stored in 1.3.x, so legacy installations upgraded to 2.0.0
do not check any alert conditions anymore.

This change allows to handle both formats by generally assuming they are
Number/Double types and returning their intValue().

Fixes #2169

@dennisoelkers dennisoelkers added this to the 2.0.1 milestone May 4, 2016
@dennisoelkers
Copy link
Member Author

Adding some tests as well.

In 1.3.x numerical alert condition parameters were stored by MongoDB
(resp. the mongo java driver) in a way, that they are returned as
Doubles to the server, while alert conditions persisted in 2.0.0 return
numerical parameters as Integer. Therefore, 2.0.0 cannot handle alert
conditions stored in 1.3.x, so legacy installations upgraded to 2.0.0
do not check any alert conditions anymore.

This change allows to handle both formats by generally assuming they are
Number/Double types and returning their `intValue()`.

Fixes #2169
This also helps us, because the "threshold" parameter could also be a
double in some cases.
@@ -195,4 +187,16 @@ public NegativeCheckResult(AlertCondition alertCondition) {
}
}

@VisibleForTesting
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor semantic nitpick: Child classes are also using this (protected) method, so it's not just visible for testing. 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

@joschi joschi self-assigned this May 4, 2016

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't use wildcard imports.

Configuration for IntelliJ IDEA: http://stackoverflow.com/a/3348855

Copy link
Member Author

Choose a reason for hiding this comment

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

@joschi
Copy link
Contributor

joschi commented May 4, 2016

LGTM (except for those 2 minor nitpicks). 👍

@joschi
Copy link
Contributor

joschi commented May 9, 2016

LGTM. 👍

@joschi joschi merged commit d164e7f into 2.0 May 9, 2016
@joschi joschi deleted the issue-2169 branch May 9, 2016 08:59
dennisoelkers added a commit that referenced this pull request May 9, 2016
In 1.3.x numerical alert condition parameters were stored by MongoDB
(resp. the Mongo Java Driver) in a way, that they are returned as
Double  while alert conditions persisted in 2.0.0 return numerical parameters
as Integer. Therefore, 2.0.0 cannot handle alert conditions stored in 1.3.x
and legacy installations upgraded to 2.0.0 do not check any alert conditions
anymore.

This change allows to handle both formats by generally assuming they are
Number/Double types and returning their `intValue()`.

Fixes #2169
(cherry picked from commit d164e7f)
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