Skip to content

Commit

Permalink
Ensure getOrComputeIfAbsent() does not shadow values from parent
Browse files Browse the repository at this point in the history
Prior to this commit, the getOrComputeIfAbsent() methods in
ExtensionContext.Store only performed a local lookup before invoking
the supplied defaultCreator function. Consequently, even if a value had
been stored under the supplied key in the parent Store, the
getOrComputeIfAbsent() methods always created a new default value and
stored it locally, thereby shadowing the desired, existing value from
the parent.

This commit addresses this issue by performing lookups in parents before
invoking the defaultCreator function.

Fixes: #349
  • Loading branch information
sbrannen committed Jun 23, 2016
1 parent 4ac2596 commit bc5d93a
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 14 deletions.
Expand Up @@ -152,11 +152,12 @@ default Store getStore() {
interface Store {

/**
* Get the value that has been stored under the supplied {@code key}.
* Get the value that is stored under the supplied {@code key}.
*
* <p>If no value has been saved in the current {@link ExtensionContext}
* <p>If no value is stored in the current {@link ExtensionContext}
* for the supplied {@code key}, ancestors of the context will be queried
* for a value with the same {@code key} in the store's {@code Namespace}.
* for a value with the same {@code key} in the {@code Namespace} used
* to create this store.
*
* <p>For greater type safety, consider using {@link #get(Object, Class)}
* instead.
Expand All @@ -168,12 +169,13 @@ interface Store {
Object get(Object key);

/**
* Get the value of the specified required type that has been stored under
* Get the value of the specified required type that is stored under
* the supplied {@code key}.
*
* <p>If no value has been saved in the current {@link ExtensionContext}
* <p>If no value is stored in the current {@link ExtensionContext}
* for the supplied {@code key}, ancestors of the context will be queried
* for a value with the same {@code key} in the store's {@code Namespace}.
* for a value with the same {@code key} in the {@code Namespace} used
* to create this store.
*
* @param key the key; never {@code null}
* @param requiredType the required type of the value; never {@code null}
Expand All @@ -185,7 +187,10 @@ interface Store {
/**
* Get the value that is stored under the supplied {@code key}.
*
* <p>If no value is currently stored under the supplied {@code key},
* <p>If no value is stored in the current {@link ExtensionContext}
* for the supplied {@code key}, ancestors of the context will be queried
* for a value with the same {@code key} in the {@code Namespace} used
* to create this store. If no value is found for the supplied {@code key},
* a new value will be computed by the {@code defaultCreator} (given
* the {@code key} as input), stored, and returned.
*
Expand All @@ -206,7 +211,10 @@ interface Store {
* Get the value of the specified required type that is stored under the
* supplied {@code key}.
*
* <p>If no value is currently stored under the supplied {@code key},
* <p>If no value is stored in the current {@link ExtensionContext}
* for the supplied {@code key}, ancestors of the context will be queried
* for a value with the same {@code key} in the {@code Namespace} used
* to create this store. If no value is found for the supplied {@code key},
* a new value will be computed by the {@code defaultCreator} (given
* the {@code key} as input), stored, and returned.
*
Expand Down
Expand Up @@ -66,8 +66,13 @@ <T> T get(Namespace namespace, Object key, Class<T> requiredType) {
<K, V> Object getOrComputeIfAbsent(Namespace namespace, K key, Function<K, V> defaultCreator) {
StoredValue storedValue = getStoredValue(namespace, key);
if (storedValue == null) {
storedValue = new StoredValue(defaultCreator.apply(key));
putStoredValue(namespace, key, storedValue);
if (parentStore != null) {
storedValue = parentStore.getStoredValue(namespace, key);
}
if (storedValue == null) {
storedValue = new StoredValue(defaultCreator.apply(key));
putStoredValue(namespace, key, storedValue);
}
}
return storedValue.value;
}
Expand Down
Expand Up @@ -40,6 +40,7 @@
* {@linkplain MethodBasedTestExtensionContext}
*
* @since 5.0
* @see ExtensionValuesStoreTests
*/
public class ExtensionContextTests {

Expand Down Expand Up @@ -164,6 +165,7 @@ public void usingStore() {
final Object parentKey = "parent key";
final String parentValue = "parent value";
parentStore.put(parentKey, parentValue);
assertEquals(parentValue, childStore.getOrComputeIfAbsent(parentKey, k -> "a different value"));
assertEquals(parentValue, childStore.get(parentKey));
}

Expand Down
Expand Up @@ -26,8 +26,9 @@
* Microtests for {@link ExtensionValuesStore}.
*
* @since 5.0
* @see ExtensionContextTests
*/
class ExtensionValuesStoreTests {
public class ExtensionValuesStoreTests {

private final Object key = "key";
private final Object value = "value";
Expand Down Expand Up @@ -69,24 +70,33 @@ void valueCanBeReplaced() {

@Test
void valueIsComputedIfAbsent() {
assertNull(store.get(namespace, key));
assertEquals(value, store.getOrComputeIfAbsent(namespace, key, innerKey -> value));
assertEquals(value, store.get(namespace, key));
}

@Test
void valueIsNotComputedIfPresent() {
void valueIsNotComputedIfPresentLocally() {
store.put(namespace, key, value);

assertEquals(value, store.getOrComputeIfAbsent(namespace, key, innerKey -> "a different value"));
assertEquals(value, store.get(namespace, key));
}

@Test
void valueIsNotComputedIfPresentInParent() {
parentStore.put(namespace, key, value);

assertEquals(value, store.getOrComputeIfAbsent(namespace, key, k -> "a different value"));
assertEquals(value, store.get(namespace, key));
}

@Test
void nullIsAValidValueToPut() {
store.put(namespace, key, null);

assertEquals(null, store.getOrComputeIfAbsent(namespace, key, innerKey -> "a different value"));
assertEquals(null, store.get(namespace, key));
assertNull(store.getOrComputeIfAbsent(namespace, key, innerKey -> "a different value"));
assertNull(store.get(namespace, key));
}

@Test
Expand Down

0 comments on commit bc5d93a

Please sign in to comment.