From 401204ac097f4aaf55f572feb713c75c306976d0 Mon Sep 17 00:00:00 2001 From: Volodymyr Kravets Date: Fri, 26 Jul 2024 15:16:53 +0300 Subject: [PATCH] refactor: update Bytes and BytesSlice to 'mimic' System.arraycopy and Arrays.copyOfRange; add more tests --- .../java/co/rsk/core/types/bytes/Bytes.java | 5 +- .../co/rsk/core/types/bytes/BytesSlice.java | 84 +++++++++++++++-- .../core/types/bytes/HexPrintableBytes.java | 2 +- .../main/java/co/rsk/util/StringUtils.java | 2 + .../rsk/core/types/bytes/BytesSliceTest.java | 85 +++++++++++++++++- .../co/rsk/core/types/bytes/BytesTest.java | 89 ++++++++++++++++++- .../src/test/java/co/rsk/util/Functions.java | 30 +++++++ 7 files changed, 279 insertions(+), 18 deletions(-) create mode 100644 rskj-core/src/test/java/co/rsk/util/Functions.java diff --git a/rskj-core/src/main/java/co/rsk/core/types/bytes/Bytes.java b/rskj-core/src/main/java/co/rsk/core/types/bytes/Bytes.java index ce988888068..cb4c16d17d3 100644 --- a/rskj-core/src/main/java/co/rsk/core/types/bytes/Bytes.java +++ b/rskj-core/src/main/java/co/rsk/core/types/bytes/Bytes.java @@ -23,7 +23,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.util.Arrays; import java.util.Objects; /** @@ -140,8 +139,8 @@ public byte byteAt(int index) { } @Override - public byte[] copyArrayOfRange(int from, int to) { - return Arrays.copyOfRange(byteArray, from, to); + public void arraycopy(int srcPos, byte[] dest, int destPos, int length) { + System.arraycopy(byteArray, srcPos, dest, destPos, length); } @Override diff --git a/rskj-core/src/main/java/co/rsk/core/types/bytes/BytesSlice.java b/rskj-core/src/main/java/co/rsk/core/types/bytes/BytesSlice.java index 5210ff8eea0..ac3616fd8f0 100644 --- a/rskj-core/src/main/java/co/rsk/core/types/bytes/BytesSlice.java +++ b/rskj-core/src/main/java/co/rsk/core/types/bytes/BytesSlice.java @@ -18,11 +18,62 @@ package co.rsk.core.types.bytes; +import java.util.Arrays; + /** * A {@link BytesSlice} is a subsequence of bytes backed by another broader byte sequence. */ public interface BytesSlice extends HexPrintableBytes { + /** + * Copies an array from the {@link BytesSlice} source, beginning at the + * specified position, to the specified position of the destination array. + * A subsequence of array components are copied from this instance to the + * destination array referenced by {@code dest}. The number of components + * copied is equal to the {@code length} argument. The components at + * positions {@code srcPos} through {@code srcPos+length-1} in the source + * array are copied into positions {@code destPos} through + * {@code destPos+length-1}, respectively, of the destination + * array. + *

+ * If the underlying byte array and {@code dest} argument refer to the + * same array object, then the copying is performed as if the + * components at positions {@code srcPos} through + * {@code srcPos+length-1} were first copied to a temporary + * array with {@code length} components and then the contents of + * the temporary array were copied into positions + * {@code destPos} through {@code destPos+length-1} of the + * destination array. + *

+ * If {@code dest} is {@code null}, then a + * {@code NullPointerException} is thrown. + *

+ * Otherwise, if any of the following is true, an + * {@code IndexOutOfBoundsException} is + * thrown and the destination is not modified: + *

+ * + *

+ * Note: this method mimics behaviour of {@link System#arraycopy(Object, int, Object, int, int)} + * + * @param srcPos starting position in the source array. + * @param dest the destination array. + * @param destPos starting position in the destination data. + * @param length the number of array elements to be copied. + * @throws IndexOutOfBoundsException if copying would cause + * access of data outside array bounds. + * @throws NullPointerException if {@code dest} is {@code null}. + */ + void arraycopy(int srcPos, byte[] dest, int destPos, int length); + /** * Copies the specified range of the specified array into a new array. * The initial index of the range (from) must lie between zero @@ -37,17 +88,30 @@ public interface BytesSlice extends HexPrintableBytes { * greater than or equal to original.length - from. The length * of the returned array will be to - from. * + *

+ * Note: this method mimics behaviour of {@link Arrays#copyOfRange(Object[], int, int)} + * * @param from the initial index of the range to be copied, inclusive * @param to the final index of the range to be copied, exclusive. * (This index may lie outside the array.) * @return a new array containing the specified range from the original array, * truncated or padded with zeros to obtain the required length - * @throws ArrayIndexOutOfBoundsException if {@code from < 0} + * @throws IndexOutOfBoundsException if {@code from < 0} * or {@code from > original.length} * @throws IllegalArgumentException if from > to - * @throws NullPointerException if original is null */ - byte[] copyArrayOfRange(int from, int to); + default byte[] copyArrayOfRange(int from, int to) { + if (from < 0 || from > length()) { + throw new IndexOutOfBoundsException("invalid 'from': " + from); + } + int newLength = to - from; + if (newLength < 0) { + throw new IllegalArgumentException(from + " > " + to); + } + byte[] copy = new byte[newLength]; + arraycopy(from, copy, 0, Math.min(length() - from, newLength)); + return copy; + } default byte[] copyArray() { return copyArrayOfRange(0, length()); @@ -104,11 +168,17 @@ public byte byteAt(int index) { } @Override - public byte[] copyArrayOfRange(int from, int to) { - if (from < 0 || from > to || to > length()) { - throw new IndexOutOfBoundsException("invalid 'from' and/or 'to': [" + from + ";" + to + ")"); + public void arraycopy(int srcPos, byte[] dest, int destPos, int length) { + if (length < 0) { + throw new IndexOutOfBoundsException("invalid 'length': " + length); + } + if (srcPos < 0 || srcPos + length > length()) { + throw new IndexOutOfBoundsException("invalid 'srcPos' and/or 'length': [" + srcPos + ";" + length + ")"); + } + if (destPos < 0 || destPos + length > dest.length) { + throw new IndexOutOfBoundsException("invalid 'destPos' and/or 'length': [" + destPos + ";" + length + ")"); } - return originBytes.copyArrayOfRange(this.from + from, this.from + to); + originBytes.arraycopy(this.from + srcPos, dest, destPos, length); } @Override diff --git a/rskj-core/src/main/java/co/rsk/core/types/bytes/HexPrintableBytes.java b/rskj-core/src/main/java/co/rsk/core/types/bytes/HexPrintableBytes.java index d700c0cc6a6..3f91f918eda 100644 --- a/rskj-core/src/main/java/co/rsk/core/types/bytes/HexPrintableBytes.java +++ b/rskj-core/src/main/java/co/rsk/core/types/bytes/HexPrintableBytes.java @@ -77,7 +77,7 @@ public String toFormattedString(@Nonnull HexPrintableBytes printableBytes, int o } if (length > 32) { - return printableBytes.toHexString(off, 15) + ".." + printableBytes.toHexString(off + length - 15, 15); + return printableBytes.toHexString(off, 16) + ".." + printableBytes.toHexString(off + length - 15, 15); } return printableBytes.toHexString(off, length); } diff --git a/rskj-core/src/main/java/co/rsk/util/StringUtils.java b/rskj-core/src/main/java/co/rsk/util/StringUtils.java index a7c58a322b3..02ef4106555 100644 --- a/rskj-core/src/main/java/co/rsk/util/StringUtils.java +++ b/rskj-core/src/main/java/co/rsk/util/StringUtils.java @@ -24,6 +24,8 @@ public class StringUtils { private static final int DEFAULT_MAX_LEN = 64; + private StringUtils() { /* hidden */ } + public static String trim(@Nullable String src) { return trim(src, DEFAULT_MAX_LEN); } diff --git a/rskj-core/src/test/java/co/rsk/core/types/bytes/BytesSliceTest.java b/rskj-core/src/test/java/co/rsk/core/types/bytes/BytesSliceTest.java index 93707953604..001aee51f35 100644 --- a/rskj-core/src/test/java/co/rsk/core/types/bytes/BytesSliceTest.java +++ b/rskj-core/src/test/java/co/rsk/core/types/bytes/BytesSliceTest.java @@ -19,17 +19,56 @@ package co.rsk.core.types.bytes; +import co.rsk.util.Functions; import org.junit.jupiter.api.Test; +import java.util.Arrays; + import static org.junit.jupiter.api.Assertions.*; class BytesSliceTest { + @Test + void testBytesLength() { + assertEquals(0, Bytes.of(new byte[]{}).slice(0, 0).length()); + assertEquals(0, Bytes.of(new byte[]{1}).slice(0, 0).length()); + assertEquals(1, Bytes.of(new byte[]{1}).slice(0, 1).length()); + assertEquals(0, Bytes.of(new byte[]{1,2,3}).slice(1, 1).length()); + assertEquals(1, Bytes.of(new byte[]{1,2,3}).slice(1, 2).length()); + assertEquals(2, Bytes.of(new byte[]{1,2,3}).slice(0, 2).length()); + assertEquals(3, Bytes.of(new byte[]{1,2,3}).slice(0, 3).length()); + } + + @Test + void testBytesAt() { + assertThrows(IndexOutOfBoundsException.class, () -> Bytes.of(new byte[]{}).slice(0, 0).byteAt(0)); + assertThrows(IndexOutOfBoundsException.class, () -> Bytes.of(new byte[]{1}).slice(0, 1).byteAt(1)); + assertThrows(IndexOutOfBoundsException.class, () -> Bytes.of(new byte[]{1}).slice(0, 1).byteAt(-1)); + assertThrows(IndexOutOfBoundsException.class, () -> Bytes.of(new byte[]{1,2,3}).slice(1, 2).byteAt(1)); + assertEquals(1, Bytes.of(new byte[]{1}).slice(0, 1).byteAt(0)); + assertEquals(2, Bytes.of(new byte[]{1,2}).slice(0, 2).byteAt(1)); + assertEquals(2, Bytes.of(new byte[]{1,2,3}).slice(1, 2).byteAt(0)); + assertEquals(4, Bytes.of(new byte[]{1,2,3,4}).slice(2, 4).byteAt(1)); + } + + @Test + void testBytesSliceArraycopy() { + checkArraycopy((src, srcPos, dest, destPos, length) -> Bytes.of((byte[]) src).slice(1, 4).arraycopy(srcPos, (byte[]) dest, destPos, length)); + } + + @Test + void testBytesSliceArraycopyMimicsSystemOne() { + checkArraycopy((src, srcPos, dest, destPos, length) -> System.arraycopy(Arrays.copyOfRange((byte[]) src, 1, 4), srcPos, dest, destPos, length)); + } + @Test void testCopyArrayOfRange() { - byte[] bArray = new byte[]{1, 2, 3, 4, 5, 6}; - byte[] expectedResult = new byte[]{3, 4, 5}; - assertArrayEquals(expectedResult, Bytes.of(bArray).slice(0, bArray.length).copyArrayOfRange(2, 5)); + checkCopyOfRange(BytesSlice::copyArrayOfRange, (origin, from, to) -> Bytes.of(origin).slice(from, to)); + } + + @Test + void testCopyArrayOfRangeMimicsSystemOne() { + checkCopyOfRange(Arrays::copyOfRange, Arrays::copyOfRange); } @Test @@ -74,4 +113,44 @@ void testEmptySlice() { assertEquals(0, actualResult.length()); assertArrayEquals(expectedResult, actualResult.copyArray()); } + + private static void checkArraycopy(Functions.Action5 fun) { + byte[] dest = new byte[3]; + byte[] origin = new byte[]{1,2,3,4,5}; + + assertThrows(NullPointerException.class, () -> fun.apply(origin, 0, null, 0, 3)); + + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, -1, dest, 0, 3)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, 0, dest, -1, 3)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, 0, dest, 0, -1)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, 0, dest, 0, 4)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, 1, dest, 0, 3)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, 0, dest, 1, 3)); + + assertArrayEquals(new byte[3], dest); // yet unmodified + + fun.apply(origin, 0, dest, 0, 3); + assertArrayEquals(new byte[]{2,3,4}, dest); + + byte[] dest2 = new byte[3]; + fun.apply(origin, 1, dest2, 1, 1); + assertArrayEquals(new byte[]{0,3,0}, dest2); + } + + private static void checkCopyOfRange(Functions.Function3 fun, + Functions.Function3 slicer) { + byte[] bArray = new byte[]{1, 2, 3, 4, 5, 6}; + + assertEquals(bArray.length, fun.apply(slicer.apply(bArray, 0, 6), 0, 6).length); + assertNotSame(bArray, fun.apply(slicer.apply(bArray, 0, 6), 0, 6)); + + assertArrayEquals(new byte[]{3, 4, 5}, fun.apply(slicer.apply(bArray, 0, 6), 2, 5)); + assertArrayEquals(new byte[]{3, 4}, fun.apply(slicer.apply(bArray, 1, 5), 1, 3)); + assertArrayEquals(new byte[]{3, 4, 5, 0, 0, 0, 0}, fun.apply(slicer.apply(bArray, 1, 5), 1, 8)); + assertArrayEquals(new byte[]{}, fun.apply(slicer.apply(bArray, 1, 5), 4, 4)); + + assertThrows(IllegalArgumentException.class, () -> fun.apply(slicer.apply(bArray, 1, 5), 1, 0)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(slicer.apply(bArray, 1, 5), -1, 0)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(slicer.apply(bArray, 1, 5), 5, 5)); + } } diff --git a/rskj-core/src/test/java/co/rsk/core/types/bytes/BytesTest.java b/rskj-core/src/test/java/co/rsk/core/types/bytes/BytesTest.java index b60ca431ddc..7cce5532f17 100644 --- a/rskj-core/src/test/java/co/rsk/core/types/bytes/BytesTest.java +++ b/rskj-core/src/test/java/co/rsk/core/types/bytes/BytesTest.java @@ -19,10 +19,13 @@ package co.rsk.core.types.bytes; +import co.rsk.util.Functions; import org.ethereum.TestUtils; import org.ethereum.util.ByteUtil; import org.junit.jupiter.api.Test; +import java.util.Arrays; + import static org.junit.jupiter.api.Assertions.*; class BytesTest { @@ -34,6 +37,41 @@ void testBytesOf() { assertNotNull(Bytes.of(new byte[]{1})); } + @Test + void testBytesLength() { + assertEquals(0, Bytes.of(new byte[]{}).length()); + assertEquals(1, Bytes.of(new byte[]{1}).length()); + } + + @Test + void testBytesAt() { + assertThrows(IndexOutOfBoundsException.class, () -> Bytes.of(new byte[]{}).byteAt(0)); + assertThrows(IndexOutOfBoundsException.class, () -> Bytes.of(new byte[]{1}).byteAt(1)); + assertThrows(IndexOutOfBoundsException.class, () -> Bytes.of(new byte[]{1}).byteAt(-1)); + assertEquals(1, Bytes.of(new byte[]{1}).byteAt(0)); + assertEquals(2, Bytes.of(new byte[]{1,2}).byteAt(1)); + } + + @Test + void testBytesArraycopy() { + checkArraycopy((src, srcPos, dest, destPos, length) -> Bytes.of((byte[]) src).arraycopy(srcPos, (byte[]) dest, destPos, length)); + } + + @Test + void testBytesArraycopyMimicsSystemOne() { + checkArraycopy(System::arraycopy); + } + + @Test + void testCopyArrayOfRange() { + checkCopyOfRange((original, from, to) -> Bytes.of(original).copyArrayOfRange(from, to)); + } + + @Test + void testCopyArrayOfRangeMimicsSystemOne() { + checkCopyOfRange(Arrays::copyOfRange); + } + @Test void testToPrintableString() { assertEquals("0a", Bytes.toPrintableString(new byte[]{10})); @@ -83,13 +121,17 @@ void testShortEnoughBytesToString() { @Test void testLongBytesToString() { - byte[] bArray1 = TestUtils.generateBytes("hash1",15); + byte[] bArray1 = TestUtils.generateBytes("hash1",16); byte[] bArray2 = TestUtils.generateBytes("hash2",15); byte[] finalArray = ByteUtil.merge(bArray1, new byte[]{1, 2, 3}, bArray2); - assertEquals(33, finalArray.length); + assertEquals(34, finalArray.length); + + Bytes bytes = Bytes.of(finalArray); + + assertEquals(64, String.format("%s", bytes).length()); - String actualMessage = String.format("Some '%s' hex", Bytes.of(finalArray)); + String actualMessage = String.format("Some '%s' hex", bytes); String expectedMessage = "Some '" + ByteUtil.toHexString(bArray1) + ".." @@ -116,4 +158,43 @@ void testEmptyBytesToString() { assertEquals(expectedMessage, actualMessage); } -} \ No newline at end of file + + private static void checkArraycopy(Functions.Action5 fun) { + byte[] dest = new byte[5]; + byte[] origin = new byte[]{1,2,3,4,5}; + + assertThrows(NullPointerException.class, () -> fun.apply(origin, 0, null, 0, 5)); + + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, -1, dest, 0, 5)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, 0, dest, -1, 5)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, 0, dest, 0, -1)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, 0, dest, 0, 6)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, 1, dest, 0, 5)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(origin, 0, dest, 1, 5)); + + assertArrayEquals(new byte[5], dest); // yet unmodified + + fun.apply(origin, 0, dest, 0, 5); + assertArrayEquals(new byte[]{1,2,3,4,5}, dest); + + byte[] dest2 = new byte[5]; + fun.apply(origin, 1, dest2, 1, 3); + assertArrayEquals(new byte[]{0,2,3,4,0}, dest2); + } + + private static void checkCopyOfRange(Functions.Function3 fun) { + byte[] bArray = new byte[]{1, 2, 3, 4, 5}; + + assertEquals(bArray.length, fun.apply(bArray, 0, 5).length); + assertNotSame(bArray, fun.apply(bArray, 0, 5)); + + assertArrayEquals(new byte[]{2, 3, 4}, fun.apply(bArray, 1, 4)); + assertArrayEquals(new byte[]{2}, fun.apply(bArray, 1, 2)); + assertArrayEquals(new byte[]{2, 3, 4, 5, 0, 0, 0}, fun.apply(bArray, 1, 8)); + assertArrayEquals(new byte[]{}, fun.apply(bArray, 3, 3)); + + assertThrows(IllegalArgumentException.class, () -> fun.apply(bArray, 1, 0)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(bArray, -1, 0)); + assertThrows(IndexOutOfBoundsException.class, () -> fun.apply(bArray, 6, 6)); + } +} diff --git a/rskj-core/src/test/java/co/rsk/util/Functions.java b/rskj-core/src/test/java/co/rsk/util/Functions.java new file mode 100644 index 00000000000..de687407e28 --- /dev/null +++ b/rskj-core/src/test/java/co/rsk/util/Functions.java @@ -0,0 +1,30 @@ +/* + * This file is part of RskJ + * Copyright (C) 2024 RSK Labs Ltd. + * (derived from ethereumJ library, Copyright (c) 2016 ) + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ + +package co.rsk.util; + +public interface Functions { + interface Function3 { + TResult apply(T1 arg1, T2 arg2, T3 arg3); + } + + interface Action5 { + void apply(T1 arg1, T2 arg2, T3 arg3, T4 arg4, T5 arg5); + } +}