jihoonson commented on a change in pull request #10203: URL: https://github.com/apache/druid/pull/10203#discussion_r459852843
########## File path: core/src/main/java/org/apache/druid/java/util/common/Bytes.java ########## @@ -0,0 +1,198 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.java.util.common; + +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + +import java.io.Serializable; +import java.util.Locale; + +@JsonSerialize(using = BytesSerializer.class) +public class Bytes implements Serializable +{ + public static final Bytes ZERO = new Bytes(0L); + + private long value; + + public Bytes(String value) + { + this.value = Bytes.parse(value); + } + + public Bytes(long value) + { + this.value = value; + } + + public long getValue() + { + return value; + } + + @Override + public boolean equals(Object thatObj) + { + if (thatObj == null) { + return false; + } + if (thatObj instanceof Bytes) { + return value == ((Bytes) thatObj).value; + } else { + return false; + } + } + + @Override + public int hashCode() + { + return Long.hashCode(value); + } + + @Override + public String toString() + { + return String.valueOf(value); + } + + public static Bytes valueOf(int bytes) + { + return new Bytes(bytes); + } + + public static Bytes valueOf(long bytes) + { + return new Bytes(bytes); + } + + public static long parse(String number) + { + if (number == null) { + throw new IAE("number is null"); + } + + number = number.trim().toLowerCase(Locale.getDefault()); Review comment: Please use `StringUtils.toLowerCase()` instead. ########## File path: processing/src/main/java/org/apache/druid/query/DruidProcessingConfig.java ########## @@ -40,9 +41,14 @@ private AtomicReference<Integer> computedBufferSizeBytes = new AtomicReference<>(); @Config({"druid.computation.buffer.size", "${base_path}.buffer.sizeBytes"}) + public String bufferSizeConfigured() + { + return "-1"; + } + public int intermediateComputeSizeBytesConfigured() { - return DEFAULT_PROCESSING_BUFFER_SIZE_BYTES; + return (int) Bytes.parse(bufferSizeConfigured(), DEFAULT_PROCESSING_BUFFER_SIZE_BYTES); Review comment: Hmm, this method can be called several times during processing a query. Why not returning `Bytes` in `bufferSizeConfigured()` instead of `String`? ########## File path: core/src/main/java/org/apache/druid/java/util/common/Bytes.java ########## @@ -0,0 +1,198 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.java.util.common; + +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + +import java.io.Serializable; +import java.util.Locale; + +@JsonSerialize(using = BytesSerializer.class) +public class Bytes implements Serializable Review comment: nit: `Bytes` seems too broad. For example, `Bytes.getValue()` doesn't seem intuitive enough to me what it would return. Maybe `HumanReadableBytes` could be a more readable name. ########## File path: core/src/main/java/org/apache/druid/java/util/common/Bytes.java ########## @@ -0,0 +1,198 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.java.util.common; + +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + +import java.io.Serializable; +import java.util.Locale; + +@JsonSerialize(using = BytesSerializer.class) +public class Bytes implements Serializable +{ + public static final Bytes ZERO = new Bytes(0L); + + private long value; + + public Bytes(String value) + { + this.value = Bytes.parse(value); + } + + public Bytes(long value) + { + this.value = value; + } + + public long getValue() + { + return value; + } + + @Override + public boolean equals(Object thatObj) + { + if (thatObj == null) { + return false; + } + if (thatObj instanceof Bytes) { + return value == ((Bytes) thatObj).value; + } else { + return false; + } + } + + @Override + public int hashCode() + { + return Long.hashCode(value); + } + + @Override + public String toString() + { + return String.valueOf(value); + } + + public static Bytes valueOf(int bytes) + { + return new Bytes(bytes); + } + + public static Bytes valueOf(long bytes) + { + return new Bytes(bytes); + } + + public static long parse(String number) + { + if (number == null) { + throw new IAE("number is null"); + } + + number = number.trim().toLowerCase(Locale.getDefault()); + int len = number.length(); + if (len == 0) { + throw new IAE("number is empty"); + } + + return parseInner(number); + } + + /** + * parse the case-insensitive string number, which is either: + * <p> + * a number string + * <p> + * or + * <p> + * a number string with a suffix which indicates the unit the of number + * the unit must be one of following + * k - kilobyte = 1000 + * m - megabyte = 1,000,000 + * g - gigabyte = 1,000,000,000 + * t - terabyte = 1,000,000,000,000 + * p - petabyte = 1,000,000,000,000,000 + * KiB - kilo binary byte = 1024 + * MiB - mega binary byte = 1024*1204 + * GiB - giga binary byte = 1024*1024*1024 + * TiB - tera binary byte = 1024*1024*1024*1024 + * PiB - peta binary byte = 1024*1024*1024*1024*1024 + * <p> + * + * @param nullValue to be returned when given number is null or empty + * @return nullValue if input is null or empty + * value of number + * @throws IAE if the input is invalid + */ + public static long parse(String number, long nullValue) + { + if (number == null) { + return nullValue; + } + + number = number.trim().toLowerCase(Locale.getDefault()); + if (number.length() == 0) { + return nullValue; + } + return parseInner(number); + } + + private static long parseInner(String number) + { + int index = number.length() - 1; + boolean isBinary = false; + char unit = number.charAt(index--); + if (unit == 'b') { + if (index < 2) { + throw new IAE("invalid format of number[%s]", number); + } + if (number.charAt(index--) != 'i') { + throw new IAE("invalid format of number[%s]", number); + } + + unit = number.charAt(index--); + isBinary = true; + } + long base = 1; + switch (unit) { + case 'k': + base = isBinary ? 1024 : 1_000; + break; + + case 'm': + base = isBinary ? 1024 * 1024 : 1_000_000; + break; + + case 'g': + base = isBinary ? 1024 * 1024 * 1024 : 1_000_000_000; + break; + + case 't': + base = isBinary ? 1024 * 1024 * 1024 * 1024L : 1_000_000_000_000L; + break; + + case 'p': + base = isBinary ? 1024L * 1024 * 1024 * 1024 * 1024 : 1_000_000_000_000_000L; + break; + + default: + if (!Character.isDigit(unit)) { + throw new IAE("invalid character in number[%s]", number); + } + break; + } + + try { + if (base > 1 && index >= 0) { + long value = Long.parseLong(number.substring(0, index + 1)) * base; + if (value < base) { + throw new IAE("number [%s] overflow", number); + } + return value; + } else { + return Long.parseLong(number); Review comment: Why not checking overflow here too? ########## File path: core/src/main/java/org/apache/druid/java/util/common/Bytes.java ########## @@ -0,0 +1,198 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.java.util.common; + +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + +import java.io.Serializable; +import java.util.Locale; + +@JsonSerialize(using = BytesSerializer.class) +public class Bytes implements Serializable +{ + public static final Bytes ZERO = new Bytes(0L); + + private long value; + + public Bytes(String value) + { + this.value = Bytes.parse(value); + } + + public Bytes(long value) + { + this.value = value; + } + + public long getValue() + { + return value; + } + + @Override + public boolean equals(Object thatObj) + { + if (thatObj == null) { + return false; + } + if (thatObj instanceof Bytes) { + return value == ((Bytes) thatObj).value; + } else { + return false; + } + } + + @Override + public int hashCode() + { + return Long.hashCode(value); + } + + @Override + public String toString() + { + return String.valueOf(value); + } + + public static Bytes valueOf(int bytes) + { + return new Bytes(bytes); + } + + public static Bytes valueOf(long bytes) + { + return new Bytes(bytes); + } + + public static long parse(String number) + { + if (number == null) { + throw new IAE("number is null"); + } + + number = number.trim().toLowerCase(Locale.getDefault()); + int len = number.length(); + if (len == 0) { + throw new IAE("number is empty"); + } + + return parseInner(number); + } + + /** + * parse the case-insensitive string number, which is either: + * <p> + * a number string + * <p> + * or + * <p> + * a number string with a suffix which indicates the unit the of number + * the unit must be one of following + * k - kilobyte = 1000 + * m - megabyte = 1,000,000 + * g - gigabyte = 1,000,000,000 + * t - terabyte = 1,000,000,000,000 + * p - petabyte = 1,000,000,000,000,000 + * KiB - kilo binary byte = 1024 + * MiB - mega binary byte = 1024*1204 + * GiB - giga binary byte = 1024*1024*1024 + * TiB - tera binary byte = 1024*1024*1024*1024 + * PiB - peta binary byte = 1024*1024*1024*1024*1024 + * <p> + * + * @param nullValue to be returned when given number is null or empty + * @return nullValue if input is null or empty + * value of number + * @throws IAE if the input is invalid + */ + public static long parse(String number, long nullValue) + { + if (number == null) { + return nullValue; + } + + number = number.trim().toLowerCase(Locale.getDefault()); + if (number.length() == 0) { + return nullValue; + } + return parseInner(number); + } + + private static long parseInner(String number) + { + int index = number.length() - 1; + boolean isBinary = false; + char unit = number.charAt(index--); + if (unit == 'b') { + if (index < 2) { + throw new IAE("invalid format of number[%s]", number); + } + if (number.charAt(index--) != 'i') { + throw new IAE("invalid format of number[%s]", number); + } + + unit = number.charAt(index--); + isBinary = true; + } + long base = 1; + switch (unit) { + case 'k': + base = isBinary ? 1024 : 1_000; + break; + + case 'm': + base = isBinary ? 1024 * 1024 : 1_000_000; + break; + + case 'g': + base = isBinary ? 1024 * 1024 * 1024 : 1_000_000_000; + break; + + case 't': + base = isBinary ? 1024 * 1024 * 1024 * 1024L : 1_000_000_000_000L; + break; + + case 'p': + base = isBinary ? 1024L * 1024 * 1024 * 1024 * 1024 : 1_000_000_000_000_000L; + break; + + default: + if (!Character.isDigit(unit)) { + throw new IAE("invalid character in number[%s]", number); + } + break; + } + + try { + if (base > 1 && index >= 0) { + long value = Long.parseLong(number.substring(0, index + 1)) * base; + if (value < base) { Review comment: Out of curiosity, why does this compare against `base` instead of 0? ########## File path: processing/src/main/java/org/apache/druid/query/DruidProcessingConfig.java ########## @@ -40,9 +41,14 @@ private AtomicReference<Integer> computedBufferSizeBytes = new AtomicReference<>(); @Config({"druid.computation.buffer.size", "${base_path}.buffer.sizeBytes"}) + public String bufferSizeConfigured() + { + return "-1"; + } + public int intermediateComputeSizeBytesConfigured() { - return DEFAULT_PROCESSING_BUFFER_SIZE_BYTES; + return (int) Bytes.parse(bufferSizeConfigured(), DEFAULT_PROCESSING_BUFFER_SIZE_BYTES); Review comment: I guess we can call `Bytes.parse()` without default value since `bufferSizeConfigured()` has default? ########## File path: processing/src/main/java/org/apache/druid/query/DruidProcessingConfig.java ########## @@ -40,9 +41,14 @@ private AtomicReference<Integer> computedBufferSizeBytes = new AtomicReference<>(); @Config({"druid.computation.buffer.size", "${base_path}.buffer.sizeBytes"}) + public String bufferSizeConfigured() + { + return "-1"; Review comment: Please make it as a static variable. ########## File path: core/src/main/java/org/apache/druid/java/util/common/BytesRange.java ########## @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.java.util.common; + +import javax.validation.Constraint; +import javax.validation.ConstraintValidator; +import javax.validation.ConstraintValidatorContext; +import javax.validation.Payload; +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ + ElementType.FIELD, + ElementType.PARAMETER +}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Constraint(validatedBy = BytesRange.DateRangeValidator.class) +public @interface BytesRange Review comment: Please add simple javadoc about what this annotation is and how it is used. ########## File path: core/src/main/java/org/apache/druid/java/util/common/Bytes.java ########## @@ -0,0 +1,198 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.java.util.common; + +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + +import java.io.Serializable; +import java.util.Locale; + +@JsonSerialize(using = BytesSerializer.class) +public class Bytes implements Serializable +{ + public static final Bytes ZERO = new Bytes(0L); + + private long value; + + public Bytes(String value) + { + this.value = Bytes.parse(value); + } + + public Bytes(long value) + { + this.value = value; + } + + public long getValue() + { + return value; + } + + @Override + public boolean equals(Object thatObj) + { + if (thatObj == null) { + return false; + } + if (thatObj instanceof Bytes) { + return value == ((Bytes) thatObj).value; + } else { + return false; + } + } + + @Override + public int hashCode() + { + return Long.hashCode(value); + } + + @Override + public String toString() + { + return String.valueOf(value); + } + + public static Bytes valueOf(int bytes) + { + return new Bytes(bytes); + } + + public static Bytes valueOf(long bytes) + { + return new Bytes(bytes); + } + + public static long parse(String number) + { + if (number == null) { + throw new IAE("number is null"); + } + + number = number.trim().toLowerCase(Locale.getDefault()); + int len = number.length(); + if (len == 0) { + throw new IAE("number is empty"); + } + + return parseInner(number); + } + + /** + * parse the case-insensitive string number, which is either: + * <p> + * a number string + * <p> + * or + * <p> + * a number string with a suffix which indicates the unit the of number + * the unit must be one of following + * k - kilobyte = 1000 + * m - megabyte = 1,000,000 + * g - gigabyte = 1,000,000,000 + * t - terabyte = 1,000,000,000,000 + * p - petabyte = 1,000,000,000,000,000 + * KiB - kilo binary byte = 1024 + * MiB - mega binary byte = 1024*1204 + * GiB - giga binary byte = 1024*1024*1024 + * TiB - tera binary byte = 1024*1024*1024*1024 + * PiB - peta binary byte = 1024*1024*1024*1024*1024 + * <p> + * + * @param nullValue to be returned when given number is null or empty + * @return nullValue if input is null or empty + * value of number + * @throws IAE if the input is invalid + */ + public static long parse(String number, long nullValue) + { + if (number == null) { + return nullValue; + } + + number = number.trim().toLowerCase(Locale.getDefault()); Review comment: Please use `StringUtils.toLowerCase()` instead. ########## File path: indexing-service/src/main/java/org/apache/druid/indexing/overlord/config/RemoteTaskRunnerConfig.java ########## @@ -81,15 +83,14 @@ public Period getTaskCleanupTimeout() public int getMaxZnodeBytes() { - return maxZnodeBytes; + return (int) maxZnodeBytes.getValue(); Review comment: How about adding `asInt()` which has a check for integer overflow? ########## File path: core/src/test/java/org/apache/druid/java/util/common/BytesTest.java ########## @@ -0,0 +1,300 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.java.util.common; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import javax.validation.ConstraintViolation; +import javax.validation.Validation; +import javax.validation.Validator; +import javax.validation.groups.Default; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +public class BytesTest +{ + @Test + public void testNumberString() + { + Assert.assertEquals(1, Bytes.parse("1", 0L)); + Assert.assertEquals(10000000, Bytes.parse("10000000", 0)); + } + + @Test + public void testWithWhiteSpace() + { + Assert.assertEquals(12345, Bytes.parse(" 12345 ", 0)); + Assert.assertEquals(12345, Bytes.parse("\t12345\t", 0)); + } + + @Test + public void testK() + { + Assert.assertEquals(1000, Bytes.parse("1k", 0)); + Assert.assertEquals(1000, Bytes.parse("1K", 0)); + } + + @Test + public void testM() + { + Assert.assertEquals(1000_000, Bytes.parse("1m", 0)); + Assert.assertEquals(1000_000, Bytes.parse("1M", 0)); + } + + @Test + public void testG() + { + Assert.assertEquals(1000_000_000, Bytes.parse("1g", 0)); + Assert.assertEquals(1000_000_000, Bytes.parse("1G", 0)); + } + + @Test + public void testT() + { + Assert.assertEquals(1000_000_000_000L, Bytes.parse("1t", 0)); + Assert.assertEquals(1000_000_000_000L, Bytes.parse("1T", 0)); + } + + @Test + public void testKiB() + { + Assert.assertEquals(1024, Bytes.parse("1kib", 0)); + Assert.assertEquals(9 * 1024, Bytes.parse("9KiB", 0)); + Assert.assertEquals(9 * 1024, Bytes.parse("9Kib", 0)); + } + + @Test + public void testMb() + { + Assert.assertEquals(1024 * 1024, Bytes.parse("1mib", 0)); + Assert.assertEquals(9 * 1024 * 1024, Bytes.parse("9MiB", 0)); + Assert.assertEquals(9 * 1024 * 1024, Bytes.parse("9Mib", 0)); + } + + @Test + public void testGiB() + { + Assert.assertEquals(1024 * 1024 * 1024, Bytes.parse("1gib", 0)); + Assert.assertEquals(1024 * 1024 * 1024, Bytes.parse("1GiB", 0)); + Assert.assertEquals(9L * 1024 * 1024 * 1024, Bytes.parse("9Gib", 0)); + } + + @Test + public void testTiB() + { + Assert.assertEquals(1024 * 1024 * 1024 * 1024L, Bytes.parse("1tib", 0)); + Assert.assertEquals(9L * 1024 * 1024 * 1024 * 1024, Bytes.parse("9TiB", 0)); + Assert.assertEquals(9L * 1024 * 1024 * 1024 * 1024, Bytes.parse("9Tib", 0)); + } + + @Test + public void testPiB() + { + Assert.assertEquals(1024L * 1024 * 1024 * 1024 * 1024, Bytes.parse("1pib", 0)); + Assert.assertEquals(9L * 1024 * 1024 * 1024 * 1024 * 1024, Bytes.parse("9PiB", 0)); + Assert.assertEquals(9L * 1024 * 1024 * 1024 * 1024 * 1024, Bytes.parse("9Pib", 0)); + } + + @Test + public void testDefault() + { + Assert.assertEquals(-123, Bytes.parse(" ", -123)); + Assert.assertEquals(-456, Bytes.parse(null, -456)); + Assert.assertEquals(-789, Bytes.parse("\t", -789)); + } + + @Test + public void testException() + { + try { + Bytes.parse("b", 0); + Assert.assertFalse("IAE should be thrown", true); Review comment: nit: `ExpectedException` is preferred since you can verify the error message as well as the exception type. This is useful since even when the same type of exception is thrown, exceptions from different area of code can have different messages. ########## File path: core/src/test/java/org/apache/druid/java/util/common/BytesTest.java ########## @@ -0,0 +1,300 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.java.util.common; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +import javax.validation.ConstraintViolation; +import javax.validation.Validation; +import javax.validation.Validator; +import javax.validation.groups.Default; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +public class BytesTest +{ + @Test + public void testNumberString() + { + Assert.assertEquals(1, Bytes.parse("1", 0L)); + Assert.assertEquals(10000000, Bytes.parse("10000000", 0)); + } + + @Test + public void testWithWhiteSpace() + { + Assert.assertEquals(12345, Bytes.parse(" 12345 ", 0)); + Assert.assertEquals(12345, Bytes.parse("\t12345\t", 0)); + } + + @Test + public void testK() + { + Assert.assertEquals(1000, Bytes.parse("1k", 0)); + Assert.assertEquals(1000, Bytes.parse("1K", 0)); + } + + @Test + public void testM() + { + Assert.assertEquals(1000_000, Bytes.parse("1m", 0)); + Assert.assertEquals(1000_000, Bytes.parse("1M", 0)); + } + + @Test + public void testG() + { + Assert.assertEquals(1000_000_000, Bytes.parse("1g", 0)); + Assert.assertEquals(1000_000_000, Bytes.parse("1G", 0)); + } + + @Test + public void testT() + { + Assert.assertEquals(1000_000_000_000L, Bytes.parse("1t", 0)); + Assert.assertEquals(1000_000_000_000L, Bytes.parse("1T", 0)); + } + + @Test + public void testKiB() + { + Assert.assertEquals(1024, Bytes.parse("1kib", 0)); + Assert.assertEquals(9 * 1024, Bytes.parse("9KiB", 0)); + Assert.assertEquals(9 * 1024, Bytes.parse("9Kib", 0)); + } + + @Test + public void testMb() + { + Assert.assertEquals(1024 * 1024, Bytes.parse("1mib", 0)); + Assert.assertEquals(9 * 1024 * 1024, Bytes.parse("9MiB", 0)); + Assert.assertEquals(9 * 1024 * 1024, Bytes.parse("9Mib", 0)); + } + + @Test + public void testGiB() + { + Assert.assertEquals(1024 * 1024 * 1024, Bytes.parse("1gib", 0)); + Assert.assertEquals(1024 * 1024 * 1024, Bytes.parse("1GiB", 0)); + Assert.assertEquals(9L * 1024 * 1024 * 1024, Bytes.parse("9Gib", 0)); + } + + @Test + public void testTiB() + { + Assert.assertEquals(1024 * 1024 * 1024 * 1024L, Bytes.parse("1tib", 0)); + Assert.assertEquals(9L * 1024 * 1024 * 1024 * 1024, Bytes.parse("9TiB", 0)); + Assert.assertEquals(9L * 1024 * 1024 * 1024 * 1024, Bytes.parse("9Tib", 0)); + } + + @Test + public void testPiB() + { + Assert.assertEquals(1024L * 1024 * 1024 * 1024 * 1024, Bytes.parse("1pib", 0)); + Assert.assertEquals(9L * 1024 * 1024 * 1024 * 1024 * 1024, Bytes.parse("9PiB", 0)); + Assert.assertEquals(9L * 1024 * 1024 * 1024 * 1024 * 1024, Bytes.parse("9Pib", 0)); + } + + @Test + public void testDefault() + { + Assert.assertEquals(-123, Bytes.parse(" ", -123)); + Assert.assertEquals(-456, Bytes.parse(null, -456)); + Assert.assertEquals(-789, Bytes.parse("\t", -789)); + } + + @Test + public void testException() + { + try { + Bytes.parse("b", 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + Assert.assertTrue(true); + } + + try { + Bytes.parse("1 b", 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + Assert.assertTrue(true); + } + + try { + Bytes.parse("tb", 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + Assert.assertTrue(true); + } + + try { + Bytes.parse("1 mb", 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + Assert.assertTrue(true); + } + + try { + Bytes.parse("gb", 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + Assert.assertTrue(true); + } + + try { + Bytes.parse("tb", 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + Assert.assertTrue(true); + } + } + + @Test + public void testOverflow() + { + try { + Bytes.parse("123456789123456789123", 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + } + + try { + String max = (Long.MAX_VALUE / 1000 + 1) + "k"; + Bytes.parse(max, 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + } + + try { + String max = (Long.MAX_VALUE / 1000_000 + 1) + "m"; + Bytes.parse(max, 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + } + + try { + String max = (Long.MAX_VALUE / 1000_000_000L + 1) + "g"; + Bytes.parse(max, 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + } + + try { + String max = (Long.MAX_VALUE / 1000_000_000_000L + 1) + "t"; + Bytes.parse(max, 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + } + + try { + String max = (Long.MAX_VALUE / 1024 + 1) + "kb"; + Bytes.parse(max, 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + } + + try { + String max = (Long.MAX_VALUE / (1024 * 1024) + 1) + "mb"; + Bytes.parse(max, 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + } + + try { + String max = (Long.MAX_VALUE / (1024L * 1024 * 1024) + 1) + "gb"; + Bytes.parse(max, 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + } + + try { + String max = (Long.MAX_VALUE / (1024L * 1024 * 1024 * 1024) + 1) + "tb"; + Bytes.parse(max, 0); + Assert.assertFalse("IAE should be thrown", true); + } + catch (IAE e) { + } + } + + @Test + public void testJSON() throws JsonProcessingException + { + ObjectMapper mapper = new ObjectMapper(); + Bytes bytes = new Bytes("5m"); + String serialized = mapper.writeValueAsString(bytes); + Bytes deserialized = mapper.readValue(serialized, Bytes.class); + Assert.assertEquals(bytes, deserialized); + } + + public static class BytesRangeTest Review comment: We usually name a test class to have the `Test` suffix if it has unit tests. How about `TestBytesRange`? ########## File path: core/src/main/java/org/apache/druid/java/util/common/Bytes.java ########## @@ -0,0 +1,198 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.java.util.common; + +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + +import java.io.Serializable; +import java.util.Locale; + +@JsonSerialize(using = BytesSerializer.class) +public class Bytes implements Serializable +{ + public static final Bytes ZERO = new Bytes(0L); + + private long value; + + public Bytes(String value) + { + this.value = Bytes.parse(value); + } + + public Bytes(long value) + { + this.value = value; + } + + public long getValue() + { + return value; + } + + @Override + public boolean equals(Object thatObj) + { + if (thatObj == null) { + return false; + } + if (thatObj instanceof Bytes) { + return value == ((Bytes) thatObj).value; + } else { + return false; + } + } + + @Override + public int hashCode() + { + return Long.hashCode(value); + } + + @Override + public String toString() + { + return String.valueOf(value); + } + + public static Bytes valueOf(int bytes) + { + return new Bytes(bytes); + } + + public static Bytes valueOf(long bytes) + { + return new Bytes(bytes); + } + + public static long parse(String number) + { + if (number == null) { + throw new IAE("number is null"); Review comment: All error messages should start with a capital letter. Please fix other places too. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
