Saturation + test, also test negative numbers

pkg/api/resource/quantity.go

@@ -53,8 +53,8 @@ const (
 //   (Note that 1024 = 1Ki but 1000 = 1k; I didn't choose the capitalization.)
 //
 // No matter which of the three exponent forms is used, no quantity may represent
-// a number less than .001m or greater than 2^63-1 in magnitude. Numbers that exceed
+// a number less than 1m or greater than 2^63-1 in magnitude. Numbers that exceed
-// a bound will be capped at that bound. (E.g.: 0.0001m will be treated as 0.001m.)
+// a bound will be capped at that bound. (E.g.: 0.1m will be treated as 1m.)
 // This may be extended in the future if we require larger or smaller quantities.
 //
 // Numbers with binary suffixes may not have any fractional part.
@@ -104,10 +104,22 @@ var (
 	// splitRE is used to get the various parts of a number.
 	splitRE = regexp.MustCompile(splitREString)
 
+	// Errors that could happen while parsing a string.
 	ErrFormatWrong      = errors.New("quantities must match the regular expression '" + splitREString + "'")
 	ErrNumeric          = errors.New("unable to parse numeric part of quantity")
 	ErrSuffix           = errors.New("unable to parse quantity's suffix")
 	ErrFractionalBinary = errors.New("numbers with binary-style SI suffixes can't have fractional parts")
+
+	// Commonly needed big.Ints-- treat as read only!
+	ten      = big.NewInt(10)
+	zero     = big.NewInt(0)
+	thousand = big.NewInt(1000)
+	ten24    = big.NewInt(1024)
+	decZero  = inf.NewDec(0, 0)
+
+	// Smallest and largest (in magnitude) numbers allowed.
+	minAllowed = inf.NewDec(1, 3)         // == 1/1000
+	maxAllowed = inf.NewDec((1<<63)-1, 0) // == max int64
 )
 
 // ParseQuantity turns str into a Quantity, or returns an error.
@@ -140,27 +152,36 @@ func ParseQuantity(str string) (*Quantity, error) {
 			return nil, ErrFractionalBinary
 		}
 		// exponent will always be a multiple of 10.
-		dec1024 := inf.NewDec(1024, 0)
 		for exponent > 0 {
-			amount.Mul(amount, dec1024)
+			amount.UnscaledBig().Mul(amount.UnscaledBig(), ten24)
 			exponent -= 10
 		}
 	}
 
+	// Cap at min/max bounds.
+	sign := amount.Sign()
+	if sign == -1 {
+		amount.Neg(amount)
+	}
+	// This rounds non-zero values up to the minimum representable
+	// value, under the theory that if you want some resources, you
+	// should get some resources, even if you asked for way too small
+	// of an amount.
+	// Arguably, this should be inf.RoundHalfUp (normal rounding), but
+	// that would have the side effect of rounding values < .5m to zero.
+	amount.Round(amount, 3, inf.RoundUp)
+
+	// The max is just a simple cap.
+	if amount.Cmp(maxAllowed) > 0 {
+		amount.Set(maxAllowed)
+	}
+	if sign == -1 {
+		amount.Neg(amount)
+	}
+
 	return &Quantity{amount, format}, nil
 }
 
-var (
-	// Commonly needed big.Ints-- treat as read only!
-	ten      = big.NewInt(10)
-	zero     = big.NewInt(0)
-	thousand = big.NewInt(1000)
-	ten24    = big.NewInt(1024)
-
-	minAllowed = inf.NewDec(1, 6)
-	maxAllowed = inf.NewDec(999, -18)
-)
-
 // removeFactors divides in a loop; the return values have the property that
 // d == result * factor ^ times
 // d may be modified in place.

pkg/api/resource/quantity_test.go

@@ -97,21 +97,20 @@ func TestQuantityParse(t *testing.T) {
 
 		// Things that look like floating point
 		{"0.001", Quantity{dec(1, -3), DecimalSI}},
-		{"0.0005", Quantity{dec(5, -4), DecimalSI}},
+		{"0.0005k", Quantity{dec(5, -1), DecimalSI}},
 		{"0.005", Quantity{dec(5, -3), DecimalSI}},
 		{"0.05", Quantity{dec(5, -2), DecimalSI}},
 		{"0.5", Quantity{dec(5, -1), DecimalSI}},
-		{"0.00050", Quantity{dec(5, -4), DecimalSI}},
+		{"0.00050k", Quantity{dec(5, -1), DecimalSI}},
 		{"0.00500", Quantity{dec(5, -3), DecimalSI}},
 		{"0.05000", Quantity{dec(5, -2), DecimalSI}},
 		{"0.50000", Quantity{dec(5, -1), DecimalSI}},
-		{"0.5e-3", Quantity{dec(5, -4), DecimalExponent}},
+		{"0.5e0", Quantity{dec(5, -1), DecimalExponent}},
-		{"0.5e-2", Quantity{dec(5, -3), DecimalExponent}},
 		{"0.5e-1", Quantity{dec(5, -2), DecimalExponent}},
+		{"0.5e-2", Quantity{dec(5, -3), DecimalExponent}},
 		{"0.5e0", Quantity{dec(5, -1), DecimalExponent}},
 		{"10.035M", Quantity{dec(10035, 3), DecimalSI}},
 
-		{"1.1E-3", Quantity{dec(11, -4), DecimalExponent}},
 		{"1.2e3", Quantity{dec(12, 2), DecimalExponent}},
 		{"1.3E6", Quantity{dec(13, 5), DecimalExponent}},
 		{"1.40e9", Quantity{dec(14, 8), DecimalExponent}},
@@ -119,7 +118,6 @@ func TestQuantityParse(t *testing.T) {
 		{"1.6e15", Quantity{dec(16, 14), DecimalExponent}},
 		{"1.7E18", Quantity{dec(17, 17), DecimalExponent}},
 
-		{"3.001m", Quantity{dec(3001, -6), DecimalSI}},
 		{"9.01", Quantity{dec(901, -2), DecimalSI}},
 		{"8.1k", Quantity{dec(81, 2), DecimalSI}},
 		{"7.123456M", Quantity{dec(7123456, 0), DecimalSI}},
@@ -130,11 +128,19 @@ func TestQuantityParse(t *testing.T) {
 		{"2.5P", Quantity{dec(25, 14), DecimalSI}},
 		{"1.01E", Quantity{dec(101, 16), DecimalSI}},
 
-		// Things that saturate
+		// Things that saturate/round
-		//{"0.1m",
+		{"3.001m", Quantity{dec(4, -3), DecimalSI}},
-		//{"9Ei",
+		{"1.1E-3", Quantity{dec(2, -3), DecimalExponent}},
-		//{"9223372036854775807Ki",
+		{"0.0001", Quantity{dec(1, -3), DecimalSI}},
-		//{"12E",
+		{"0.0005", Quantity{dec(1, -3), DecimalSI}},
+		{"0.00050", Quantity{dec(1, -3), DecimalSI}},
+		{"0.5e-3", Quantity{dec(1, -3), DecimalExponent}},
+		{"0.9m", Quantity{dec(1, -3), DecimalSI}},
+		{"0.12345", Quantity{dec(124, -3), DecimalSI}},
+		{"0.12354", Quantity{dec(124, -3), DecimalSI}},
+		{"9Ei", Quantity{maxAllowed, BinarySI}},
+		{"9223372036854775807Ki", Quantity{maxAllowed, BinarySI}},
+		{"12E", Quantity{maxAllowed, DecimalSI}},
 	}
 
 	for _, item := range table {
@@ -151,6 +157,38 @@ func TestQuantityParse(t *testing.T) {
 		}
 	}
 
+	// Try the negative version of everything
+	desired := &inf.Dec{}
+	for _, item := range table {
+		got, err := ParseQuantity("-" + item.input)
+		if err != nil {
+			t.Errorf("-%v: unexpected error: %v", item.input, err)
+			continue
+		}
+		desired.Neg(item.expect.Amount)
+		if e, a := desired, got.Amount; e.Cmp(a) != 0 {
+			t.Errorf("%v: expected %v, got %v", item.input, e, a)
+		}
+		if e, a := item.expect.Format, got.Format; e != a {
+			t.Errorf("%v: expected %#v, got %#v", item.input, e, a)
+		}
+	}
+
+	// Try everything with an explicit +
+	for _, item := range table {
+		got, err := ParseQuantity("+" + item.input)
+		if err != nil {
+			t.Errorf("-%v: unexpected error: %v", item.input, err)
+			continue
+		}
+		if e, a := item.expect.Amount, got.Amount; e.Cmp(a) != 0 {
+			t.Errorf("%v: expected %v, got %v", item.input, e, a)
+		}
+		if e, a := item.expect.Format, got.Format; e != a {
+			t.Errorf("%v: expected %#v, got %#v", item.input, e, a)
+		}
+	}
+
 	invalid := []string{
 		"1.1.M",
 		"1+1.0M",
@@ -208,9 +246,6 @@ func TestQuantityString(t *testing.T) {
 		{Quantity{dec(3, 3), DecimalExponent}, "3e3"},
 		{Quantity{dec(3, 3), DecimalSI}, "3k"},
 		{Quantity{dec(0, 0), DecimalExponent}, "0"},
-
-		{Quantity{dec(-1080, -3), DecimalSI}, "-1.080"},
-		{Quantity{dec(-80*1024, 0), BinarySI}, "-80Ki"},
 	}
 	for _, item := range table {
 		got := item.in.String()
@@ -218,4 +253,16 @@ func TestQuantityString(t *testing.T) {
 			t.Errorf("%#v: expected %v, got %v", item.in, e, a)
 		}
 	}
+	desired := &inf.Dec{} // Avoid modifying the values in the table.
+	for _, item := range table {
+		if item.in.Amount.Cmp(decZero) == 0 {
+			// Don't expect it to print "-0" ever
+			continue
+		}
+		q := item.in
+		q.Amount = desired.Neg(q.Amount)
+		if e, a := "-"+item.expect, q.String(); e != a {
+			t.Errorf("%#v: expected %v, got %v", item.in, e, a)
+		}
+	}
 }