Put back ByteSizeValue.toString

Author: prdoyle

server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java

@@ -10,17 +10,16 @@
 package org.elasticsearch.common.unit;
 
 import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.logging.DeprecationLogger;
-import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.xcontent.ToXContentFragment;
 import org.elasticsearch.xcontent.XContentBuilder;
 
 import java.io.IOException;
 import java.math.BigDecimal;
-import java.text.DecimalFormat;
 import java.util.Locale;
 import java.util.Objects;
 
@@ -233,33 +232,26 @@ public String getStringRep() {
 
     @Override
     public String toString() {
-        return format2Decimals(sizeInBytes, preferredUnit.toBytes(1)) + preferredUnit.getSuffix();
-    }
-
-    /**
-     * Strictly speaking, this is only <em>guaranteed</em> to return the exact right string
-     * when {@code numerator} is the rounded value of some multiple of 1% of {@code denominator}.
-     * Other arbitrary values could yield surprising results; for example, you might see
-     * {@code 123.0} or even {@code 1221.00} in place of {@code 123}.
-     */
-    static String format2Decimals(long numerator, long denominator) {
-        // If numerator >> denominator, we risk exceeding the precision of a double, so we enlist
-        // the help of MAX_TWO_DECIMALS only for the fractional portion, and we handle the whole-number
-        // portion ourselves.
-        long wholeNumberPortion = numerator / denominator;
-        long remainder = numerator % denominator;
-        if (remainder == 0) {
-            return Long.toString(wholeNumberPortion);
-        } else {
-            return wholeNumberPortion + MAX_TWO_DECIMALS.format(remainder / (double) denominator);
+        long bytes = getBytes();
+        double value = bytes;
+        String suffix = ByteSizeUnit.BYTES.getSuffix();
+        if (bytes >= ByteSizeUnit.C5) {
+            value = getPbFrac();
+            suffix = ByteSizeUnit.PB.getSuffix();
+        } else if (bytes >= ByteSizeUnit.C4) {
+            value = getTbFrac();
+            suffix = ByteSizeUnit.TB.getSuffix();
+        } else if (bytes >= ByteSizeUnit.C3) {
+            value = getGbFrac();
+            suffix = ByteSizeUnit.GB.getSuffix();
+        } else if (bytes >= ByteSizeUnit.C2) {
+            value = getMbFrac();
+            suffix = ByteSizeUnit.MB.getSuffix();
+        } else if (bytes >= ByteSizeUnit.C1) {
+            value = getKbFrac();
+            suffix = ByteSizeUnit.KB.getSuffix();
         }
-    }
-
-    private static final DecimalFormat MAX_TWO_DECIMALS = maxTwoDecimalsFormat();
-
-    @SuppressForbidden(reason = "We truly just want two decimals; Locale settings don't matter here")
-    private static DecimalFormat maxTwoDecimalsFormat() {
-        return new DecimalFormat(".##");
+        return Strings.format1Decimals(value, suffix);
     }
 
     public static ByteSizeValue parseBytesSizeValue(String sValue, String settingName) throws ElasticsearchParseException {

server/src/test/java/org/elasticsearch/common/unit/ByteSizeValueTests.java

@@ -25,7 +25,6 @@
 import static org.elasticsearch.common.unit.ByteSizeUnit.MB;
 import static org.elasticsearch.common.unit.ByteSizeUnit.PB;
 import static org.elasticsearch.common.unit.ByteSizeUnit.TB;
-import static org.elasticsearch.common.unit.ByteSizeValue.format2Decimals;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
@@ -518,43 +517,6 @@ public void testParseFractionsExhaustively() {
         }
     }
 
-    public void testFormat2DecimalsExhaustively() {
-        for (var unit : ByteSizeUnit.values()) {
-            if (unit == BYTES) {
-                // Can't specify fractions of a byte
-                continue;
-            }
-            var granularity = unit.toBytes(1);
-            long wholeUnits = Long.MAX_VALUE / granularity - 1; // Try to provoke a failure by exceeding double precision
-            assertEquals(
-                "Integers should not have decimals",
-                Long.toString(wholeUnits),
-                format2Decimals(wholeUnits * granularity, granularity)
-            );
-            for (var percent = 1; percent < 100; percent++) {
-                long numerator = wholeUnits * granularity + (long) (percent * 0.01 * granularity);
-                String expected = wholeUnits + percentToDecimal(percent);
-                assertEquals(
-                    "format2Decimals on " + percent + "% of one " + unit.getSuffix(),
-                    expected,
-                    format2Decimals(numerator, granularity)
-                );
-            }
-        }
-    }
-
-    public void testFormat2DecimalsForHugeValues() {
-        // The largest magnitude we can get with fractions is if we specify a value very close to Long.MAX_VALUE bytes using KB.
-        long granularity = KB.toBytes(1);
-        long wholeUnits = Long.MAX_VALUE / granularity;
-        for (var percent = 1; percent < 100; percent++) {
-            long fractionalPart = (long) (0.01 * percent * granularity);
-            long numerator = wholeUnits * granularity + fractionalPart;
-            String expected = wholeUnits + percentToDecimal(percent);
-            assertEquals("String should match for " + percent + "%", expected, format2Decimals(numerator, granularity));
-        }
-    }
-
     private static String percentToDecimal(int percent) {
         if (percent <= 9) {
             return ".0" + percent;