You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "squah-confluent (via GitHub)" <gi...@apache.org> on 2026/06/05 14:44:25 UTC
[PR] KAFKA-20292 [1/N]: Prepare to split TargetAssignmentBuilders [kafka]
squah-confluent opened a new pull request, #22485:
URL: https://github.com/apache/kafka/pull/22485
To accommodate asynchronous assignments, such as those from client-side
assignors and assignors offloaded to background threads, we want to
split the TargetAssignmentBuilders into two: one builder for building
the target assignment and another for building the target assignment
records. Client-side assignors will only use the second builder.
Both builders require an up-to-date view of group members at the time
they are run. In the non-offloaded case, this is the same view. However,
the view needs to include the unwritten member operations from the
ongoing heartbeat request. Currently the operations are applied within
the TargetAssignmentBuilders. To avoid duplicating the logic once the
TargetAssignmentBuilders are split, we would like to lift it out and
pass the TargetAssignmentBuilders the updated view of members and
assignments.
Add an OverlayMap class, to be used to provide the updated views of
members and assignments.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20292 [1/N]: Prepare to split TargetAssignmentBuilders [kafka]
Posted by "squah-confluent (via GitHub)" <gi...@apache.org>.
squah-confluent commented on PR #22485:
URL: https://github.com/apache/kafka/pull/22485#issuecomment-4670363653
https://github.com/apache/kafka/compare/0d036c87a5895d2833e49c7b3ccba77511fc4f6f..65b29a766b1fc55bbd54b1f815d9c2ea9d9ede3d#diff-3348a3606859398432ede2a538363e9135c6b796f968ff7a1ecffa31293d5300
Added note to javadoc
```diff
+ * <p>Null values are not supported.
```
Added null check
```diff
public V put(K key, V value) {
+ Objects.requireNonNull(value);
```
https://github.com/apache/kafka/compare/0d036c87a5895d2833e49c7b3ccba77511fc4f6f..65b29a766b1fc55bbd54b1f815d9c2ea9d9ede3d#diff-272971e71983532a976ca13273331162b16622ad1227db72468f8114240cd97b
Removed unused method
```diff
- private static OverlayMap<String, String> populated() {
- ...
- }
```
Renamed method
```diff
- public void testIsEmptyWhenAllBaseRemoved() {
+ public void testIsEmptyWhenAllBaseEntriesRemoved() {
```
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20292 [1/N]: Prepare to split TargetAssignmentBuilders [kafka]
Posted by "squah-confluent (via GitHub)" <gi...@apache.org>.
squah-confluent commented on PR #22485:
URL: https://github.com/apache/kafka/pull/22485#issuecomment-4632799121
The usage of the new class can be found in #22486.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20292 [1/N]: Prepare to split TargetAssignmentBuilders [kafka]
Posted by "dongnuo123 (via GitHub)" <gi...@apache.org>.
dongnuo123 commented on code in PR #22485:
URL: https://github.com/apache/kafka/pull/22485#discussion_r3383885602
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/util/OverlayMapTest.java:
##########
@@ -0,0 +1,376 @@
+/*
+ * 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.kafka.coordinator.group.util;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.NoSuchElementException;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class OverlayMapTest {
+
+ // Test simple additions, replacements and removals. This is enough to populate the three
+ // internal fields of OverlayMap for use in the rest of the tests.
+
+ @Test
+ public void testPutNewKey() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "existing", "v1"
+ ));
+
+ assertNull(map.put("new", "x"));
+
+ assertEquals("x", map.get("new"));
+ }
+
+ @Test
+ public void testPutExistingKey() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "existing", "v1"
+ ));
+
+ assertEquals("v1", map.put("existing", "v2"));
+
+ assertEquals("v2", map.get("existing"));
+ }
+
+ @Test
+ public void testRemoveExistingKey() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "existing", "v1"
+ ));
+
+ assertEquals("v1", map.remove("existing"));
+
+ assertNull(map.get("existing"));
+ }
+
+ // Now test everything else.
+
+ /**
+ * Returns a map whose state populates all three overlay regions plus an untouched base key.
+ * Effective contents: {@code {addition=addition-value, replacement=replacement-new, base=base-value}}.
+ */
+ private static OverlayMap<String, String> populated() {
+ Map<String, String> base = Map.of(
+ "replacement", "replacement-base",
+ "removal", "removal-base",
+ "base", "base-value"
+ );
+ OverlayMap<String, String> map = new OverlayMap<>(base);
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-new");
+ map.remove("removal");
+ return map;
+ }
+
+ @Test
+ public void testSize() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ assertEquals(3, map.size());
+
+ // Additions increase the size.
+ map.put("addition", "addition-value");
+ assertEquals(4, map.size());
+
+ // Replacements do not change the size.
+ map.put("replacement", "replacement-value-2");
+ assertEquals(4, map.size());
+
+ // Removals decrease the size.
+ map.remove("removal");
+ assertEquals(3, map.size());
+ }
+
+ @Test
+ public void testIsEmptyWithEmptyBase() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of());
+ assertTrue(map.isEmpty());
+
+ // Additions make it non-empty.
+ map.put("k", "v");
+ assertFalse(map.isEmpty());
+ }
+
+ @Test
+ public void testIsEmptyWhenAllBaseRemoved() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "k1", "v1",
+ "k2", "v2"
+ ));
+
+ map.remove("k1");
+ assertFalse(map.isEmpty());
+
+ map.remove("k2");
+ assertTrue(map.isEmpty());
+ }
+
+ @Test
+ public void testContainsKey() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ assertTrue(map.containsKey("addition"));
+ assertTrue(map.containsKey("replacement"));
+ assertFalse(map.containsKey("removal"));
+ assertTrue(map.containsKey("base"));
+ assertFalse(map.containsKey("unknown"));
+ }
+
+ @Test
+ public void testGet() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ assertEquals("addition-value", map.get("addition"));
+ assertEquals("replacement-value-2", map.get("replacement"));
+ assertNull(map.get("removal"));
+ assertEquals("base-value", map.get("base"));
+ assertNull(map.get("unknown"));
+ }
+
+ @Test
+ public void testPut() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ assertEquals(3, map.size());
+
+ // Replace a new value.
+ assertEquals("addition-value", map.put("addition", "addition-value-2"));
+ assertEquals(3, map.size());
+ assertTrue(map.containsKey("addition"));
+ assertEquals("addition-value-2", map.get("addition"));
+ assertEquals(Map.of(
+ "addition", "addition-value-2",
+ "replacement", "replacement-value-2",
+ "base", "base-value"
+ ), map);
+
+ // Replace an already-replaced value.
+ assertEquals("replacement-value-2", map.put("replacement", "replacement-value-3"));
+ assertEquals(3, map.size());
+ assertTrue(map.containsKey("replacement"));
+ assertEquals("replacement-value-3", map.get("replacement"));
+ assertEquals(Map.of(
+ "addition", "addition-value-2",
+ "replacement", "replacement-value-3",
+ "base", "base-value"
+ ), map);
+
+ // Replace a removed value.
+ assertNull(map.put("removal", "removal-value-2"));
+ assertEquals(4, map.size());
+ assertTrue(map.containsKey("removal"));
+ assertEquals("removal-value-2", map.get("removal"));
+ assertEquals(Map.of(
+ "addition", "addition-value-2",
+ "replacement", "replacement-value-3",
+ "removal", "removal-value-2",
+ "base", "base-value"
+ ), map);
+
+ // Put over a base value and put over an unknown value are already test separately in
+ // testPutExistingKey and testPutNewKey.
+ }
+
+ @Test
+ public void testRemove() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ assertEquals(3, map.size());
+
+ // Remove a new value.
+ assertEquals("addition-value", map.remove("addition"));
+ assertEquals(2, map.size());
+ assertFalse(map.containsKey("addition"));
+ assertNull(map.get("addition"));
+ assertEquals(Map.of(
+ "replacement", "replacement-value-2",
+ "base", "base-value"
+ ), map);
+
+ // Remove a replaced value.
+ assertEquals("replacement-value-2", map.remove("replacement"));
+ assertEquals(1, map.size());
+ assertFalse(map.containsKey("replacement"));
+ assertNull(map.get("replacement"));
+ assertEquals(Map.of(
+ "base", "base-value"
+ ), map);
+
+ // Remove a removed value.
+ assertNull(map.remove("removal"));
+ assertEquals(1, map.size());
+ assertFalse(map.containsKey("removal"));
+ assertNull(map.get("removal"));
+ assertEquals(Map.of(
+ "base", "base-value"
+ ), map);
+
+ // Remove an unknown value.
+ assertNull(map.remove("unknown"));
+ assertEquals(1, map.size());
+ assertFalse(map.containsKey("unknown"));
+ assertNull(map.get("unknown"));
+ assertEquals(Map.of(
+ "base", "base-value"
+ ), map);
+
+ // Removing a base value is already tested separately in testRemoveExistingKey.
+ }
+
+ @Test
+ public void testClear() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+ map.clear();
+
+ assertEquals(0, map.size());
+ assertTrue(map.isEmpty());
+ assertFalse(map.containsKey("addition"));
+ assertFalse(map.containsKey("replacement"));
+ assertFalse(map.containsKey("base"));
+ assertNull(map.get("addition"));
+ assertNull(map.get("base"));
+ }
+
+ @Test
+ public void testEntrySet() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ assertEquals(
+ Map.of(
+ "addition", "addition-value",
+ "replacement", "replacement-value-2",
+ "base", "base-value"
+ ),
+ new HashMap<>(map)
+ );
+ }
+
+ @Test
+ public void testEntrySetIterator() {
+ Map<String, String> base = new LinkedHashMap<>();
+ base.put("base", "base-value");
+ base.put("replacement", "replacement-value");
+ base.put("removal", "removal-value");
+
+ OverlayMap<String, String> map = new OverlayMap<>(base);
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ Iterator<Map.Entry<String, String>> iterator = map.entrySet().iterator();
+
+ assertTrue(iterator.hasNext());
+ assertEquals(Map.entry("base", "base-value"), iterator.next());
+
+ assertTrue(iterator.hasNext());
+ assertEquals(Map.entry("replacement", "replacement-value-2"), iterator.next());
+
+ assertTrue(iterator.hasNext());
+ assertEquals(Map.entry("addition", "addition-value"), iterator.next());
+
+ assertFalse(iterator.hasNext());
+ assertThrows(NoSuchElementException.class, iterator::next);
+ }
+
+ @Test
+ public void testEntrySetIteratorOverEmptyMap() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of());
+
+ Iterator<Map.Entry<String, String>> iterator = map.entrySet().iterator();
+
+ assertFalse(iterator.hasNext());
+ assertThrows(NoSuchElementException.class, iterator::next);
+ }
+
+ @Test
+ public void testEntrySetSize() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ assertEquals(3, map.entrySet().size());
+
+ // Additions increase the size.
+ map.put("addition", "addition-value");
+ assertEquals(4, map.entrySet().size());
+
+ // Replacements do not change the size.
+ map.put("replacement", "replacement-value-2");
+ assertEquals(4, map.entrySet().size());
+
+ // Removals decrease the size.
+ map.remove("removal");
+ assertEquals(3, map.entrySet().size());
+ }
Review Comment:
This might not be used, so just out of curiosity, what happens if we do something like `put(key, null)`? would the iterator iterates on it?
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20292 [1/N]: Prepare to split TargetAssignmentBuilders [kafka]
Posted by "squah-confluent (via GitHub)" <gi...@apache.org>.
squah-confluent merged PR #22485:
URL: https://github.com/apache/kafka/pull/22485
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20292 [1/N]: Prepare to split TargetAssignmentBuilders [kafka]
Posted by "squah-confluent (via GitHub)" <gi...@apache.org>.
squah-confluent commented on code in PR #22485:
URL: https://github.com/apache/kafka/pull/22485#discussion_r3388309584
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/util/OverlayMapTest.java:
##########
@@ -0,0 +1,376 @@
+/*
+ * 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.kafka.coordinator.group.util;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.NoSuchElementException;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class OverlayMapTest {
+
+ // Test simple additions, replacements and removals. This is enough to populate the three
+ // internal fields of OverlayMap for use in the rest of the tests.
+
+ @Test
+ public void testPutNewKey() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "existing", "v1"
+ ));
+
+ assertNull(map.put("new", "x"));
+
+ assertEquals("x", map.get("new"));
+ }
+
+ @Test
+ public void testPutExistingKey() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "existing", "v1"
+ ));
+
+ assertEquals("v1", map.put("existing", "v2"));
+
+ assertEquals("v2", map.get("existing"));
+ }
+
+ @Test
+ public void testRemoveExistingKey() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "existing", "v1"
+ ));
+
+ assertEquals("v1", map.remove("existing"));
+
+ assertNull(map.get("existing"));
+ }
+
+ // Now test everything else.
+
+ /**
+ * Returns a map whose state populates all three overlay regions plus an untouched base key.
+ * Effective contents: {@code {addition=addition-value, replacement=replacement-new, base=base-value}}.
+ */
+ private static OverlayMap<String, String> populated() {
+ Map<String, String> base = Map.of(
+ "replacement", "replacement-base",
+ "removal", "removal-base",
+ "base", "base-value"
+ );
+ OverlayMap<String, String> map = new OverlayMap<>(base);
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-new");
+ map.remove("removal");
+ return map;
+ }
+
+ @Test
+ public void testSize() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ assertEquals(3, map.size());
+
+ // Additions increase the size.
+ map.put("addition", "addition-value");
+ assertEquals(4, map.size());
+
+ // Replacements do not change the size.
+ map.put("replacement", "replacement-value-2");
+ assertEquals(4, map.size());
+
+ // Removals decrease the size.
+ map.remove("removal");
+ assertEquals(3, map.size());
+ }
+
+ @Test
+ public void testIsEmptyWithEmptyBase() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of());
+ assertTrue(map.isEmpty());
+
+ // Additions make it non-empty.
+ map.put("k", "v");
+ assertFalse(map.isEmpty());
+ }
+
+ @Test
+ public void testIsEmptyWhenAllBaseRemoved() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "k1", "v1",
+ "k2", "v2"
+ ));
+
+ map.remove("k1");
+ assertFalse(map.isEmpty());
+
+ map.remove("k2");
+ assertTrue(map.isEmpty());
+ }
+
+ @Test
+ public void testContainsKey() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ assertTrue(map.containsKey("addition"));
+ assertTrue(map.containsKey("replacement"));
+ assertFalse(map.containsKey("removal"));
+ assertTrue(map.containsKey("base"));
+ assertFalse(map.containsKey("unknown"));
+ }
+
+ @Test
+ public void testGet() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ assertEquals("addition-value", map.get("addition"));
+ assertEquals("replacement-value-2", map.get("replacement"));
+ assertNull(map.get("removal"));
+ assertEquals("base-value", map.get("base"));
+ assertNull(map.get("unknown"));
+ }
+
+ @Test
+ public void testPut() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ assertEquals(3, map.size());
+
+ // Replace a new value.
+ assertEquals("addition-value", map.put("addition", "addition-value-2"));
+ assertEquals(3, map.size());
+ assertTrue(map.containsKey("addition"));
+ assertEquals("addition-value-2", map.get("addition"));
+ assertEquals(Map.of(
+ "addition", "addition-value-2",
+ "replacement", "replacement-value-2",
+ "base", "base-value"
+ ), map);
+
+ // Replace an already-replaced value.
+ assertEquals("replacement-value-2", map.put("replacement", "replacement-value-3"));
+ assertEquals(3, map.size());
+ assertTrue(map.containsKey("replacement"));
+ assertEquals("replacement-value-3", map.get("replacement"));
+ assertEquals(Map.of(
+ "addition", "addition-value-2",
+ "replacement", "replacement-value-3",
+ "base", "base-value"
+ ), map);
+
+ // Replace a removed value.
+ assertNull(map.put("removal", "removal-value-2"));
+ assertEquals(4, map.size());
+ assertTrue(map.containsKey("removal"));
+ assertEquals("removal-value-2", map.get("removal"));
+ assertEquals(Map.of(
+ "addition", "addition-value-2",
+ "replacement", "replacement-value-3",
+ "removal", "removal-value-2",
+ "base", "base-value"
+ ), map);
+
+ // Put over a base value and put over an unknown value are already test separately in
+ // testPutExistingKey and testPutNewKey.
+ }
+
+ @Test
+ public void testRemove() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ assertEquals(3, map.size());
+
+ // Remove a new value.
+ assertEquals("addition-value", map.remove("addition"));
+ assertEquals(2, map.size());
+ assertFalse(map.containsKey("addition"));
+ assertNull(map.get("addition"));
+ assertEquals(Map.of(
+ "replacement", "replacement-value-2",
+ "base", "base-value"
+ ), map);
+
+ // Remove a replaced value.
+ assertEquals("replacement-value-2", map.remove("replacement"));
+ assertEquals(1, map.size());
+ assertFalse(map.containsKey("replacement"));
+ assertNull(map.get("replacement"));
+ assertEquals(Map.of(
+ "base", "base-value"
+ ), map);
+
+ // Remove a removed value.
+ assertNull(map.remove("removal"));
+ assertEquals(1, map.size());
+ assertFalse(map.containsKey("removal"));
+ assertNull(map.get("removal"));
+ assertEquals(Map.of(
+ "base", "base-value"
+ ), map);
+
+ // Remove an unknown value.
+ assertNull(map.remove("unknown"));
+ assertEquals(1, map.size());
+ assertFalse(map.containsKey("unknown"));
+ assertNull(map.get("unknown"));
+ assertEquals(Map.of(
+ "base", "base-value"
+ ), map);
+
+ // Removing a base value is already tested separately in testRemoveExistingKey.
+ }
+
+ @Test
+ public void testClear() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+ map.clear();
+
+ assertEquals(0, map.size());
+ assertTrue(map.isEmpty());
+ assertFalse(map.containsKey("addition"));
+ assertFalse(map.containsKey("replacement"));
+ assertFalse(map.containsKey("base"));
+ assertNull(map.get("addition"));
+ assertNull(map.get("base"));
+ }
+
+ @Test
+ public void testEntrySet() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ assertEquals(
+ Map.of(
+ "addition", "addition-value",
+ "replacement", "replacement-value-2",
+ "base", "base-value"
+ ),
+ new HashMap<>(map)
+ );
+ }
+
+ @Test
+ public void testEntrySetIterator() {
+ Map<String, String> base = new LinkedHashMap<>();
+ base.put("base", "base-value");
+ base.put("replacement", "replacement-value");
+ base.put("removal", "removal-value");
+
+ OverlayMap<String, String> map = new OverlayMap<>(base);
+ map.put("addition", "addition-value");
+ map.put("replacement", "replacement-value-2");
+ map.remove("removal");
+
+ Iterator<Map.Entry<String, String>> iterator = map.entrySet().iterator();
+
+ assertTrue(iterator.hasNext());
+ assertEquals(Map.entry("base", "base-value"), iterator.next());
+
+ assertTrue(iterator.hasNext());
+ assertEquals(Map.entry("replacement", "replacement-value-2"), iterator.next());
+
+ assertTrue(iterator.hasNext());
+ assertEquals(Map.entry("addition", "addition-value"), iterator.next());
+
+ assertFalse(iterator.hasNext());
+ assertThrows(NoSuchElementException.class, iterator::next);
+ }
+
+ @Test
+ public void testEntrySetIteratorOverEmptyMap() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of());
+
+ Iterator<Map.Entry<String, String>> iterator = map.entrySet().iterator();
+
+ assertFalse(iterator.hasNext());
+ assertThrows(NoSuchElementException.class, iterator::next);
+ }
+
+ @Test
+ public void testEntrySetSize() {
+ OverlayMap<String, String> map = new OverlayMap<>(Map.of(
+ "base", "base-value",
+ "replacement", "replacement-value",
+ "removal", "removal-value"
+ ));
+ assertEquals(3, map.entrySet().size());
+
+ // Additions increase the size.
+ map.put("addition", "addition-value");
+ assertEquals(4, map.entrySet().size());
+
+ // Replacements do not change the size.
+ map.put("replacement", "replacement-value-2");
+ assertEquals(4, map.entrySet().size());
+
+ // Removals decrease the size.
+ map.remove("removal");
+ assertEquals(3, map.entrySet().size());
+ }
Review Comment:
Good question. It turns out that null replacements are not supported because `Map.entry()` disallows nulls and throws.
To support null values, we can replace `Map.entry` with `AbstractMap.SimpleImmutableEntry`. But since we don't require the ability to store `null` values in #22486, I'd rather not add all the extra tests. Instead I'll add a null check in `put()`.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org