From 9455ea18680bf9476099ed27157b84d569e33486 Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Tue, 26 Nov 2013 19:01:08 +0100 Subject: [PATCH] Fixed bug with multiple DataReaders participating in one read operation Change-Id: I562c070acb0b16b1ffe41abb014c22319e1b5f2a Signed-off-by: Tony Tkacik --- .../RuntimeGeneratedMappingServiceImpl.xtend | 47 ++++++---- ...indingIndependentDataServiceConnector.java | 80 ++++++++++++---- .../dom/BindingIndependentMappingService.java | 4 +- .../connect/dom/DeserializationException.java | 24 +++++ .../dom/broker/impl/DataReaderRouter.xtend | 93 ++++++++++++++++++- 5 files changed, 206 insertions(+), 42 deletions(-) create mode 100644 opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DeserializationException.java diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/RuntimeGeneratedMappingServiceImpl.xtend b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/RuntimeGeneratedMappingServiceImpl.xtend index 0ddc2c88c8..ec69fd3b68 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/RuntimeGeneratedMappingServiceImpl.xtend +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/RuntimeGeneratedMappingServiceImpl.xtend @@ -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.>create; val promisedSchemas = HashMultimap.>create; - + ServiceRegistration 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 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 tryDeserialization(Callable 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 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()); + if (ctx !== null) { + listenerRegistration = ctx.registerService(SchemaServiceListener, this, new Hashtable()); } } @@ -217,9 +232,9 @@ class RuntimeGeneratedMappingServiceImpl implements BindingIndependentMappingSer } promisedSchemas.removeAll(builder); } - + override close() throws Exception { listenerRegistration?.unregister(); } - + } diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/BindingIndependentDataServiceConnector.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/BindingIndependentDataServiceConnector.java index ccd6079cc9..7a72afc885 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/BindingIndependentDataServiceConnector.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/BindingIndependentDataServiceConnector.java @@ -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 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 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 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 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 { @@ -271,10 +310,11 @@ public class BindingIndependentDataServiceConnector implements // @Override public void onRegister(DataCommitHandlerRegistration, 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 diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/BindingIndependentMappingService.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/BindingIndependentMappingService.java index 9e175b8cb0..b1983fe224 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/BindingIndependentMappingService.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/BindingIndependentMappingService.java @@ -16,8 +16,8 @@ public interface BindingIndependentMappingService { org.opendaylight.yangtools.yang.data.api.InstanceIdentifier toDataDom(InstanceIdentifier path); - DataObject dataObjectFromDataDom(InstanceIdentifier path, CompositeNode result); + DataObject dataObjectFromDataDom(InstanceIdentifier 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 index 0000000000..9331899686 --- /dev/null +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DeserializationException.java @@ -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); + } + +} diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/impl/DataReaderRouter.xtend b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/impl/DataReaderRouter.xtend index fbed2ca113..504a3d6394 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/impl/DataReaderRouter.xtend +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/impl/DataReaderRouter.xtend @@ -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 { + 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 data) { + val pathArgument = path.path.last; + var empty = true; + var name = pathArgument?.nodeType; + val nodes = new ArrayList>(); + val keyNodes = new HashMap>(); 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>(); + finalNodes.addAll(keyNodes.values); + finalNodes.addAll(nodes); + return new CompositeNodeTOImpl(name,null,finalNodes); } - + + + + dispatch def Map> getKeyNodes(PathArgument argument, CompositeNode node) { + return Collections.emptyMap(); + } + + dispatch def getKeyNodes(NodeIdentifierWithPredicates argument, CompositeNode node) { + val ret = new HashMap>(); + 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> childrenWithout(CompositeNode node, Set>> entries) { + if(entries.empty) { + return node.children; + } + val filteredNodes = new ArrayList>(); + for(scannedNode : node.children) { + if(!entries.contains(scannedNode.nodeType)) { + filteredNodes.add(scannedNode); + } + } + return filteredNodes; + } + } -- 2.36.6