Browse Source

Fix contract violations in ConcurrentReferenceHashMap's EntrySet/Iterator

Closes gh-27454

(cherry picked from commit 208fafa4a3)
pull/28694/head
Juergen Hoeller 5 years ago
parent
commit
ebd351753f
  1. 3
      spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java
  2. 27
      spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java

3
spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java

@ -858,7 +858,7 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
Reference<K, V> ref = ConcurrentReferenceHashMap.this.getReference(entry.getKey(), Restructure.NEVER); Reference<K, V> ref = ConcurrentReferenceHashMap.this.getReference(entry.getKey(), Restructure.NEVER);
Entry<K, V> otherEntry = (ref != null ? ref.get() : null); Entry<K, V> otherEntry = (ref != null ? ref.get() : null);
if (otherEntry != null) { if (otherEntry != null) {
return ObjectUtils.nullSafeEquals(otherEntry.getValue(), otherEntry.getValue()); return ObjectUtils.nullSafeEquals(entry.getValue(), otherEntry.getValue());
} }
} }
return false; return false;
@ -966,6 +966,7 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
public void remove() { public void remove() {
Assert.state(this.last != null, "No element to remove"); Assert.state(this.last != null, "No element to remove");
ConcurrentReferenceHashMap.this.remove(this.last.getKey()); ConcurrentReferenceHashMap.this.remove(this.last.getKey());
this.last = null;
} }
} }

27
spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2019 the original author or authors. * Copyright 2002-2021 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -41,11 +41,13 @@ import org.springframework.util.comparator.NullSafeComparator;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
/** /**
* Tests for {@link ConcurrentReferenceHashMap}. * Tests for {@link ConcurrentReferenceHashMap}.
* *
* @author Phillip Webb * @author Phillip Webb
* @author Juergen Hoeller
*/ */
class ConcurrentReferenceHashMapTests { class ConcurrentReferenceHashMapTests {
@ -466,6 +468,7 @@ class ConcurrentReferenceHashMapTests {
iterator.next(); iterator.next();
iterator.next(); iterator.next();
iterator.remove(); iterator.remove();
assertThatIllegalStateException().isThrownBy(iterator::remove);
iterator.next(); iterator.next();
assertThat(iterator.hasNext()).isFalse(); assertThat(iterator.hasNext()).isFalse();
assertThat(this.map).hasSize(2); assertThat(this.map).hasSize(2);
@ -486,6 +489,26 @@ class ConcurrentReferenceHashMapTests {
assertThat(this.map.get(2)).isEqualTo("2b"); assertThat(this.map.get(2)).isEqualTo("2b");
} }
@Test
void containsViaEntrySet() {
this.map.put(1, "1");
this.map.put(2, "2");
this.map.put(3, "3");
Set<Map.Entry<Integer, String>> entrySet = this.map.entrySet();
Set<Map.Entry<Integer, String>> copy = new HashMap<>(this.map).entrySet();
copy.forEach(entry -> assertThat(entrySet.contains(entry)).isTrue());
this.map.put(1, "A");
this.map.put(2, "B");
this.map.put(3, "C");
copy.forEach(entry -> assertThat(entrySet.contains(entry)).isFalse());
this.map.put(1, "1");
this.map.put(2, "2");
this.map.put(3, "3");
copy.forEach(entry -> assertThat(entrySet.contains(entry)).isTrue());
entrySet.clear();
copy.forEach(entry -> assertThat(entrySet.contains(entry)).isFalse());
}
@Test @Test
@Disabled("Intended for use during development only") @Disabled("Intended for use during development only")
void shouldBeFasterThanSynchronizedMap() throws InterruptedException { void shouldBeFasterThanSynchronizedMap() throws InterruptedException {
@ -583,7 +606,7 @@ class ConcurrentReferenceHashMapTests {
} }
// For testing we want more control of the hash // For testing we want more control of the hash
this.supplementalHash = super.getHash(o); this.supplementalHash = super.getHash(o);
return o == null ? 0 : o.hashCode(); return (o != null ? o.hashCode() : 0);
} }
public int getSupplementalHash() { public int getSupplementalHash() {

Loading…
Cancel
Save