Commit 766833c
authored
Fix/5.x whole time units (#2976)
* Whole time units from TimeSpan
This commit changes the implementation of Time within the client for better consistency with Elasticsearch's TimeValue. The Time type in NEST 5.x is used to represent both Elasticsearch's TimeValue (which allows from nanos up to days as unit), and as time modifiers within DateMath expressions, for which Elasticsearch accepts units of y (years) M (months), w (weeks), d (days), h/H (hours), m (minutes) and s (seconds).
- When converting from a TimeSpan to Time, the serialized value is a whole number with the largest unit of days. This fixes the implementation to be in line with Elasticsearch 5.0+ with elastic/elasticsearch#19102 where weeks was removed as a supported unit for TimeValue.
- When constructing a Time using the constructor that takes a double of milliseconds, make the implementation consistent with the implicit conversion from double implementation; in the case of -1, both should be Time.MinusOne and the case of 0, both should be Time.Zero. If needing to express -1ms or 0ms, the constructor taking a factor and unit, or the constructor taking a string value with unit should be used.
- When constructing a Time from double or TimeSpan, try to reduce the value to a whole number factor and largest represented unit (up to unit of days). For values smaller than 1 nanosecond, retain the factor as a fraction of nanoseconds.
- When converting a Time to a TimeSpan, round the value to the nearest TimeSpan tick for microseconds and nanoseconds, and milliseconds for all other units.
- When converting to a Time within a DateMath expression using Time.ToFirstUnitYieldingInteger(), when the value is fractional nanoseconds, round to the nearest nanosecond. Although this method is only used within the client for DateMath expressions and can return times with units smaller than a second (the smallest unit that DateMath within Elasticsearch accepts), the method is public so could be used by consumers for purposes other than DateMath. Because of this, don't break the implementation for units smaller than one second.
- Allow the implicit conversion from string and the constructor that takes a string to continue to accept fractional units, which can be returned from Elasticsearch as per TimeValue's toString implementation https://github.com/elastic/elasticsearch/blob/f2501130399f6e3788e110d74dd62b3aad66804a/core/src/main/java/org/elasticsearch/common/unit/TimeValue.java#L253-L290
- Update documentation
Closes #2818
* Update DateMath documentation
* Additional Time unit tests
Add XML documentation to Time methods to describe what ToTimeSpan() and ToFirstUnitYieldingInteger() will do
Simplify multiplication by reciprocals
* Support months and weeks in ToFirstUnitYieldingInteger()
ToFirstUnitYieldingInteger() is used in NEST only within DateMath expressions. Support whole values in months and weeks.1 parent d58e712 commit 766833c
File tree
5 files changed
+583
-316
lines changed- docs/common-options
- date-math
- time-unit
- src
- Nest/CommonOptions/TimeUnit
- Tests/CommonOptions
- DateMath
- TimeUnit
5 files changed
+583
-316
lines changedLines changed: 22 additions & 15 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
137 | 137 | | |
138 | 138 | | |
139 | 139 | | |
140 | | - | |
141 | | - | |
| 140 | + | |
| 141 | + | |
142 | 142 | | |
143 | 143 | | |
144 | 144 | | |
145 | | - | |
146 | | - | |
147 | | - | |
148 | | - | |
149 | | - | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
154 | 150 | | |
155 | 151 | | |
156 | 152 | | |
157 | 153 | | |
158 | 154 | | |
159 | 155 | | |
160 | 156 | | |
161 | | - | |
162 | | - | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
163 | 169 | | |
164 | | - | |
165 | | - | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
166 | 173 | | |
167 | 174 | | |
0 commit comments