Skip to content

Commit bca6b60

Browse files
modular-magicianmodular-magician
authored
Fix issues with unpopulated gc_rules and handling of Xd duration (#8686) (#15595)
* Fix issues with unpopulated gc_rules and handling of Xd duration * Fix copt/paste issues * Extract duration_parsing_helper.go * Extract duration_parsing_helper.go * Add unit tests * Add time.UNIT * Fixed duration helper * Add better handling for days and hours * Add better handling for days and hours * fixed issue with diffs between gc_rules * minor fix * Add tests * package name fix * fix tests * fix tests * fix tests * fix tests * fix tests Signed-off-by: Modular Magician <[email protected]>
1 parent 1903caa commit bca6b60

6 files changed

+484
-8
lines changed

‎.changelog/8686.txt

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
```release-note:bug
2+
bigtable: fixed permadiff in `google_bigtable_gc_policy.gc_rules` when `mode` is specified
3+
```
4+
```release-note:bug
5+
bigtable: fixed permadiff in `google_bigtable_gc_policy.gc_rules` when `max_age` is specified using increments larger than hours
6+
```

‎google/services/bigtable/duration_parsing_helper.go

+223
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: MPL-2.0
3+
package bigtable
4+
5+
import (
6+
"errors"
7+
"time"
8+
)
9+
10+
var unitMap = map[string]uint64{
11+
"ns": uint64(time.Nanosecond),
12+
"us": uint64(time.Microsecond),
13+
"µs": uint64(time.Microsecond), // U+00B5 = micro symbol
14+
"μs": uint64(time.Microsecond), // U+03BC = Greek letter mu
15+
"ms": uint64(time.Millisecond),
16+
"s": uint64(time.Second),
17+
"m": uint64(time.Minute),
18+
"h": uint64(time.Hour),
19+
"d": uint64(time.Hour) * 24,
20+
"w": uint64(time.Hour) * 24 * 7,
21+
}
22+
23+
// ParseDuration parses a duration string.
24+
// A duration string is a possibly signed sequence of
25+
// decimal numbers, each with optional fraction and a unit suffix,
26+
// such as "300ms", "-1.5h" or "2h45m".
27+
// Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
28+
func ParseDuration(s string) (time.Duration, error) {
29+
// [-+]?([0-9]*(\.[0-9]*)?[a-z]+)+
30+
orig := s
31+
var d uint64
32+
neg := false
33+
34+
// Consume [-+]?
35+
if s != "" {
36+
c := s[0]
37+
if c == '-' || c == '+' {
38+
neg = c == '-'
39+
s = s[1:]
40+
}
41+
}
42+
// Special case: if all that is left is "0", this is zero.
43+
if s == "0" {
44+
return 0, nil
45+
}
46+
if s == "" {
47+
return 0, errors.New("time: invalid duration " + quote(orig))
48+
}
49+
for s != "" {
50+
var (
51+
v, f uint64 // integers before, after decimal point
52+
scale float64 = 1 // value = v + f/scale
53+
)
54+
55+
var err error
56+
57+
// The next character must be [0-9.]
58+
if !(s[0] == '.' || '0' <= s[0] && s[0] <= '9') {
59+
return 0, errors.New("time: invalid duration " + quote(orig))
60+
}
61+
// Consume [0-9]*
62+
pl := len(s)
63+
v, s, err = leadingInt(s)
64+
if err != nil {
65+
return 0, errors.New("time: invalid duration " + quote(orig))
66+
}
67+
pre := pl != len(s) // whether we consumed anything before a period
68+
69+
// Consume (\.[0-9]*)?
70+
post := false
71+
if s != "" && s[0] == '.' {
72+
s = s[1:]
73+
pl := len(s)
74+
f, scale, s = leadingFraction(s)
75+
post = pl != len(s)
76+
}
77+
if !pre && !post {
78+
// no digits (e.g. ".s" or "-.s")
79+
return 0, errors.New("time: invalid duration " + quote(orig))
80+
}
81+
82+
// Consume unit.
83+
i := 0
84+
for ; i < len(s); i++ {
85+
c := s[i]
86+
if c == '.' || '0' <= c && c <= '9' {
87+
break
88+
}
89+
}
90+
if i == 0 {
91+
return 0, errors.New("time: missing unit in duration " + quote(orig))
92+
}
93+
u := s[:i]
94+
s = s[i:]
95+
unit, ok := unitMap[u]
96+
if !ok {
97+
return 0, errors.New("time: unknown unit " + quote(u) + " in duration " + quote(orig))
98+
}
99+
if v > 1<<63/unit {
100+
// overflow
101+
return 0, errors.New("time: invalid duration " + quote(orig))
102+
}
103+
v *= unit
104+
if f > 0 {
105+
// float64 is needed to be nanosecond accurate for fractions of hours.
106+
// v >= 0 && (f*unit/scale) <= 3.6e+12 (ns/h, h is the largest unit)
107+
v += uint64(float64(f) * (float64(unit) / scale))
108+
if v > 1<<63 {
109+
// overflow
110+
return 0, errors.New("time: invalid duration " + quote(orig))
111+
}
112+
}
113+
d += v
114+
if d > 1<<63 {
115+
return 0, errors.New("time: invalid duration " + quote(orig))
116+
}
117+
}
118+
if neg {
119+
return -time.Duration(d), nil
120+
}
121+
if d > 1<<63-1 {
122+
return 0, errors.New("time: invalid duration " + quote(orig))
123+
}
124+
return time.Duration(d), nil
125+
}
126+
127+
var errLeadingInt = errors.New("time: bad [0-9]*") // never printed
128+
129+
// leadingInt consumes the leading [0-9]* from s.
130+
func leadingInt[bytes []byte | string](s bytes) (x uint64, rem bytes, err error) {
131+
i := 0
132+
for ; i < len(s); i++ {
133+
c := s[i]
134+
if c < '0' || c > '9' {
135+
break
136+
}
137+
if x > 1<<63/10 {
138+
// overflow
139+
return 0, rem, errLeadingInt
140+
}
141+
x = x*10 + uint64(c) - '0'
142+
if x > 1<<63 {
143+
// overflow
144+
return 0, rem, errLeadingInt
145+
}
146+
}
147+
return x, s[i:], nil
148+
}
149+
150+
// leadingFraction consumes the leading [0-9]* from s.
151+
// It is used only for fractions, so does not return an error on overflow,
152+
// it just stops accumulating precision.
153+
func leadingFraction(s string) (x uint64, scale float64, rem string) {
154+
i := 0
155+
scale = 1
156+
overflow := false
157+
for ; i < len(s); i++ {
158+
c := s[i]
159+
if c < '0' || c > '9' {
160+
break
161+
}
162+
if overflow {
163+
continue
164+
}
165+
if x > (1<<63-1)/10 {
166+
// It's possible for overflow to give a positive number, so take care.
167+
overflow = true
168+
continue
169+
}
170+
y := x*10 + uint64(c) - '0'
171+
if y > 1<<63 {
172+
overflow = true
173+
continue
174+
}
175+
x = y
176+
scale *= 10
177+
}
178+
return x, scale, s[i:]
179+
}
180+
181+
// These are borrowed from unicode/utf8 and strconv and replicate behavior in
182+
// that package, since we can't take a dependency on either.
183+
const (
184+
lowerhex = "0123456789abcdef"
185+
runeSelf = 0x80
186+
runeError = '\uFFFD'
187+
)
188+
189+
func quote(s string) string {
190+
buf := make([]byte, 1, len(s)+2) // slice will be at least len(s) + quotes
191+
buf[0] = '"'
192+
for i, c := range s {
193+
if c >= runeSelf || c < ' ' {
194+
// This means you are asking us to parse a time.Duration or
195+
// time.Location with unprintable or non-ASCII characters in it.
196+
// We don't expect to hit this case very often. We could try to
197+
// reproduce strconv.Quote's behavior with full fidelity but
198+
// given how rarely we expect to hit these edge cases, speed and
199+
// conciseness are better.
200+
var width int
201+
if c == runeError {
202+
width = 1
203+
if i+2 < len(s) && s[i:i+3] == string(runeError) {
204+
width = 3
205+
}
206+
} else {
207+
width = len(string(c))
208+
}
209+
for j := 0; j < width; j++ {
210+
buf = append(buf, `\x`...)
211+
buf = append(buf, lowerhex[s[i+j]>>4])
212+
buf = append(buf, lowerhex[s[i+j]&0xF])
213+
}
214+
} else {
215+
if c == '"' || c == '\\' {
216+
buf = append(buf, '\\')
217+
}
218+
buf = append(buf, string(c)...)
219+
}
220+
}
221+
buf = append(buf, '"')
222+
return string(buf)
223+
}

‎google/services/bigtable/duration_parsing_helper_test.go

+157
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: MPL-2.0
3+
package bigtable_test
4+
5+
import (
6+
"math"
7+
"math/rand"
8+
"strings"
9+
"testing"
10+
"time"
11+
12+
"github.com/hashicorp/terraform-provider-google/google/services/bigtable"
13+
)
14+
15+
var parseDurationTests = []struct {
16+
in string
17+
want time.Duration
18+
}{
19+
// simple
20+
{"0", 0},
21+
{"5s", 5 * time.Second},
22+
{"30s", 30 * time.Second},
23+
{"1478s", 1478 * time.Second},
24+
// sign
25+
{"-5s", -5 * time.Second},
26+
{"+5s", 5 * time.Second},
27+
{"-0", 0},
28+
{"+0", 0},
29+
// decimal
30+
{"5.0s", 5 * time.Second},
31+
{"5.6s", 5*time.Second + 600*time.Millisecond},
32+
{"5.s", 5 * time.Second},
33+
{".5s", 500 * time.Millisecond},
34+
{"1.0s", 1 * time.Second},
35+
{"1.00s", 1 * time.Second},
36+
{"1.004s", 1*time.Second + 4*time.Millisecond},
37+
{"1.0040s", 1*time.Second + 4*time.Millisecond},
38+
{"100.00100s", 100*time.Second + 1*time.Millisecond},
39+
// different units
40+
{"10ns", 10 * time.Nanosecond},
41+
{"11us", 11 * time.Microsecond},
42+
{"12µs", 12 * time.Microsecond}, // U+00B5
43+
{"12μs", 12 * time.Microsecond}, // U+03BC
44+
{"13ms", 13 * time.Millisecond},
45+
{"14s", 14 * time.Second},
46+
{"15m", 15 * time.Minute},
47+
{"16h", 16 * time.Hour},
48+
{"5d", 5 * 24 * time.Hour},
49+
// composite durations
50+
{"3h30m", 3*time.Hour + 30*time.Minute},
51+
{"10.5s4m", 4*time.Minute + 10*time.Second + 500*time.Millisecond},
52+
{"-2m3.4s", -(2*time.Minute + 3*time.Second + 400*time.Millisecond)},
53+
{"1h2m3s4ms5us6ns", 1*time.Hour + 2*time.Minute + 3*time.Second + 4*time.Millisecond + 5*time.Microsecond + 6*time.Nanosecond},
54+
{"39h9m14.425s", 39*time.Hour + 9*time.Minute + 14*time.Second + 425*time.Millisecond},
55+
// large value
56+
{"52763797000ns", 52763797000 * time.Nanosecond},
57+
// more than 9 digits after decimal point, see https://golang.org/issue/6617
58+
{"0.3333333333333333333h", 20 * time.Minute},
59+
// 9007199254740993 = 1<<53+1 cannot be stored precisely in a float64
60+
{"9007199254740993ns", (1<<53 + 1) * time.Nanosecond},
61+
// largest duration that can be represented by int64 in nanoseconds
62+
{"9223372036854775807ns", (1<<63 - 1) * time.Nanosecond},
63+
{"9223372036854775.807us", (1<<63 - 1) * time.Nanosecond},
64+
{"9223372036s854ms775us807ns", (1<<63 - 1) * time.Nanosecond},
65+
{"-9223372036854775808ns", -1 << 63 * time.Nanosecond},
66+
{"-9223372036854775.808us", -1 << 63 * time.Nanosecond},
67+
{"-9223372036s854ms775us808ns", -1 << 63 * time.Nanosecond},
68+
// largest negative value
69+
{"-9223372036854775808ns", -1 << 63 * time.Nanosecond},
70+
// largest negative round trip value, see https://golang.org/issue/48629
71+
{"-2562047h47m16.854775808s", -1 << 63 * time.Nanosecond},
72+
// huge string; issue 15011.
73+
{"0.100000000000000000000h", 6 * time.Minute},
74+
// This value tests the first overflow check in leadingFraction.
75+
{"0.830103483285477580700h", 49*time.Minute + 48*time.Second + 372539827*time.Nanosecond},
76+
}
77+
78+
func TestParseDuration(t *testing.T) {
79+
for _, tc := range parseDurationTests {
80+
d, err := bigtable.ParseDuration(tc.in)
81+
if err != nil || d != tc.want {
82+
t.Errorf("bigtable.ParseDuration(%q) = %v, %v, want %v, nil", tc.in, d, err, tc.want)
83+
}
84+
}
85+
}
86+
87+
var parseDurationErrorTests = []struct {
88+
in string
89+
expect string
90+
}{
91+
// invalid
92+
{"", `""`},
93+
{"3", `"3"`},
94+
{"-", `"-"`},
95+
{"s", `"s"`},
96+
{".", `"."`},
97+
{"-.", `"-."`},
98+
{".s", `".s"`},
99+
{"+.s", `"+.s"`},
100+
{"\x85\x85", `"\x85\x85"`},
101+
{"\xffff", `"\xffff"`},
102+
{"hello \xffff world", `"hello \xffff world"`},
103+
{"\uFFFD", `"\xef\xbf\xbd"`}, // utf8.RuneError
104+
{"\uFFFD hello \uFFFD world", `"\xef\xbf\xbd hello \xef\xbf\xbd world"`}, // utf8.RuneError
105+
// overflow
106+
{"9223372036854775810ns", `"9223372036854775810ns"`},
107+
{"9223372036854775808ns", `"9223372036854775808ns"`},
108+
{"-9223372036854775809ns", `"-9223372036854775809ns"`},
109+
{"9223372036854776us", `"9223372036854776us"`},
110+
{"3000000h", `"3000000h"`},
111+
{"9223372036854775.808us", `"9223372036854775.808us"`},
112+
{"9223372036854ms775us808ns", `"9223372036854ms775us808ns"`},
113+
}
114+
115+
func TestParseDurationErrors(t *testing.T) {
116+
for _, tc := range parseDurationErrorTests {
117+
_, err := bigtable.ParseDuration(tc.in)
118+
if err == nil {
119+
t.Errorf("bigtable.ParseDuration(%q) = _, nil, want _, non-nil", tc.in)
120+
} else if !strings.Contains(err.Error(), tc.expect) {
121+
t.Errorf("bigtable.ParseDuration(%q) = _, %q, error does not contain %q", tc.in, err, tc.expect)
122+
}
123+
}
124+
}
125+
126+
func TestParseDurationRoundTrip(t *testing.T) {
127+
// https://golang.org/issue/48629
128+
max0 := time.Duration(math.MaxInt64)
129+
max1, err := bigtable.ParseDuration(max0.String())
130+
if err != nil || max0 != max1 {
131+
t.Errorf("round-trip failed: %d => %q => %d, %v", max0, max0.String(), max1, err)
132+
}
133+
134+
min0 := time.Duration(math.MinInt64)
135+
min1, err := bigtable.ParseDuration(min0.String())
136+
if err != nil || min0 != min1 {
137+
t.Errorf("round-trip failed: %d => %q => %d, %v", min0, min0.String(), min1, err)
138+
}
139+
140+
for i := 0; i < 100; i++ {
141+
// Resolutions finer than milliseconds will result in
142+
// imprecise round-trips.
143+
d0 := time.Duration(rand.Int31()) * time.Millisecond
144+
s := d0.String()
145+
d1, err := bigtable.ParseDuration(s)
146+
if err != nil || d0 != d1 {
147+
t.Errorf("round-trip failed: %d => %q => %d, %v", d0, s, d1, err)
148+
}
149+
}
150+
}
151+
152+
func BenchmarkParseDuration(b *testing.B) {
153+
for i := 0; i < b.N; i++ {
154+
bigtable.ParseDuration("9007199254.740993ms")
155+
bigtable.ParseDuration("9007199254740993ns")
156+
}
157+
}

0 commit comments

Comments
 (0)