Repository: eclipse/leshan
Pull request #1009 (2021-05-10 14:30:24)

Remove lazy loading for getSupportedObjects in Registration.
By the past `getSupportedObjects` was added in "Lazy loading" way.

But there is now more and more data to extract from LinkObject : 
 - root path
 - supported objects
 - supported content format
 - (To be done : available instances)
 
 It sounds better parse ObjectLink and extract data only when needed, then use classic serialize/deserialize for those data. (and so for supported objects too) 


Patch:
diff --git a/leshan-core/src/main/java/org/eclipse/leshan/core/node/LwM2mPath.java b/leshan-core/src/main/java/org/eclipse/leshan/core/node/LwM2mPath.java
index 4786af19..0173311d 100644
--- a/leshan-core/src/main/java/org/eclipse/leshan/core/node/LwM2mPath.java
+++ b/leshan-core/src/main/java/org/eclipse/leshan/core/node/LwM2mPath.java
@@ -368,4 +368,29 @@ public class LwM2mPath {
         return true;
     }
 
+    /**
+     * Parse a string containing a full LWM2M path containing rootpath (rt="oma.lwm2m").
+     * <p>
+     * E.g. : fullpath="/myrootpath/1/0 and rootpath="/myrootpat/" will return <code>new LwM2mPath(1,0)</code>
+     * 
+     * @param fullpath the path to parse.
+     * @param lwm2mRootpath the expected rootpath. <code>null</code> is considered as "/"
+     * @return A valid {@link LwM2mPath} or null it does not start by lwm2mRootPath
+     * 
+     * @exception NumberFormatException if path contains not Numeric value
+     * @exception LwM2mNodeException if path is invalid (e.g. too big number in path)
+     * @exception IllegalArgumentException if path length is invalid
+     */
+    public static LwM2mPath parse(String fullpath, String lwm2mRootpath)
+            throws NumberFormatException, LwM2mNodeException, IllegalArgumentException {
+        if (lwm2mRootpath == null) {
+            return new LwM2mPath(fullpath);
+        }
+
+        if (!fullpath.startsWith(lwm2mRootpath))
+            return null;
+        String path = fullpath.substring(lwm2mRootpath.length());
+
+        return new LwM2mPath(path);
+    }
 }
\ No newline at end of file
diff --git a/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/Registration.java b/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/Registration.java
index d9c2e63f..23dbeb64 100644
--- a/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/Registration.java
+++ b/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/Registration.java
@@ -27,14 +27,12 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 import org.eclipse.leshan.core.Link;
 import org.eclipse.leshan.core.LwM2m.Version;
 import org.eclipse.leshan.core.attributes.Attribute;
 import org.eclipse.leshan.core.model.ObjectModel;
+import org.eclipse.leshan.core.node.LwM2mPath;
 import org.eclipse.leshan.core.request.BindingMode;
 import org.eclipse.leshan.core.request.ContentFormat;
 import org.eclipse.leshan.core.request.Identity;
@@ -73,10 +71,6 @@ public class Registration {
 
     private final Link[] objectLinks;
 
-    // Lazy Loaded map of supported object (object id => version)
-    // built from objectLinks
-    private final AtomicReference<Map<Integer, String>> supportedObjects;
-
     private final Map<String, String> additionalRegistrationAttributes;
 
     // The location where LWM2M objects are hosted on the device
@@ -85,6 +79,9 @@ public class Registration {
     // All ContentFormat supported by the client
     private final Set<ContentFormat> supportedContentFormats;
 
+    // All supported object (object id => version)
+    private final Map<Integer, String> supportedObjects;
+
     private final Date lastUpdate;
 
     protected Registration(Builder builder) {
@@ -102,7 +99,7 @@ public class Registration {
         objectLinks = builder.objectLinks;
         rootPath = builder.rootPath;
         supportedContentFormats = builder.supportedContentFormats;
-        supportedObjects = new AtomicReference<Map<Integer, String>>(builder.supportedObjects);
+        supportedObjects = builder.supportedObjects;
 
         // other params
         lifeTimeInSec = builder.lifeTimeInSec;
@@ -320,12 +317,7 @@ public class Registration {
      * @return a map from {@code objectId} {@literal =>} {@code supportedVersion} for each supported objects. supported.
      */
     public Map<Integer, String> getSupportedObject() {
-        Map<Integer, String> objects = supportedObjects.get();
-        if (objects != null)
-            return objects;
-
-        supportedObjects.compareAndSet(null, Collections.unmodifiableMap(getSupportedObject(rootPath, objectLinks)));
-        return supportedObjects.get();
+        return supportedObjects;
     }
 
     @Override
@@ -356,6 +348,7 @@ public class Registration {
         result = prime * result + ((rootPath == null) ? 0 : rootPath.hashCode());
         result = prime * result + ((smsNumber == null) ? 0 : smsNumber.hashCode());
         result = prime * result + ((supportedContentFormats == null) ? 0 : supportedContentFormats.hashCode());
+        result = prime * result + ((supportedObjects == null) ? 0 : supportedObjects.hashCode());
         return result;
     }
 
@@ -432,55 +425,14 @@ public class Registration {
                 return false;
         } else if (!supportedContentFormats.equals(other.supportedContentFormats))
             return false;
+        if (supportedObjects == null) {
+            if (other.supportedObjects != null)
+                return false;
+        } else if (!supportedObjects.equals(other.supportedObjects))
+            return false;
         return true;
     }
 
-    /**
-     * Build a Map {@code objectId} {@literal =>} {@code supportedVersion} from root path and registration object links.
-     * 
-     * @param rootPath the rootpath of LWM2M tree.
-     * @param objectLinks the registraiton object links payload.
-     * @return a Map {@code objectId} {@literal =>} {@code supportedVersion}.
-     */
-    public static Map<Integer, String> getSupportedObject(String rootPath, Link[] objectLinks) {
-        Map<Integer, String> objects = new HashMap<>();
-        for (Link link : objectLinks) {
-            if (link != null) {
-                Pattern p = Pattern.compile("^\\Q" + rootPath + "\\E(\\d+)(?:/\\d+)*$");
-                Matcher m = p.matcher(link.getUrl());
-                if (m.matches()) {
-                    try {
-                        // extract object id and version
-                        int objectId = Integer.parseInt(m.group(1));
-                        String version = link.getAttributes().get(Attribute.OBJECT_VERSION);
-                        // un-quote version (see https://github.com/eclipse/leshan/issues/732)
-                        version = Link.unquote(version);
-                        String currentVersion = objects.get(objectId);
-
-                        // store it in map
-                        if (currentVersion == null) {
-                            // we never find version for this object add it
-                            if (version != null) {
-                                objects.put(objectId, version);
-                            } else {
-                                objects.put(objectId, ObjectModel.DEFAULT_VERSION);
-                            }
-                        } else {
-                            // if version is already set, we override it only if new version is not DEFAULT_VERSION
-                            if (version != null && !version.equals(ObjectModel.DEFAULT_VERSION)) {
-                                objects.put(objectId, version);
-                            }
-                        }
-                    } catch (NumberFormatException e) {
-                        // This should not happened except maybe if the number in url is too long...
-                        // In this case we just ignore it because this is not an object id.
-                    }
-                }
-            }
-        }
-        return objects;
-    }
-
     public static class Builder {
         private final String registrationId;
         private final String endpoint;
@@ -575,6 +527,11 @@ public class Registration {
             return this;
         }
 
+        public Builder supportedObjects(Map<Integer, String> supportedObjects) {
+            this.supportedObjects = supportedObjects;
+            return this;
+        }
+
         public Builder additionalRegistrationAttributes(Map<String, String> additionalRegistrationAttributes) {
             this.additionalRegistrationAttributes = additionalRegistrationAttributes;
             return this;
@@ -597,8 +554,8 @@ public class Registration {
                 }
 
                 // Extract data from link object
+                supportedObjects = new HashMap<>();
                 for (Link link : objectLinks) {
-                    // TODO extract object supported
                     // TODO extract available instances
 
                     if (link != null) {
@@ -608,6 +565,14 @@ public class Registration {
                             if (ctValue != null) {
                                 supportedContentFormats = extractContentFormat(ctValue);
                             }
+                        } else {
+                            LwM2mPath path = LwM2mPath.parse(link.getUrl(), rootPath);
+                            if (path != null) {
+                                // add supported objects
+                                if (path.isObject() || path.isObjectInstance()) {
+                                    addSupportedObject(link, path);
+                                }
+                            }
                         }
                     }
                 }
@@ -643,6 +608,30 @@ public class Registration {
             return supportedContentFormats;
         }
 
+        private void addSupportedObject(Link link, LwM2mPath path) {
+            // extract object id and version
+            int objectId = path.getObjectId();
+            String version = link.getAttributes().get(Attribute.OBJECT_VERSION);
+            // un-quote version (see https://github.com/eclipse/leshan/issues/732)
+            version = Link.unquote(version);
+            String currentVersion = supportedObjects.get(objectId);
+
+            // store it in map
+            if (currentVersion == null) {
+                // we never find version for this object add it
+                if (version != null) {
+                    supportedObjects.put(objectId, version);
+                } else {
+                    supportedObjects.put(objectId, ObjectModel.DEFAULT_VERSION);
+                }
+            } else {
+                // if version is already set, we override it only if new version is not DEFAULT_VERSION
+                if (version != null && !version.equals(ObjectModel.DEFAULT_VERSION)) {
+                    supportedObjects.put(objectId, version);
+                }
+            }
+        }
+
         public Registration build() {
             // Define Default value
             rootPath = rootPath == null ? "/" : rootPath;
@@ -665,6 +654,11 @@ public class Registration {
             } else {
                 supportedContentFormats = Collections.unmodifiableSet(new HashSet<>(supportedContentFormats));
             }
+            if (supportedObjects == null || supportedObjects.isEmpty()) {
+                supportedObjects = Collections.emptyMap();
+            } else {
+                supportedObjects = Collections.unmodifiableMap(new HashMap<>(supportedObjects));
+            }
             if (additionalRegistrationAttributes == null || additionalRegistrationAttributes.isEmpty()) {
                 additionalRegistrationAttributes = Collections.emptyMap();
             } else {
diff --git a/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/RegistrationUpdate.java b/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/RegistrationUpdate.java
index f41a0358..6a7208ee 100644
--- a/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/RegistrationUpdate.java
+++ b/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/RegistrationUpdate.java
@@ -88,7 +88,8 @@ public class RegistrationUpdate {
                 .bindingMode(bindingMode).queueMode(registration.getQueueMode()).objectLinks(linkObject)
                 .registrationDate(registration.getRegistrationDate()).lastUpdate(lastUpdate)
                 .additionalRegistrationAttributes(additionalAttributes).rootPath(registration.getRootPath())
-                .supportedContentFormats(registration.getSupportedContentFormats());
+                .supportedContentFormats(registration.getSupportedContentFormats())
+                .supportedObjects(registration.getSupportedObject());
 
         return builder.build();
 
diff --git a/leshan-server-redis/src/main/java/org/eclipse/leshan/server/redis/serialization/RegistrationSerDes.java b/leshan-server-redis/src/main/java/org/eclipse/leshan/server/redis/serialization/RegistrationSerDes.java
index 8bcd07e8..3dd634ab 100644
--- a/leshan-server-redis/src/main/java/org/eclipse/leshan/server/redis/serialization/RegistrationSerDes.java
+++ b/leshan-server-redis/src/main/java/org/eclipse/leshan/server/redis/serialization/RegistrationSerDes.java
@@ -19,6 +19,7 @@ import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 
 import org.eclipse.leshan.core.Link;
@@ -30,6 +31,7 @@ import org.eclipse.leshan.server.registration.Registration;
 import com.eclipsesource.json.Json;
 import com.eclipsesource.json.JsonArray;
 import com.eclipsesource.json.JsonObject;
+import com.eclipsesource.json.JsonObject.Member;
 import com.eclipsesource.json.JsonValue;
 
 /**
@@ -83,6 +85,13 @@ public class RegistrationSerDes {
             ct.add(contentFormat.getCode());
         }
         o.add("ct", ct);
+
+        // handle supported object
+        JsonObject so = Json.object();
+        for (Entry<Integer, String> supportedObject : r.getSupportedObject().entrySet()) {
+            so.add(supportedObject.getKey().toString(), supportedObject.getValue());
+        }
+        o.add("suppObjs", so);
         return o;
     }
 
@@ -156,6 +165,19 @@ public class RegistrationSerDes {
             }
             b.supportedContentFormats(supportedContentFormat);
         }
+        // parse supported object
+        JsonValue so = jObj.get("suppObjs");
+        if (so == null) {
+            // Backward compatibility : if suppObjs doesn't exist we extract supported object from object link
+            b.extractDataFromObjectLink(true);
+        } else {
+            Map<Integer, String> supportedObject = new HashMap<>();
+            for (Member member : so.asObject()) {
+                supportedObject.put(Integer.parseInt(member.getName()), member.getValue().asString());
+            }
+            b.supportedObjects(supportedObject);
+        }
+
         return b.build();
     }
 


Test patch:
diff --git a/leshan-server-core/src/test/java/org/eclipse/leshan/server/registration/RegistrationTest.java b/leshan-server-core/src/test/java/org/eclipse/leshan/server/registration/RegistrationTest.java
index 2c6edee4..c66d2514 100644
--- a/leshan-server-core/src/test/java/org/eclipse/leshan/server/registration/RegistrationTest.java
+++ b/leshan-server-core/src/test/java/org/eclipse/leshan/server/registration/RegistrationTest.java
@@ -157,18 +157,10 @@ public class RegistrationTest {
         assertEquals("1.1", supportedObject.get(3));
     }
 
-    @Test
+    @Test(expected = IllegalArgumentException.class)
     public void test_object_links_with_text_in_lwm2m_path() {
-        Registration reg = given_a_registration_with_object_link_like(
+        given_a_registration_with_object_link_like(
                 "<text>,</1/text/0/in/path>,empty,</2/O/test/in/path>,</3/0>;ver=\"1.1\",</4/0/0/>");
-
-        // check root path
-        assertEquals("/", reg.getRootPath());
-
-        // Ensure supported objects are correct
-        Map<Integer, String> supportedObject = reg.getSupportedObject();
-        assertEquals(1, supportedObject.size());
-        assertEquals("1.1", supportedObject.get(3));
     }
 
     private Registration given_a_registration_with_object_link_like(String objectLinks) {