From 91a508664f73e3d99d30c7a30adcdda565128e1b Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Wed, 4 Mar 2015 15:23:34 +0100
Subject: [PATCH] Document the contract of Attribute.getAndSet(...) and
 set(...)

Motivation:

Attribute.getAndRemove() will return the value but also remove the AttributeKey itself from the AttributeMap. This may not
what you want as you may want to keep an instance of it and just set it later again. Document the contract so the user know what to expect.

Modifications:

- Make it clear when to use AttributeKey.getAndRemove() / AttributeKey.remove() and when AttributeKey.getAndSet(null) / AttributeKey.set(null).

Result:

Less suprising behaviour.
---
 .../main/java/io/netty/util/Attribute.java    | 14 ++++++++---
 .../netty/util/DefaultAttributeMapTest.java   | 25 ++++++++++++++++---
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/common/src/main/java/io/netty/util/Attribute.java b/common/src/main/java/io/netty/util/Attribute.java
index 4e4e44b971..42f3e4fbb9 100644
--- a/common/src/main/java/io/netty/util/Attribute.java
+++ b/common/src/main/java/io/netty/util/Attribute.java
@@ -43,14 +43,17 @@ public interface Attribute<T> {
     T getAndSet(T value);
 
     /**
-     *  Atomically sets to the given value if this {@link Attribute} does not contain a value at the moment.
+     *  Atomically sets to the given value if this {@link Attribute}'s value is {@code null}.
      *  If it was not possible to set the value as it contains a value it will just return the current value.
      */
     T setIfAbsent(T value);
 
     /**
-     * Removes this attribute from the {@link AttributeMap} and returns the old value..  Subsequent {@link #get()}
-     * calls will return @{code null}.
+     * Removes this attribute from the {@link AttributeMap} and returns the old value. Subsequent {@link #get()}
+     * calls will return {@code null}.
+     *
+     * If you only want to return the old value and clear the {@link Attribute} while still keep it in
+     * {@link AttributeMap} use {@link #getAndSet(Object)} with a value of {@code null}.
      */
     T getAndRemove();
 
@@ -61,7 +64,10 @@ public interface Attribute<T> {
     boolean compareAndSet(T oldValue, T newValue);
 
     /**
-     * Removes this attribute from the {@link AttributeMap}.  Subsequent {@link #get()} calls will return @{code null}.
+     * Removes this attribute from the {@link AttributeMap}. Subsequent {@link #get()} calls will return @{code null}.
+     *
+     * If you only want to remove the value and clear the {@link Attribute} while still keep it in
+     * {@link AttributeMap} use {@link #set(Object)} with a value of {@code null}.
      */
     void remove();
 }
diff --git a/common/src/test/java/io/netty/util/DefaultAttributeMapTest.java b/common/src/test/java/io/netty/util/DefaultAttributeMapTest.java
index 1bdb8b019a..447512688b 100644
--- a/common/src/test/java/io/netty/util/DefaultAttributeMapTest.java
+++ b/common/src/test/java/io/netty/util/DefaultAttributeMapTest.java
@@ -73,10 +73,27 @@ public class DefaultAttributeMapTest {
     public void testSetRemove() {
         AttributeKey<Integer> key = AttributeKey.valueOf("key");
 
-        map.attr(key).set(1);
-        assertSame(1, map.attr(key).getAndRemove());
+        Attribute<Integer> attr = map.attr(key);
+        attr.set(1);
+        assertSame(1, attr.getAndRemove());
 
-        map.attr(key).set(2);
-        assertSame(2, map.attr(key).get());
+        Attribute<Integer> attr2 = map.attr(key);
+        attr2.set(2);
+        assertSame(2, attr2.get());
+        assertNotSame(attr, attr2);
+    }
+
+    @Test
+    public void testGetAndSetWithNull() {
+        AttributeKey<Integer> key = AttributeKey.valueOf("key");
+
+        Attribute<Integer> attr = map.attr(key);
+        attr.set(1);
+        assertSame(1, attr.getAndSet(null));
+
+        Attribute<Integer> attr2 = map.attr(key);
+        attr2.set(2);
+        assertSame(2, attr2.get());
+        assertSame(attr, attr2);
     }
 }