Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

read composite on root path #1520

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*******************************************************************************/
package org.eclipse.leshan.client.resource;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -28,10 +30,12 @@
import org.eclipse.leshan.core.model.ResourceModel;
import org.eclipse.leshan.core.node.LwM2mMultipleResource;
import org.eclipse.leshan.core.node.LwM2mNode;
import org.eclipse.leshan.core.node.LwM2mObject;
import org.eclipse.leshan.core.node.LwM2mObjectInstance;
import org.eclipse.leshan.core.node.LwM2mPath;
import org.eclipse.leshan.core.node.LwM2mResource;
import org.eclipse.leshan.core.node.LwM2mResourceInstance;
import org.eclipse.leshan.core.node.LwM2mRoot;
import org.eclipse.leshan.core.node.LwM2mSingleResource;
import org.eclipse.leshan.core.request.ObserveCompositeRequest;
import org.eclipse.leshan.core.request.ObserveRequest;
Expand Down Expand Up @@ -74,9 +78,22 @@ public void removeListener(ObjectsListener listener) {
@Override
public ReadCompositeResponse read(LwM2mServer server, ReadCompositeRequest request) {
List<LwM2mPath> paths = request.getPaths();
// TODO: add checks for failure and log it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I think some check should be done here.

The spec says :

The Read-Composite operation is treated as non-atomic and handled as best effort by the client. That is, if any of the requested resources do not have a valid value to return, they will not be included in the response.

(http://www.openmobilealliance.org/release/LightweightM2M/V1_1_1-20190617-A/HTML-Version/OMA-TS-LightweightM2M_Core-V1_1_1-20190617-A.html#6-3-8-0-638-Read-Composite-Operation)

So I think on failure, we should not stop the read-composite operation but just log as error.
(so error will not be seen at server side)

if (paths.size() == 1 && paths.get(0).isRoot()) {
// TODO implement read for "/" use case.
return ReadCompositeResponse.internalServerError("Not implemented yet");
Map<Integer, LwM2mObjectEnabler> objectEnablers = tree.getObjectEnablers();
List<LwM2mObject> lwM2mObjects = new ArrayList<>();

for (Map.Entry<Integer, LwM2mObjectEnabler> entry : objectEnablers.entrySet()) {
LwM2mObjectEnabler objectEnabler = entry.getValue();
ReadRequest readRequest = new ReadRequest(request.getRequestContentFormat(),
new LwM2mPath(entry.getKey()), request.getCoapRequest());
ReadResponse readResponse = objectEnabler.read(server, readRequest);
if (readResponse.isSuccess()) {
lwM2mObjects.add((LwM2mObject) readResponse.getContent());
}
}
LwM2mRoot lwM2mRoot = new LwM2mRoot(lwM2mObjects);
return ReadCompositeResponse.success(Collections.singletonMap(paths.get(0), lwM2mRoot));
}

// Read Nodes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,6 @@ public interface LwM2mNodeVisitor {

void visit(LwM2mResourceInstance instance);

default void visit(LwM2mRoot root) {
}
Comment on lines +31 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we should add a default visit.
Better to add to all visitor the new function and raise exception is not supported.

}
108 changes: 108 additions & 0 deletions leshan-core/src/main/java/org/eclipse/leshan/core/node/LwM2mRoot.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*******************************************************************************
* Copyright (c) 2013-2015 Sierra Wireless and others.
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* and Eclipse Distribution License v1.0 which accompany this distribution.
*
* The Eclipse Public License is available at
* http://www.eclipse.org/legal/epl-v20.html
* and the Eclipse Distribution License is available at
* http://www.eclipse.org/org/documents/edl-v10.html.
*
* Contributors:
* Orange Polska S.A. - initial API and implementation
*******************************************************************************/
package org.eclipse.leshan.core.node;

import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.TreeMap;

public class LwM2mRoot implements LwM2mNode {
private final Map<Integer, LwM2mObject> objects;

public LwM2mRoot(Collection<LwM2mObject> objects) {
LwM2mNodeUtil.validateNotNull(objects, "objects MUST NOT be null");
HashMap<Integer, LwM2mObject> objectsMap = new HashMap<>(objects.size());
for (LwM2mObject object : objects) {
LwM2mObject previous = objectsMap.put(object.getId(), object);
if (previous != null) {
throw new LwM2mNodeException(
"Unable to create LwM2mRoot : there are several objects with the same id %d", object.getId());
}
}
this.objects = Collections.unmodifiableMap(objectsMap);
}

@Override
public int getId() {
return 0;
}

@Override
public void accept(LwM2mNodeVisitor visitor) {
visitor.visit(this);
}

@Override
public String toPrettyString(LwM2mPath path) {
return appendPrettyNode(new StringBuilder(), path).toString();
}

@Override
public StringBuilder appendPrettyNode(StringBuilder b, LwM2mPath path) {
if (!path.isRoot())
throw new IllegalArgumentException("Path must be a root path");
if (path.getObjectId() != getId())
throw new IllegalArgumentException("Path id must match this LwM2mMultipleResource id");

if (objects.isEmpty()) {
b.append(path).append(" : {}");
} else {
boolean first = true;
for (Map.Entry<Integer, LwM2mObject> entry : new TreeMap<>(objects).entrySet()) {
if (first) {
first = false;
} else {
b.append("\n");
}
entry.getValue().appendPrettyNode(b, path.append(entry.getKey()));
}
}
return b;
}

public Map<Integer, LwM2mObject> getObjects() {
return objects;
}

@Override
public boolean equals(Object obj) {
if (this == obj)
return true;
if (obj == null)
return false;
if (getClass() != obj.getClass())
return false;
LwM2mRoot other = (LwM2mRoot) obj;
if (objects == null) {
if (other.objects != null)
return false;
} else if (!objects.equals(other.objects))
return false;
return true;
}

@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + getId();
result = prime * result + ((objects == null) ? 0 : objects.hashCode());
return result;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.eclipse.leshan.core.node.LwM2mPath;
import org.eclipse.leshan.core.node.LwM2mResource;
import org.eclipse.leshan.core.node.LwM2mResourceInstance;
import org.eclipse.leshan.core.node.LwM2mRoot;
import org.eclipse.leshan.core.node.TimestampedLwM2mNode;
import org.eclipse.leshan.core.node.TimestampedLwM2mNodes;
import org.eclipse.leshan.core.node.codec.cbor.LwM2mNodeCborDecoder;
Expand Down Expand Up @@ -264,6 +265,8 @@ public static Class<? extends LwM2mNode> nodeClassFromPath(LwM2mPath path) {
return LwM2mResource.class;
} else if (path.isResourceInstance()) {
return LwM2mResourceInstance.class;
} else if (path.isRoot()) {
return LwM2mRoot.class;
}
throw new IllegalArgumentException("invalid path level: " + path);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ private class InternalEncoder implements LwM2mNodeVisitor {

@Override
public void visit(LwM2mObject object) {
throw new CodecException("Object %s cannot be encoded in cbor format", path);
if (path.isRoot())
throw new CodecException("Root %s cannot be encoded in cbor format", path);
else
throw new CodecException("Object %s cannot be encoded in cbor format", path);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,12 @@ private class InternalEncoder implements LwM2mNodeVisitor {
public void visit(LwM2mObject object) {
LOG.trace("Encoding Object {} into JSON", object);
// Validate request path
if (!requestPath.isObject()) {
if (requestPath.isRoot()) {
throw new CodecException("Invalid request path %s for JSON root encoding", requestPath);
} else if (!requestPath.isObject()) {
throw new CodecException("Invalid request path %s for JSON object encoding", requestPath);
}

baseName = requestPath.toString() + "/";

// Create resources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ private static class InternalEncoder implements LwM2mNodeVisitor {

@Override
public void visit(LwM2mObject object) {
throw new CodecException("Object %s cannot be encoded in opaque format", path);
if (path.isRoot())
throw new CodecException("Root %s cannot be encoded in opaque forma", path);
else
throw new CodecException("Object %s cannot be encoded in opaque format", path);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.eclipse.leshan.core.node.LwM2mPath;
import org.eclipse.leshan.core.node.LwM2mResource;
import org.eclipse.leshan.core.node.LwM2mResourceInstance;
import org.eclipse.leshan.core.node.LwM2mRoot;
import org.eclipse.leshan.core.node.LwM2mSingleResource;
import org.eclipse.leshan.core.node.ObjectLink;
import org.eclipse.leshan.core.node.TimestampedLwM2mNode;
Expand Down Expand Up @@ -228,7 +229,22 @@ private LwM2mNode parseRecords(Collection<LwM2mResolvedSenMLRecord> records, LwM

// Create lwm2m node
LwM2mNode node = null;
if (nodeClass == LwM2mObject.class) {
if (nodeClass == LwM2mRoot.class) {
Collection<LwM2mObject> objects = new ArrayList<>();
Map<Integer, Collection<LwM2mResolvedSenMLRecord>> recordsByObjectId = groupRecordsByObjectId(records);
for (Entry<Integer, Collection<LwM2mResolvedSenMLRecord>> entryByObjectId : recordsByObjectId.entrySet()) {
Collection<LwM2mObjectInstance> instances = new ArrayList<>();
recordsByInstanceId = groupRecordsByInstanceId(entryByObjectId.getValue());
for (Entry<Integer, Collection<LwM2mResolvedSenMLRecord>> entryByInstanceId : recordsByInstanceId
.entrySet()) {
Map<Integer, LwM2mResource> resourcesMap = extractLwM2mResources(entryByInstanceId.getValue(), path,
model);
instances.add(new LwM2mObjectInstance(entryByInstanceId.getKey(), resourcesMap.values()));
}
objects.add(new LwM2mObject(entryByObjectId.getKey(), instances));
}
node = new LwM2mRoot(objects);
} else if (nodeClass == LwM2mObject.class) {
Collection<LwM2mObjectInstance> instances = new ArrayList<>();
for (Entry<Integer, Collection<LwM2mResolvedSenMLRecord>> entryByInstanceId : recordsByInstanceId
.entrySet()) {
Expand Down Expand Up @@ -431,6 +447,28 @@ private Map<Integer, Collection<LwM2mResolvedSenMLRecord>> groupRecordsByInstanc
return result;
}

/**
* Group all SenML record by objectId
*
* @return a map (objectId => collection of SenML Record)
*/
private Map<Integer, Collection<LwM2mResolvedSenMLRecord>> groupRecordsByObjectId(
Collection<LwM2mResolvedSenMLRecord> records) throws CodecException {
Map<Integer, Collection<LwM2mResolvedSenMLRecord>> result = new HashMap<>();
for (LwM2mResolvedSenMLRecord record : records) {
// Get SenML records for this object
Collection<LwM2mResolvedSenMLRecord> recordForObject = result.get(record.getPath().getObjectId());
if (recordForObject == null) {
recordForObject = new ArrayList<>();
result.put(record.getPath().getObjectId(), recordForObject);
}

// Add it to the list
recordForObject.add(record);
}
return result;
}

private Map<Integer, LwM2mResource> extractLwM2mResources(Collection<LwM2mResolvedSenMLRecord> records,
LwM2mPath requestPath, LwM2mModel model) throws CodecException {
if (records == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.eclipse.leshan.core.node.LwM2mPath;
import org.eclipse.leshan.core.node.LwM2mResource;
import org.eclipse.leshan.core.node.LwM2mResourceInstance;
import org.eclipse.leshan.core.node.LwM2mRoot;
import org.eclipse.leshan.core.node.ObjectLink;
import org.eclipse.leshan.core.node.TimestampedLwM2mNode;
import org.eclipse.leshan.core.node.TimestampedLwM2mNodes;
Expand Down Expand Up @@ -200,19 +201,41 @@ public byte[] encodeTimestampedNodes(TimestampedLwM2mNodes timestampedNodes, LwM

private class InternalEncoder implements LwM2mNodeVisitor {
// visitor inputs
private int objectId;
private Integer objectId;
private LwM2mModel model;
private LwM2mPath requestPath;
private LwM2mValueConverter converter;

// visitor output
private ArrayList<SenMLRecord> records = new ArrayList<>();

@Override
public void visit(LwM2mRoot root) {
LOG.trace("Encoding Root into SenML");
// Validate request path
if (!requestPath.isRoot()) {
throw new CodecException("Invalid request path %s for root encoding", requestPath);
}

// Create SenML records
for (LwM2mObject object : root.getObjects().values()) {
for (LwM2mObjectInstance instance : object.getInstances().values()) {
for (LwM2mResource resource : instance.getResources().values()) {
String prefixPath = object.getId() + "/" + instance.getId() + "/" + resource.getId();
this.objectId = object.getId();
lwM2mResourceToSenMLRecord(prefixPath, resource);
}
}
}
}

@Override
public void visit(LwM2mObject object) {
LOG.trace("Encoding Object {} into SenML", object);
// Validate request path
if (!requestPath.isObject()) {
if (requestPath.isRoot()) {
throw new CodecException("Invalid request path %s for root encoding", requestPath);
} else if (!requestPath.isObject()) {
throw new CodecException("Invalid request path %s for object encoding", requestPath);
}

Expand Down Expand Up @@ -302,7 +325,7 @@ private void addSenMLRecord(String recordName, Type valueType, Type expectedType
String n = recordName == null ? "" : recordName;

// Add slash if necessary
if (!n.isEmpty()) {
if (!n.isEmpty() && !bn.equals("/")) {
bn += "/";
Comment on lines +328 to 329
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds strange I need to look at this deeper.

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ private class InternalEncoder implements LwM2mNodeVisitor {

@Override
public void visit(LwM2mObject object) {
throw new CodecException("Object %s cannot be encoded in text format", path);
if (path.isRoot())
throw new CodecException("Root %s cannot be encoded in text format", path);
else
throw new CodecException("Object %s cannot be encoded in text format", path);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ private class InternalEncoder implements LwM2mNodeVisitor {
public void visit(LwM2mObject object) {
LOG.trace("Encoding object {} into TLV", object);

if (path.isRoot())
throw new CodecException("Path is root. Invalid request path %s for JSON instance encoding", path);

Tlv[] tlvs;

// encoded as an array of instances
Expand Down
Loading