8029055: Map.merge implementations should refuse null value param

Reviewed-by: briangoetz, dl
This commit is contained in:
Mike Duigou 2013-12-13 13:34:55 -08:00
parent 1e845ac91f
commit 5b90fb7e5e
4 changed files with 31 additions and 67 deletions

View File

@ -1212,6 +1212,8 @@ public class HashMap<K,V> extends AbstractMap<K,V>
@Override
public V merge(K key, V value,
BiFunction<? super V, ? super V, ? extends V> remappingFunction) {
if (value == null)
throw new NullPointerException();
if (remappingFunction == null)
throw new NullPointerException();
int hash = hash(key);

View File

@ -600,7 +600,7 @@ public interface Map<K,V> {
* @implSpec
* The default implementation is equivalent to, for this {@code map}:
* <pre> {@code
* for ((Map.Entry<K, V> entry : map.entrySet())
* for (Map.Entry<K, V> entry : map.entrySet())
* action.accept(entry.getKey(), entry.getValue());
* }</pre>
*
@ -640,7 +640,7 @@ public interface Map<K,V> {
* @implSpec
* <p>The default implementation is equivalent to, for this {@code map}:
* <pre> {@code
* for ((Map.Entry<K, V> entry : map.entrySet())
* for (Map.Entry<K, V> entry : map.entrySet())
* entry.setValue(function.apply(entry.getKey(), entry.getValue()));
* }</pre>
*
@ -1110,8 +1110,8 @@ public interface Map<K,V> {
/**
* If the specified key is not already associated with a value or is
* associated with null, associates it with the given value.
* Otherwise, replaces the value with the results of the given
* associated with null, associates it with the given non-null value.
* Otherwise, replaces the associated value with the results of the given
* remapping function, or removes if the result is {@code null}. This
* method may be of use when combining multiple mapped values for a key.
* For example, to either create or append a {@code String msg} to a
@ -1121,15 +1121,14 @@ public interface Map<K,V> {
* map.merge(key, msg, String::concat)
* }</pre>
*
* <p>If the function returns {@code null}, the mapping is removed (or
* remains absent if initially absent). If the function itself throws an
* (unchecked) exception, the exception is rethrown, and the current mapping
* is left unchanged.
* <p>If the function returns {@code null} the mapping is removed. If the
* function itself throws an (unchecked) exception, the exception is
* rethrown, and the current mapping is left unchanged.
*
* @implSpec
* The default implementation is equivalent to performing the
* following steps for this {@code map}, then returning the
* current value or {@code null} if absent:
* The default implementation is equivalent to performing the following
* steps for this {@code map}, then returning the current value or
* {@code null} if absent:
*
* <pre> {@code
* V oldValue = map.get(key);
@ -1137,8 +1136,6 @@ public interface Map<K,V> {
* remappingFunction.apply(oldValue, value);
* if (newValue == null)
* map.remove(key);
* else if (oldValue == null)
* map.remove(key);
* else
* map.put(key, newValue);
* }</pre>
@ -1151,42 +1148,36 @@ public interface Map<K,V> {
* whether the function is applied once atomically only if the value is not
* present.
*
* @param key key with which the specified value is to be associated
* @param value the value to use if absent
* @param key key with which the resulting value is to be associated
* @param value the non-null value to be merged with the existing value
* associated with the key or, if no existing value or a null value
* is associated with the key, to be associated with the key
* @param remappingFunction the function to recompute a value if present
* @return the new value associated with the specified key, or null if none
* @return the new value associated with the specified key, or null if no
* value is associated with the key
* @throws UnsupportedOperationException if the {@code put} operation
* is not supported by this map
* (<a href="Collection.html#optional-restrictions">optional</a>)
* @throws ClassCastException if the class of the specified key or value
* prevents it from being stored in this map
* (<a href="Collection.html#optional-restrictions">optional</a>)
* @throws NullPointerException if the specified key is null and
* this map does not support null keys, or the remappingFunction
* is null
* @throws NullPointerException if the specified key is null and this map
* does not support null keys or the value or remappingFunction is
* null
* @since 1.8
*/
default V merge(K key, V value,
BiFunction<? super V, ? super V, ? extends V> remappingFunction) {
Objects.requireNonNull(remappingFunction);
Objects.requireNonNull(value);
V oldValue = get(key);
if (oldValue != null) {
V newValue = remappingFunction.apply(oldValue, value);
if (newValue != null) {
put(key, newValue);
return newValue;
} else {
remove(key);
return null;
}
V newValue = (oldValue == null) ? value :
remappingFunction.apply(oldValue, value);
if(newValue == null) {
remove(key);
} else {
if (value == null) {
remove(key);
return null;
} else {
put(key, value);
return value;
}
put(key, newValue);
}
return newValue;
}
}

View File

@ -463,9 +463,9 @@ public interface ConcurrentMap<K, V> extends Map<K, V> {
* {@inheritDoc}
*
* @implSpec
* The default implementation is equivalent to performing the
* following steps for this {@code map}, then returning the
* current value or {@code null} if absent:
* The default implementation is equivalent to performing the following
* steps for this {@code map}, then returning the current value or
* {@code null} if absent:
*
* <pre> {@code
* V oldValue = map.get(key);
@ -473,8 +473,6 @@ public interface ConcurrentMap<K, V> extends Map<K, V> {
* remappingFunction.apply(oldValue, value);
* if (newValue == null)
* map.remove(key);
* else if (oldValue == null)
* map.remove(key);
* else
* map.put(key, newValue);
* }</pre>

View File

@ -767,7 +767,6 @@ public class Defaults {
Collection<Object[]> cases = new ArrayList<>();
cases.addAll(makeMergeTestCases());
cases.addAll(makeMergeNullValueTestCases());
return cases.iterator();
}
@ -790,32 +789,6 @@ public class Defaults {
return cases;
}
static Collection<Object[]> makeMergeNullValueTestCases() {
Collection<Object[]> cases = new ArrayList<>();
for( Object[] mapParams : makeAllRWMapsWithNulls() ) {
cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.OLDVALUE, Merging.Value.NULL, Merging.Merger.NULL, Merging.Value.ABSENT, Merging.Value.NULL });
}
for( Object[] mapParams : makeAllRWMapsWithNulls() ) {
cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.OLDVALUE, Merging.Value.NULL, Merging.Merger.RESULT, Merging.Value.RESULT, Merging.Value.RESULT });
}
for( Object[] mapParams : makeAllRWMapsWithNulls() ) {
cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.ABSENT, Merging.Value.NULL, Merging.Merger.UNUSED, Merging.Value.ABSENT, Merging.Value.NULL });
}
for( Object[] mapParams : makeAllRWMapsWithNulls() ) {
cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.NULL, Merging.Value.NULL, Merging.Merger.UNUSED, Merging.Value.ABSENT, Merging.Value.NULL });
}
for( Object[] mapParams : makeAllRWMapsWithNulls() ) {
cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.NULL, Merging.Value.NEWVALUE, Merging.Merger.UNUSED, Merging.Value.NEWVALUE, Merging.Value.NEWVALUE });
}
return cases;
}
public interface Thrower<T extends Throwable> {
public void run() throws T;