Repository: eclipse/leshan (java)

Issue:
Unset Write-Attributes
Hi,

Leshan version : M10

I tired unset the previously enable writeAttribute request by sending Write-Attributes /5/0/7?pmin=&pmax= but leshan expect value to be passed .  

Regards,
Madhu.

Pull request description:
Add unset support to write attributes request
This aims to fix #784 issue.

Patch:
diff --git a/leshan-core/src/main/java/org/eclipse/leshan/core/attributes/Attribute.java b/leshan-core/src/main/java/org/eclipse/leshan/core/attributes/Attribute.java
index 3747ec17..460426d2 100644
--- a/leshan-core/src/main/java/org/eclipse/leshan/core/attributes/Attribute.java
+++ b/leshan-core/src/main/java/org/eclipse/leshan/core/attributes/Attribute.java
@@ -86,17 +86,22 @@ public class Attribute {
     private final AttributeModel model;
     private final Object value;
 
-    public Attribute(String coRELinkParam, Object value) {
+    public Attribute(String coRELinkParam) {
         Validate.notEmpty(coRELinkParam);
         this.model = modelMap.get(coRELinkParam);
         if (model == null) {
             throw new IllegalArgumentException(String.format("Unsupported attribute '%s'", coRELinkParam));
         }
-        this.value = ensureMatchingValue(model, value);
+        this.value = null;
     }
 
-    public static Attribute create(String coRELinkParam, Object value) {
-        return new Attribute(coRELinkParam, value);
+    public Attribute(String coRELinkParam, Object value) {
+        Validate.notEmpty(coRELinkParam);
+        this.model = modelMap.get(coRELinkParam);
+        if (model == null) {
+            throw new IllegalArgumentException(String.format("Unsupported attribute '%s'", coRELinkParam));
+        }
+        this.value = ensureMatchingValue(model, value);
     }
 
     /**
diff --git a/leshan-core/src/main/java/org/eclipse/leshan/core/attributes/AttributeSet.java b/leshan-core/src/main/java/org/eclipse/leshan/core/attributes/AttributeSet.java
index 92e8315c..1eb41f9f 100644
--- a/leshan-core/src/main/java/org/eclipse/leshan/core/attributes/AttributeSet.java
+++ b/leshan-core/src/main/java/org/eclipse/leshan/core/attributes/AttributeSet.java
@@ -124,7 +124,11 @@ public class AttributeSet {
     public String[] toQueryParams() {
         List<String> queries = new LinkedList<>();
         for (Attribute attr : attributeMap.values()) {
-            queries.add(String.format("%s=%s", attr.getCoRELinkParam(), attr.getValue()));
+            if (attr.getValue() != null) {
+                queries.add(String.format("%s=%s", attr.getCoRELinkParam(), attr.getValue()));
+            } else {
+                queries.add(attr.getCoRELinkParam());
+            }
         }
         return queries.toArray(new String[queries.size()]);
     }
@@ -200,10 +204,14 @@ public class AttributeSet {
 
         for (String param : queryParams) {
             String[] keyAndValue = param.split("=");
-            if (keyAndValue.length != 2) {
+            if (keyAndValue.length == 1) {
+                attributes.add(new Attribute(keyAndValue[0]));
+            } else if (keyAndValue.length == 2) {
+                attributes.add(new Attribute(keyAndValue[0], keyAndValue[1]));
+            } else {
                 throw new IllegalArgumentException(String.format("Cannot parse query param '%s'", param));
             }
-            attributes.add(Attribute.create(keyAndValue[0], keyAndValue[1]));
+
         }
         return new AttributeSet(attributes);
     }


Test patch:
diff --git a/leshan-core/src/test/java/org/eclipse/leshan/core/attributes/AttributeSetTest.java b/leshan-core/src/test/java/org/eclipse/leshan/core/attributes/AttributeSetTest.java
index 9aac66df..f43d7bd6 100644
--- a/leshan-core/src/test/java/org/eclipse/leshan/core/attributes/AttributeSetTest.java
+++ b/leshan-core/src/test/java/org/eclipse/leshan/core/attributes/AttributeSetTest.java
@@ -29,99 +29,97 @@ public class AttributeSetTest {
 
     @Rule
     public ExpectedException exception = ExpectedException.none();
-    
+
     @Test
     public void should_provide_query_params() {
-        AttributeSet sut = new AttributeSet(
-                new Attribute(Attribute.OBJECT_VERSION, "1.1"),
-                new Attribute(Attribute.MINIMUM_PERIOD, 5L),
-                new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
+        AttributeSet sut = new AttributeSet(new Attribute(Attribute.OBJECT_VERSION, "1.1"),
+                new Attribute(Attribute.MINIMUM_PERIOD, 5L), new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
         assertEquals("ver=1.1&pmin=5&pmax=60", sut.toString());
+
+        AttributeSet res = AttributeSet.parse(sut.toString());
+        assertEquals(sut, res);
+    }
+
+    @Test
+    public void no_value_to_unset() {
+        AttributeSet sut = new AttributeSet(new Attribute(Attribute.MINIMUM_PERIOD),
+                new Attribute(Attribute.MAXIMUM_PERIOD));
+        assertEquals("pmin&pmax", sut.toString());
+
+        AttributeSet res = AttributeSet.parse(sut.toString());
+        assertEquals(sut, res);
     }
-    
+
     @Test
     public void should_get_map() {
-        AttributeSet sut = new AttributeSet(
-                new Attribute(Attribute.OBJECT_VERSION, "1.1"),
-                new Attribute(Attribute.MINIMUM_PERIOD, 5L),
-                new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
+        AttributeSet sut = new AttributeSet(new Attribute(Attribute.OBJECT_VERSION, "1.1"),
+                new Attribute(Attribute.MINIMUM_PERIOD, 5L), new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
         Map<String, Object> map = sut.getMap();
         assertEquals("1.1", map.get("ver"));
         assertEquals(5L, map.get("pmin"));
         assertEquals(60L, map.get("pmax"));
     }
-    
+
     @Test
     public void should_merge() {
-        AttributeSet sut = new AttributeSet(
-                new Attribute(Attribute.OBJECT_VERSION, "1.1"),
-                new Attribute(Attribute.MINIMUM_PERIOD, 5L),
-                new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
-        AttributeSet set2 = new AttributeSet(
-                new Attribute(Attribute.MINIMUM_PERIOD, 10L),
+        AttributeSet sut = new AttributeSet(new Attribute(Attribute.OBJECT_VERSION, "1.1"),
+                new Attribute(Attribute.MINIMUM_PERIOD, 5L), new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
+        AttributeSet set2 = new AttributeSet(new Attribute(Attribute.MINIMUM_PERIOD, 10L),
                 new Attribute(Attribute.MAXIMUM_PERIOD, 120L));
-        
+
         AttributeSet merged = sut.merge(set2);
-        
+
         Map<String, Object> map = merged.getMap();
         assertEquals("1.1", map.get("ver"));
         assertEquals(10L, map.get("pmin"));
         assertEquals(120L, map.get("pmax"));
-        
+
         // Assert that the original attribute sets are untouched
         map = sut.getMap();
         assertEquals(5L, map.get("pmin"));
         map = set2.getMap();
         assertEquals(null, map.get("ver"));
     }
-    
+
     @Test
     public void should_to_string() {
-        AttributeSet sut = new AttributeSet(
-                new Attribute(Attribute.OBJECT_VERSION, "1.1"),
-                new Attribute(Attribute.MINIMUM_PERIOD, 5L),
-                new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
-        
+        AttributeSet sut = new AttributeSet(new Attribute(Attribute.OBJECT_VERSION, "1.1"),
+                new Attribute(Attribute.MINIMUM_PERIOD, 5L), new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
+
         assertEquals("ver=1.1&pmin=5&pmax=60", sut.toString());
     }
-        
+
     @Test
     public void should_throw_on_duplicates() {
         exception.expect(IllegalArgumentException.class);
-        new AttributeSet(
-                new Attribute(Attribute.OBJECT_VERSION, "1.1"),
-                new Attribute(Attribute.MINIMUM_PERIOD, 5L),
+        new AttributeSet(new Attribute(Attribute.OBJECT_VERSION, "1.1"), new Attribute(Attribute.MINIMUM_PERIOD, 5L),
                 new Attribute(Attribute.MINIMUM_PERIOD, 60L));
     }
 
     @Test
     public void should_validate_assignation() {
-        AttributeSet sut = new AttributeSet(
-                new Attribute(Attribute.MINIMUM_PERIOD, 5L),
+        AttributeSet sut = new AttributeSet(new Attribute(Attribute.MINIMUM_PERIOD, 5L),
                 new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
         Collection<Attribute> attributes = sut.getAttributes();
         assertEquals(2, attributes.size());
         sut.validate(AssignationLevel.RESOURCE);
     }
-    
+
     @Test
     public void should_throw_on_invalid_assignation_level() {
-        AttributeSet sut = new AttributeSet(
-                new Attribute(Attribute.OBJECT_VERSION, "1.1"),
-                new Attribute(Attribute.MINIMUM_PERIOD, 5L),
-                new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
-        
+        AttributeSet sut = new AttributeSet(new Attribute(Attribute.OBJECT_VERSION, "1.1"),
+                new Attribute(Attribute.MINIMUM_PERIOD, 5L), new Attribute(Attribute.MAXIMUM_PERIOD, 60L));
+
         exception.expect(IllegalArgumentException.class);
         // OBJECT_VERSION cannot be assigned on resource level
         sut.validate(AssignationLevel.RESOURCE);
     }
-    
+
     @Test
     public void should_throw_on_invalid_pmin_pmax() {
-        AttributeSet sut = new AttributeSet(
-                new Attribute(Attribute.MINIMUM_PERIOD, 50L),
+        AttributeSet sut = new AttributeSet(new Attribute(Attribute.MINIMUM_PERIOD, 50L),
                 new Attribute(Attribute.MAXIMUM_PERIOD, 49L));
-        
+
         exception.expect(IllegalArgumentException.class);
         // pmin cannot be greater then pmax
         sut.validate(AssignationLevel.RESOURCE);
diff --git a/leshan-server-cf/src/test/java/org/eclipse/leshan/server/californium/request/CoapRequestBuilderTest.java b/leshan-server-cf/src/test/java/org/eclipse/leshan/server/californium/request/CoapRequestBuilderTest.java
index 4a314e74..01647e7b 100644
--- a/leshan-server-cf/src/test/java/org/eclipse/leshan/server/californium/request/CoapRequestBuilderTest.java
+++ b/leshan-server-cf/src/test/java/org/eclipse/leshan/server/californium/request/CoapRequestBuilderTest.java
@@ -210,6 +210,26 @@ public class CoapRequestBuilderTest {
         assertEquals("coap://127.0.0.1:12354/3/0/14?pmin=10&pmax=100", coapRequest.getURI());
     }
 
+    @Test
+    public void build_unset_write_attribute_request() throws Exception {
+        Registration reg = newRegistration();
+
+        // test
+        CoapRequestBuilder builder = new CoapRequestBuilder(reg.getIdentity(), reg.getRootPath(), reg.getId(),
+                reg.getEndpoint(), model, encoder, false);
+        AttributeSet attributes = new AttributeSet(new Attribute(Attribute.MINIMUM_PERIOD),
+                new Attribute(Attribute.MAXIMUM_PERIOD));
+        WriteAttributesRequest request = new WriteAttributesRequest(3, 0, 14, attributes);
+        builder.visit(request);
+
+        // verify
+        Request coapRequest = builder.getRequest();
+        assertEquals(CoAP.Code.PUT, coapRequest.getCode());
+        assertEquals("127.0.0.1", coapRequest.getDestinationContext().getPeerAddress().getAddress().getHostAddress());
+        assertEquals(12354, coapRequest.getDestinationContext().getPeerAddress().getPort());
+        assertEquals("coap://127.0.0.1:12354/3/0/14?pmin&pmax", coapRequest.getURI());
+    }
+
     @Test
     public void build_execute_request() throws Exception {
         Registration reg = newRegistration();