You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by ch...@apache.org on 2024/03/15 13:18:31 UTC
(kafka) branch trunk updated: MINOR: Cleanup BoundedList to Make Constructors More Safe (#15507)
This is an automated email from the ASF dual-hosted git repository.
chia7712 pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/trunk by this push:
new e878654e95c MINOR: Cleanup BoundedList to Make Constructors More Safe (#15507)
e878654e95c is described below
commit e878654e95c3f82863a7b199fc3af56994f9841c
Author: Chris Holland <41...@users.noreply.github.com>
AuthorDate: Fri Mar 15 06:18:24 2024 -0700
MINOR: Cleanup BoundedList to Make Constructors More Safe (#15507)
Reviewers: Chia-Ping Tsai <ch...@gmail.com>
---
.../apache/kafka/server/mutable/BoundedList.java | 8 +-
.../kafka/server/mutable/BoundedListTest.java | 120 +++++++++++++--------
2 files changed, 81 insertions(+), 47 deletions(-)
diff --git a/server-common/src/main/java/org/apache/kafka/server/mutable/BoundedList.java b/server-common/src/main/java/org/apache/kafka/server/mutable/BoundedList.java
index 6c5d1ba0d5a..d31de006b78 100644
--- a/server-common/src/main/java/org/apache/kafka/server/mutable/BoundedList.java
+++ b/server-common/src/main/java/org/apache/kafka/server/mutable/BoundedList.java
@@ -40,18 +40,22 @@ public class BoundedList<E> implements List<E> {
}
public static <E> BoundedList<E> newArrayBacked(int maxLength, int initialCapacity) {
+ if (initialCapacity <= 0) {
+ throw new IllegalArgumentException("Invalid non-positive initialCapacity of " + initialCapacity);
+ }
return new BoundedList<>(maxLength, new ArrayList<>(initialCapacity));
}
- public BoundedList(int maxLength, List<E> underlying) {
+ private BoundedList(int maxLength, List<E> underlying) {
if (maxLength <= 0) {
throw new IllegalArgumentException("Invalid non-positive maxLength of " + maxLength);
}
- this.maxLength = maxLength;
+
if (underlying.size() > maxLength) {
throw new BoundedListTooLongException("Cannot wrap list, because it is longer than " +
"the maximum length " + maxLength);
}
+ this.maxLength = maxLength;
this.underlying = underlying;
}
diff --git a/server-common/src/test/java/org/apache/kafka/server/mutable/BoundedListTest.java b/server-common/src/test/java/org/apache/kafka/server/mutable/BoundedListTest.java
index a5a71f264ba..df2608430f4 100644
--- a/server-common/src/test/java/org/apache/kafka/server/mutable/BoundedListTest.java
+++ b/server-common/src/test/java/org/apache/kafka/server/mutable/BoundedListTest.java
@@ -20,9 +20,7 @@ package org.apache.kafka.server.mutable;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
-import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collections;
import java.util.List;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
@@ -33,33 +31,46 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
@Timeout(120)
public class BoundedListTest {
+
@Test
public void testMaxLengthMustNotBeZero() {
assertEquals("Invalid non-positive maxLength of 0",
assertThrows(IllegalArgumentException.class,
- () -> new BoundedList<>(0, new ArrayList<Integer>())).
- getMessage());
+ () -> BoundedList.newArrayBacked(0)).getMessage());
+
+ assertEquals("Invalid non-positive maxLength of 0",
+ assertThrows(IllegalArgumentException.class,
+ () -> BoundedList.newArrayBacked(0, 100)).getMessage());
}
@Test
public void testMaxLengthMustNotBeNegative() {
assertEquals("Invalid non-positive maxLength of -123",
assertThrows(IllegalArgumentException.class,
- () -> new BoundedList<>(-123, new ArrayList<Integer>())).
- getMessage());
+ () -> BoundedList.newArrayBacked(-123)).getMessage());
+
+ assertEquals("Invalid non-positive maxLength of -123",
+ assertThrows(IllegalArgumentException.class,
+ () -> BoundedList.newArrayBacked(-123, 100)).getMessage());
+ }
+
+ @Test
+ public void testInitialCapacityMustNotBeZero() {
+ assertEquals("Invalid non-positive initialCapacity of 0",
+ assertThrows(IllegalArgumentException.class,
+ () -> BoundedList.newArrayBacked(100, 0)).getMessage());
}
@Test
- public void testOwnedListMustNotBeTooLong() {
- assertEquals("Cannot wrap list, because it is longer than the maximum length 1",
- assertThrows(BoundedListTooLongException.class,
- () -> new BoundedList<>(1, new ArrayList<>(Arrays.asList(1, 2)))).
- getMessage());
+ public void testInitialCapacityMustNotBeNegative() {
+ assertEquals("Invalid non-positive initialCapacity of -123",
+ assertThrows(IllegalArgumentException.class,
+ () -> BoundedList.newArrayBacked(100, -123)).getMessage());
}
@Test
public void testAddingToBoundedList() {
- BoundedList<Integer> list = new BoundedList<>(2, new ArrayList<>(3));
+ BoundedList<Integer> list = BoundedList.newArrayBacked(2);
assertEquals(0, list.size());
assertTrue(list.isEmpty());
assertTrue(list.add(456));
@@ -70,42 +81,42 @@ public class BoundedListTest {
assertEquals("Cannot add another element to the list because it would exceed the " +
"maximum length of 2",
assertThrows(BoundedListTooLongException.class,
- () -> list.add(912)).
- getMessage());
+ () -> list.add(912)).getMessage());
assertEquals("Cannot add another element to the list because it would exceed the " +
"maximum length of 2",
assertThrows(BoundedListTooLongException.class,
- () -> list.add(0, 912)).
- getMessage());
- }
-
- private static <E> void testHashCodeAndEquals(List<E> a) {
- assertEquals(a, new BoundedList<>(123, a));
- assertEquals(a.hashCode(), new BoundedList<>(123, a).hashCode());
- }
-
- @Test
- public void testHashCodeAndEqualsForEmptyList() {
- testHashCodeAndEquals(Collections.emptyList());
+ () -> list.add(0, 912)).getMessage());
}
@Test
public void testHashCodeAndEqualsForNonEmptyList() {
- testHashCodeAndEquals(Arrays.asList(1, 2, 3, 4, 5, 6, 7));
+ BoundedList<Integer> boundedList = BoundedList.newArrayBacked(7);
+ List<Integer> otherList = Arrays.asList(1, 2, 3, 4, 5, 6, 7);
+ boundedList.addAll(otherList);
+
+ assertEquals(otherList, boundedList);
+ assertEquals(otherList.hashCode(), boundedList.hashCode());
}
@Test
public void testSet() {
- ArrayList<Integer> underlying = new ArrayList<>(Arrays.asList(1, 2, 3));
- BoundedList<Integer> list = new BoundedList<>(3, underlying);
- list.set(1, 200);
+ BoundedList<Integer> list = BoundedList.newArrayBacked(3);
+ list.add(1);
+ list.add(200);
+ list.add(3);
assertEquals(Arrays.asList(1, 200, 3), list);
+ list.set(0, 100);
+ list.set(1, 200);
+ list.set(2, 300);
+ assertEquals(Arrays.asList(100, 200, 300), list);
}
@Test
public void testRemove() {
- ArrayList<String> underlying = new ArrayList<>(Arrays.asList("a", "a", "c"));
- BoundedList<String> list = new BoundedList<>(3, underlying);
+ BoundedList<String> list = BoundedList.newArrayBacked(3);
+ list.add("a");
+ list.add("a");
+ list.add("c");
assertEquals(0, list.indexOf("a"));
assertEquals(1, list.lastIndexOf("a"));
list.remove("a");
@@ -116,8 +127,10 @@ public class BoundedListTest {
@Test
public void testClear() {
- ArrayList<String> underlying = new ArrayList<>(Arrays.asList("a", "b", "c"));
- BoundedList<String> list = new BoundedList<>(3, underlying);
+ BoundedList<String> list = BoundedList.newArrayBacked(3);
+ list.add("a");
+ list.add("a");
+ list.add("c");
list.clear();
assertEquals(Arrays.asList(), list);
assertTrue(list.isEmpty());
@@ -125,38 +138,49 @@ public class BoundedListTest {
@Test
public void testGet() {
- BoundedList<Integer> list = new BoundedList<>(3, Arrays.asList(1, 2, 3));
+ BoundedList<Integer> list = BoundedList.newArrayBacked(3);
+ list.add(1);
+ list.add(2);
+ list.add(3);
+ assertEquals(1, list.get(0));
assertEquals(2, list.get(1));
+ assertEquals(3, list.get(2));
}
@Test
public void testToArray() {
- BoundedList<Integer> list = new BoundedList<>(3, Arrays.asList(1, 2, 3));
+ BoundedList<Integer> list = BoundedList.newArrayBacked(3);
+ list.add(1);
+ list.add(2);
+ list.add(3);
assertArrayEquals(new Integer[] {1, 2, 3}, list.toArray());
assertArrayEquals(new Integer[] {1, 2, 3}, list.toArray(new Integer[3]));
}
@Test
public void testAddAll() {
- ArrayList<String> underlying = new ArrayList<>(Arrays.asList("a", "b", "c"));
- BoundedList<String> list = new BoundedList<>(5, underlying);
+ BoundedList<String> list = BoundedList.newArrayBacked(5);
+ list.add("a");
+ list.add("b");
+ list.add("c");
assertEquals("Cannot add another 3 element(s) to the list because it would exceed the " +
"maximum length of 5",
assertThrows(BoundedListTooLongException.class,
- () -> list.addAll(Arrays.asList("d", "e", "f"))).
- getMessage());
+ () -> list.addAll(Arrays.asList("d", "e", "f"))).getMessage());
assertEquals("Cannot add another 3 element(s) to the list because it would exceed the " +
"maximum length of 5",
assertThrows(BoundedListTooLongException.class,
- () -> list.addAll(0, Arrays.asList("d", "e", "f"))).
- getMessage());
+ () -> list.addAll(0, Arrays.asList("d", "e", "f"))).getMessage());
list.addAll(Arrays.asList("d", "e"));
assertEquals(Arrays.asList("a", "b", "c", "d", "e"), list);
}
@Test
public void testIterator() {
- BoundedList<Integer> list = new BoundedList<>(3, Arrays.asList(1, 2, 3));
+ BoundedList<Integer> list = BoundedList.newArrayBacked(3);
+ list.add(1);
+ list.add(2);
+ list.add(3);
assertEquals(1, list.iterator().next());
assertEquals(1, list.listIterator().next());
assertEquals(3, list.listIterator(2).next());
@@ -165,7 +189,10 @@ public class BoundedListTest {
@Test
public void testIteratorIsImmutable() {
- BoundedList<Integer> list = new BoundedList<>(3, new ArrayList<>(Arrays.asList(1, 2, 3)));
+ BoundedList<Integer> list = BoundedList.newArrayBacked(3);
+ list.add(1);
+ list.add(2);
+ list.add(3);
assertThrows(UnsupportedOperationException.class,
() -> list.iterator().remove());
assertThrows(UnsupportedOperationException.class,
@@ -174,7 +201,10 @@ public class BoundedListTest {
@Test
public void testSubList() {
- BoundedList<Integer> list = new BoundedList<>(3, new ArrayList<>(Arrays.asList(1, 2, 3)));
+ BoundedList<Integer> list = BoundedList.newArrayBacked(3);
+ list.add(1);
+ list.add(2);
+ list.add(3);
assertEquals(Arrays.asList(2), list.subList(1, 2));
assertThrows(UnsupportedOperationException.class,
() -> list.subList(1, 2).remove(2));