Fixed bug with multiple DataReaders participating in one read operation 07/3107/2
authorTony Tkacik <ttkacik@cisco.com>
Tue, 26 Nov 2013 18:01:08 +0000 (19:01 +0100)
committerTony Tkacik <ttkacik@cisco.com>
Wed, 27 Nov 2013 09:12:01 +0000 (10:12 +0100)
Change-Id: I562c070acb0b16b1ffe41abb014c22319e1b5f2a
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/RuntimeGeneratedMappingServiceImpl.xtend
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/BindingIndependentDataServiceConnector.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/BindingIndependentMappingService.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DeserializationException.java [new file with mode: 0644]
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/impl/DataReaderRouter.xtend

index 0ddc2c88c8bd5bc99c85260bb13f7ce1c86f592b..ec69fd3b68c8ee84c39edde4e58207adb771b5bd 100644 (file)
@@ -33,6 +33,8 @@ import org.opendaylight.yangtools.binding.generator.util.Types
 import org.osgi.framework.BundleContext
 import java.util.Hashtable
 import org.osgi.framework.ServiceRegistration
+import org.opendaylight.controller.sal.binding.impl.connect.dom.DeserializationException
+import java.util.concurrent.Callable
 
 class RuntimeGeneratedMappingServiceImpl implements BindingIndependentMappingService, SchemaServiceListener, AutoCloseable {
 
@@ -59,7 +61,7 @@ class RuntimeGeneratedMappingServiceImpl implements BindingIndependentMappingSer
     val promisedTypeDefinitions = HashMultimap.<Type, SettableFuture<GeneratedTypeBuilder>>create;
 
     val promisedSchemas = HashMultimap.<Type, SettableFuture<SchemaNode>>create;
-    
+
     ServiceRegistration<SchemaServiceListener> listenerRegistration
 
     override onGlobalContextUpdated(SchemaContext arg0) {
@@ -79,7 +81,6 @@ class RuntimeGeneratedMappingServiceImpl implements BindingIndependentMappingSer
             val context = entry.value;
             updateBindingFor(context.childNodes, schemaContext);
             updateBindingFor(context.cases, schemaContext);
-            
 
             val typedefs = context.typedefs;
             for (typedef : typedefs.values) {
@@ -89,7 +90,7 @@ class RuntimeGeneratedMappingServiceImpl implements BindingIndependentMappingSer
             for (augmentation : augmentations) {
                 binding.typeToDefinition.put(augmentation, augmentation);
             }
-            
+
             binding.typeToAugmentation.putAll(context.typeToAugmentation);
         }
     }
@@ -127,22 +128,36 @@ class RuntimeGeneratedMappingServiceImpl implements BindingIndependentMappingSer
     }
 
     override dataObjectFromDataDom(InstanceIdentifier<? extends DataObject> path, CompositeNode node) {
-        if (node == null) {
-            return null;
-        }
-        val targetType = path.targetType
-        val transformer = registry.getCodecForDataObject(targetType);
-        val ret = transformer.deserialize(node)?.value as DataObject;
-        return ret;
+        return tryDeserialization[ |
+            if (node == null) {
+                return null;
+            }
+            val targetType = path.targetType
+            val transformer = registry.getCodecForDataObject(targetType);
+            val ret = transformer.deserialize(node)?.value as DataObject;
+            return ret;
+        ]
     }
-    
+
     override fromDataDom(org.opendaylight.yangtools.yang.data.api.InstanceIdentifier entry) {
-        return registry.instanceIdentifierCodec.deserialize(entry);
+        return tryDeserialization[ |
+            registry.instanceIdentifierCodec.deserialize(entry);
+        ]
+    }
+
+    private static def <T> T tryDeserialization(Callable<T> deserializationBlock) throws DeserializationException {
+        try {
+            deserializationBlock.call()
+        } catch (Exception e) {
+            // FIXME: Make this block providing more information.
+            throw new DeserializationException(e);
+        }
     }
 
     private def void updateBindingFor(Map<SchemaPath, GeneratedTypeBuilder> map, SchemaContext module) {
         for (entry : map.entrySet) {
             val schemaNode = SchemaContextUtil.findDataSchemaNode(module, entry.key);
+
             //LOG.info("{} : {}",entry.key,entry.value.fullyQualifiedName)
             if (schemaNode != null) {
                 typeToSchemaNode.put(entry.value, schemaNode);
@@ -162,8 +177,8 @@ class RuntimeGeneratedMappingServiceImpl implements BindingIndependentMappingSer
         binding.typeToDefinition = typeToDefinition
         binding.typeToSchemaNode = typeToSchemaNode
         binding.typeDefinitions = typeDefinitions
-        if(ctx !== null) {
-            listenerRegistration = ctx.registerService(SchemaServiceListener,this,new Hashtable<String,String>());
+        if (ctx !== null) {
+            listenerRegistration = ctx.registerService(SchemaServiceListener, this, new Hashtable<String, String>());
         }
     }
 
@@ -217,9 +232,9 @@ class RuntimeGeneratedMappingServiceImpl implements BindingIndependentMappingSer
         }
         promisedSchemas.removeAll(builder);
     }
-    
+
     override close() throws Exception {
         listenerRegistration?.unregister();
     }
-    
+
 }
index ccd6079cc997b00b9de8a2a6f5ee935155f01340..7a72afc88550b3871ea72286f5b791b4a90f568c 100644 (file)
@@ -32,7 +32,7 @@ import org.slf4j.LoggerFactory;
 
 public class BindingIndependentDataServiceConnector implements //
         RuntimeDataProvider, //
-        Provider {
+        Provider, AutoCloseable {
 
     private final Logger LOG = LoggerFactory.getLogger(BindingIndependentDataServiceConnector.class);
 
@@ -59,16 +59,24 @@ public class BindingIndependentDataServiceConnector implements //
 
     @Override
     public DataObject readOperationalData(InstanceIdentifier<? extends DataObject> path) {
-        org.opendaylight.yangtools.yang.data.api.InstanceIdentifier biPath = mappingService.toDataDom(path);
-        CompositeNode result = biDataService.readOperationalData(biPath);
-        return mappingService.dataObjectFromDataDom(path, result);
+        try {
+            org.opendaylight.yangtools.yang.data.api.InstanceIdentifier biPath = mappingService.toDataDom(path);
+            CompositeNode result = biDataService.readOperationalData(biPath);
+            return mappingService.dataObjectFromDataDom(path, result);
+        } catch (DeserializationException e) {
+            throw new IllegalStateException(e);
+        }
     }
 
     @Override
     public DataObject readConfigurationData(InstanceIdentifier<? extends DataObject> path) {
-        org.opendaylight.yangtools.yang.data.api.InstanceIdentifier biPath = mappingService.toDataDom(path);
-        CompositeNode result = biDataService.readConfigurationData(biPath);
-        return mappingService.dataObjectFromDataDom(path, result);
+        try {
+            org.opendaylight.yangtools.yang.data.api.InstanceIdentifier biPath = mappingService.toDataDom(path);
+            CompositeNode result = biDataService.readConfigurationData(biPath);
+            return mappingService.dataObjectFromDataDom(path, result);
+        } catch (DeserializationException e) {
+            throw new IllegalStateException(e);
+        }
     }
 
     private DataModificationTransaction createBindingToDomTransaction(
@@ -103,23 +111,42 @@ public class BindingIndependentDataServiceConnector implements //
                 .beginTransaction();
         for (Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, CompositeNode> entry : source
                 .getUpdatedConfigurationData().entrySet()) {
-            InstanceIdentifier<?> baKey = mappingService.fromDataDom(entry.getKey());
-            DataObject baData = mappingService.dataObjectFromDataDom(baKey, entry.getValue());
-            target.putConfigurationData(baKey, baData);
+            try {
+                InstanceIdentifier<?> baKey = mappingService.fromDataDom(entry.getKey());
+                DataObject baData = mappingService.dataObjectFromDataDom(baKey, entry.getValue());
+                target.putConfigurationData(baKey, baData);
+            } catch (DeserializationException e) {
+                LOG.error("Ommiting from BA transaction: {}. Reason{}.", entry.getKey(), e);
+            }
         }
         for (Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, CompositeNode> entry : source
                 .getUpdatedOperationalData().entrySet()) {
-            InstanceIdentifier<?> baKey = mappingService.fromDataDom(entry.getKey());
-            DataObject baData = mappingService.dataObjectFromDataDom(baKey, entry.getValue());
-            target.putOperationalData(baKey, baData);
+            try {
+
+                InstanceIdentifier<?> baKey = mappingService.fromDataDom(entry.getKey());
+                DataObject baData = mappingService.dataObjectFromDataDom(baKey, entry.getValue());
+                target.putOperationalData(baKey, baData);
+            } catch (DeserializationException e) {
+                LOG.error("Ommiting from BA transaction: {}. Reason{}.", entry.getKey(), e);
+            }
         }
         for (org.opendaylight.yangtools.yang.data.api.InstanceIdentifier entry : source.getRemovedConfigurationData()) {
-            InstanceIdentifier<?> baEntry = mappingService.fromDataDom(entry);
-            target.removeConfigurationData(baEntry);
+            try {
+
+                InstanceIdentifier<?> baEntry = mappingService.fromDataDom(entry);
+                target.removeConfigurationData(baEntry);
+            } catch (DeserializationException e) {
+                LOG.error("Ommiting from BA transaction: {}. Reason{}.", entry, e);
+            }
         }
         for (org.opendaylight.yangtools.yang.data.api.InstanceIdentifier entry : source.getRemovedOperationalData()) {
-            InstanceIdentifier<?> baEntry = mappingService.fromDataDom(entry);
-            target.removeOperationalData(baEntry);
+            try {
+
+                InstanceIdentifier<?> baEntry = mappingService.fromDataDom(entry);
+                target.removeOperationalData(baEntry);
+            } catch (DeserializationException e) {
+                LOG.error("Ommiting from BA transaction: {}. Reason{}.", entry, e);
+            }
         }
         return target;
     }
@@ -162,6 +189,18 @@ public class BindingIndependentDataServiceConnector implements //
         start();
     }
 
+    @Override
+    public void close() throws Exception {
+
+        if (baCommitHandlerRegistration != null) {
+            baCommitHandlerRegistration.close();
+        }
+        if (biCommitHandlerRegistration != null) {
+            biCommitHandlerRegistration.close();
+        }
+
+    }
+
     private class DomToBindingTransaction implements
             DataCommitTransaction<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, CompositeNode> {
 
@@ -271,10 +310,11 @@ public class BindingIndependentDataServiceConnector implements //
 
         @Override
         public void onRegister(DataCommitHandlerRegistration<InstanceIdentifier<?>, DataObject> registration) {
-            
-            org.opendaylight.yangtools.yang.data.api.InstanceIdentifier domPath = mappingService.toDataDom(registration.getPath());
+
+            org.opendaylight.yangtools.yang.data.api.InstanceIdentifier domPath = mappingService.toDataDom(registration
+                    .getPath());
             // FIXME: do registration based on only active commit handlers.
-            
+
         }
 
         @Override
index 9e175b8cb0e0f6c5e79550218986e0f552ae3edd..b1983fe224d89da96f4445efa86469cc0e0e6b9d 100644 (file)
@@ -16,8 +16,8 @@ public interface BindingIndependentMappingService {
 
     org.opendaylight.yangtools.yang.data.api.InstanceIdentifier toDataDom(InstanceIdentifier<? extends DataObject> path);
 
-    DataObject dataObjectFromDataDom(InstanceIdentifier<? extends DataObject> path, CompositeNode result);
+    DataObject dataObjectFromDataDom(InstanceIdentifier<? extends DataObject> path, CompositeNode result) throws DeserializationException;
 
-    InstanceIdentifier<?> fromDataDom(org.opendaylight.yangtools.yang.data.api.InstanceIdentifier entry);
+    InstanceIdentifier<?> fromDataDom(org.opendaylight.yangtools.yang.data.api.InstanceIdentifier entry)  throws DeserializationException;
 
 }
diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DeserializationException.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DeserializationException.java
new file mode 100644 (file)
index 0000000..9331899
--- /dev/null
@@ -0,0 +1,24 @@
+package org.opendaylight.controller.sal.binding.impl.connect.dom;
+
+public class DeserializationException extends Exception {
+
+    public DeserializationException() {
+    }
+
+    public DeserializationException(String message) {
+        super(message);
+    }
+
+    public DeserializationException(Throwable cause) {
+        super(cause);
+    }
+
+    public DeserializationException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    public DeserializationException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) {
+        super(message, cause, enableSuppression, writableStackTrace);
+    }
+
+}
index fbed2ca113621c728dd00a5fa4b9ae932344d7db..504a3d639413f7f6b1b901297bbf215aa9825842 100644 (file)
@@ -4,15 +4,100 @@ import org.opendaylight.controller.md.sal.common.impl.routing.AbstractDataReadRo
 import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier
 import org.opendaylight.yangtools.yang.data.api.CompositeNode
 import org.opendaylight.controller.md.sal.common.api.data.DataReader
+import org.opendaylight.yangtools.yang.common.QName
+import java.net.URI
+import java.util.List
+import org.opendaylight.yangtools.yang.data.api.Node
+import java.util.ArrayList
+import org.opendaylight.yangtools.yang.data.impl.SimpleNodeTOImpl
+import java.util.Map
+import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.PathArgument
+import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.NodeIdentifier
+import org.opendaylight.yangtools.yang.data.api.SimpleNode
+import java.util.Collections
+import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.NodeIdentifierWithPredicates
+import java.util.HashMap
+import static com.google.common.base.Preconditions.*;
+import java.util.Collection
+import java.util.Set
+import java.util.Map.Entry
+import org.slf4j.LoggerFactory
+import org.opendaylight.yangtools.yang.data.impl.CompositeNodeTOImpl
 
 class DataReaderRouter extends AbstractDataReadRouter<InstanceIdentifier, CompositeNode> {
+    private static val LOG = LoggerFactory.getLogger(DataReaderRouter);
+    private static val NETCONF_NAMESPACE = URI.create("urn:ietf:params:xml:ns:netconf:base:1.0")
+    private static val NETCONF_DATA = new QName(NETCONF_NAMESPACE,"data");
 
     override protected merge(InstanceIdentifier path, Iterable<CompositeNode> data) {
+        val pathArgument = path.path.last;
+        var empty = true;
+        var name = pathArgument?.nodeType;
+        val nodes = new ArrayList<Node<?>>();
+        val keyNodes = new HashMap<QName, SimpleNode<?>>();
         val iterator = data.iterator;
-        if(iterator.hasNext) {
-            return data.iterator.next
+        for(dataBit : data) {
+            try {
+                if(pathArgument != null && dataBit != null) {
+                    empty = false;
+                    val keyNodesLocal = getKeyNodes(pathArgument,dataBit);
+                    nodes.addAll(dataBit.childrenWithout(keyNodesLocal.entrySet));
+                } else if (dataBit != null) {
+                    empty = false;
+                    nodes.addAll(dataBit.children)
+                }
+            }   catch (IllegalStateException e) {
+                LOG.error("BUG: Readed data for path {} was invalid",path,e);
+            }
         }
-        return null;
+        if(empty) {
+            return null;
+        }
+        /**
+         * Reading from Root
+         * 
+         */
+        if(pathArgument == null) {
+            return new CompositeNodeTOImpl(NETCONF_DATA,null,nodes);
+        }
+        val finalNodes = new ArrayList<Node<?>>();
+        finalNodes.addAll(keyNodes.values);
+        finalNodes.addAll(nodes);
+        return new CompositeNodeTOImpl(name,null,finalNodes);
     }
-
+    
+    
+    
+    dispatch def Map<QName, SimpleNode<?>> getKeyNodes(PathArgument argument, CompositeNode node) {
+        return Collections.emptyMap();
+    }
+    
+    dispatch def getKeyNodes(NodeIdentifierWithPredicates argument, CompositeNode node) {
+        val ret = new HashMap<QName, SimpleNode<?>>();
+        for (keyValue : argument.keyValues.entrySet) {
+            val simpleNode = node.getSimpleNodesByName(keyValue.key);
+            if(simpleNode !== null && !simpleNode.empty) {
+                checkState(simpleNode.size <= 1,"Only one simple node for key $s is allowed in node $s",keyValue.key,node);
+                checkState(simpleNode.get(0).value == keyValue.value,"Key node must equals to instance identifier value");
+                ret.put(keyValue.key,simpleNode.get(0));
+            }
+            val compositeNode = node.getCompositesByName(keyValue.key);
+            checkState(compositeNode === null || compositeNode.empty,"Key node must be Simple Node, not composite node.");
+        }
+        return ret;
+    }
+    
+    def Collection<? extends Node<?>> childrenWithout(CompositeNode node, Set<Entry<QName, SimpleNode<?>>> entries) {
+        if(entries.empty) {
+            return node.children;
+        }
+        val filteredNodes = new ArrayList<Node<?>>();
+        for(scannedNode : node.children) {
+            if(!entries.contains(scannedNode.nodeType)) {
+                filteredNodes.add(scannedNode);
+            }
+        }
+        return filteredNodes;
+    }
+    
 }