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

[rrd4j] Avoid IAE thrown if e.g. invalid start/end time given #14238

Merged
merged 2 commits into from
Jan 20, 2023

Conversation

cweitkamp
Copy link
Contributor

@cweitkamp cweitkamp commented Jan 17, 2023

  • Avoid IAE thrown in constructor of FetchRequest called by createFetchRequest method (e.g. invalid start/end time given)
  • Enable requesting future time ranges including boundary values via REST API

See https://github.com/rrd4j/rrd4j/blob/85183713741c648b88926ed3085c4dd87fd9016f/src/main/java/org/rrd4j/core/FetchRequest.java

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp cweitkamp added the bug An unexpected problem or unintended behavior of an add-on label Jan 17, 2023
@cweitkamp cweitkamp requested a review from a team as a code owner January 17, 2023 14:37
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. I have left one comment with a suggestion. Unless I'm mistaken this would fix the root cause rather than the symptom.

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp cweitkamp changed the title [rrd4j] Catch IAE thrown if e.g. invalid start/end time given [rrd4j] Avoid IAE thrown if e.g. invalid start/end time given Jan 20, 2023
Comment on lines +321 to +327
// do not call method {@link RrdDb#createFetchRequest(ConsolFun, long, long, long)} if start > end to avoid
// an IAE to be thrown
if (start > end) {
logger.warn("Could not query rrd4j database for item '{}': start ({}) > end ({})", itemName, start,
end);
return List.of();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We could consider throwing IllegalArgumentException by ourselves here, since this would now be a bug on caller side, where previously it was an internal bug within this method. But let's leave something for future improvement also. 🙂

Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and patience.

@jlaur jlaur merged commit a78db1f into openhab:main Jan 20, 2023
@jlaur jlaur added this to the 4.0 milestone Jan 20, 2023
@cweitkamp cweitkamp deleted the bugfix-rrd4j-iae branch January 20, 2023 11:13
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
…b#14238)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
renescherer pushed a commit to renescherer/openhab-addons that referenced this pull request Mar 23, 2023
…b#14238)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
FordPrfkt pushed a commit to FordPrfkt/openhab-addons that referenced this pull request Apr 20, 2023
…b#14238)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants