From: Ed Warnicke Date: Fri, 29 Nov 2013 14:20:34 +0000 (+0000) Subject: Merge "Finding data nodes in choices and cases." X-Git-Tag: jenkins-controller-bulk-release-prepare-only-2-1~299 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=1720ef0759ca8d7ff96bc82563f7673d68a20377;hp=822d42d3709cc7b5f9fb1be96f9b7bde471dadc8 Merge "Finding data nodes in choices and cases." --- diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BlankTransactionServiceTracker.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BlankTransactionServiceTracker.java index de1a425ce6..3d0decb93d 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BlankTransactionServiceTracker.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BlankTransactionServiceTracker.java @@ -7,6 +7,7 @@ */ package org.opendaylight.controller.config.manager.impl.osgi; +import org.opendaylight.controller.config.api.ConflictingVersionException; import org.opendaylight.controller.config.api.jmx.CommitStatus; import org.opendaylight.controller.config.manager.impl.ConfigRegistryImpl; import org.opendaylight.controller.config.spi.ModuleFactory; @@ -19,7 +20,7 @@ import javax.management.ObjectName; /** * Every time factory is added or removed, blank transaction is triggered to handle - * {@link org.opendaylight.controller.config.spi.ModuleFactory#getDefaultModules(org.opendaylight.controller.config.api.DependencyResolverFactory)} + * {@link org.opendaylight.controller.config.spi.ModuleFactory#getDefaultModules(org.opendaylight.controller.config.api.DependencyResolverFactory, org.osgi.framework.BundleContext)} * functionality. */ public class BlankTransactionServiceTracker implements ServiceTrackerCustomizer { @@ -38,14 +39,30 @@ public class BlankTransactionServiceTracker implements ServiceTrackerCustomizer< } private synchronized void blankTransaction() { - // create transaction - ObjectName tx = configRegistry.beginConfig(); - CommitStatus commitStatus = configRegistry.commitConfig(tx); - logger.debug("Committed blank transaction with status {}", commitStatus); + // race condition check: config-persister might push new configuration while server is starting up. + ConflictingVersionException lastException = null; + for (int i = 0; i < 10; i++) { + try { + // create transaction + ObjectName tx = configRegistry.beginConfig(); + CommitStatus commitStatus = configRegistry.commitConfig(tx); + logger.debug("Committed blank transaction with status {}", commitStatus); + return; + } catch (ConflictingVersionException e) { + lastException = e; + try { + Thread.sleep(1000); + } catch (InterruptedException interruptedException) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(interruptedException); + } + } + } + throw lastException; } @Override - public void modifiedService(ServiceReference moduleFactoryServiceReference, Object o) { + public void modifiedService(ServiceReference moduleFactoryServiceReference, Object o) { blankTransaction(); } diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/BrokerFacade.xtend b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/BrokerFacade.xtend index 29ad7522c4..1b0fda41ca 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/BrokerFacade.xtend +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/BrokerFacade.xtend @@ -1,6 +1,5 @@ package org.opendaylight.controller.sal.restconf.impl -import javax.ws.rs.WebApplicationException import javax.ws.rs.core.Response import org.opendaylight.controller.md.sal.common.api.data.DataReader import org.opendaylight.controller.sal.core.api.Broker.ConsumerSession diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/CompositeNodeWrapper.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/CompositeNodeWrapper.java index e000c7e29e..c741c9aa19 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/CompositeNodeWrapper.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/CompositeNodeWrapper.java @@ -24,6 +24,7 @@ public final class CompositeNodeWrapper implements NodeWrapper, C private String localName; private URI namespace; + private QName name; private List> values = new ArrayList<>(); public CompositeNodeWrapper(String localName) { @@ -34,6 +35,12 @@ public final class CompositeNodeWrapper implements NodeWrapper, C this(localName); this.namespace = namespace; } + + @Override + public void setQname(QName name) { + Preconditions.checkState(compositeNode == null, "Cannot change the object, due to data inconsistencies."); + this.name = name; + } @Override public String getLocalName() { @@ -75,9 +82,11 @@ public final class CompositeNodeWrapper implements NodeWrapper, C @Override public CompositeNode unwrap(CompositeNode parent) { if (compositeNode == null) { - Preconditions.checkNotNull(namespace); - compositeNode = NodeFactory.createMutableCompositeNode(new QName(namespace, localName), - parent, new ArrayList>(), ModifyAction.CREATE, null); + if (name == null) { + Preconditions.checkNotNull(namespace); + name = new QName(namespace, localName); + } + compositeNode = NodeFactory.createMutableCompositeNode(name, parent, new ArrayList>(), null, null); List> nodeValues = new ArrayList<>(); for (NodeWrapper nodeWrapper : values) { @@ -88,6 +97,7 @@ public final class CompositeNodeWrapper implements NodeWrapper, C values = null; namespace = null; localName = null; + name = null; } return compositeNode; } diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/ControllerContext.xtend b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/ControllerContext.xtend index 065d01e8e9..882c73d001 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/ControllerContext.xtend +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/ControllerContext.xtend @@ -128,6 +128,25 @@ class ControllerContext implements SchemaServiceListener { private def dispatch CharSequence toRestconfIdentifier(PathArgument argument, DataSchemaNode node) { throw new IllegalArgumentException("Conversion of generic path argument is not supported"); } + + def findModuleByNamespace(URI namespace) { + checkPreconditions + var module = uriToModuleName.get(namespace) + if (module === null) { + val moduleSchemas = schemas.findModuleByNamespace(namespace); + if(moduleSchemas === null) throw new IllegalArgumentException() + var latestModule = moduleSchemas.head + for (m : moduleSchemas) { + if (m.revision.after(latestModule.revision)) { + latestModule = m + } + } + if(latestModule === null) throw new IllegalArgumentException() + uriToModuleName.put(namespace, latestModule.name) + module = latestModule.name; + } + return module + } def CharSequence toRestconfIdentifier(QName qname) { checkPreconditions diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/NodeWrapper.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/NodeWrapper.java index 0a3616e494..db7770fc68 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/NodeWrapper.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/NodeWrapper.java @@ -2,11 +2,14 @@ package org.opendaylight.controller.sal.restconf.impl; import java.net.URI; +import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.CompositeNode; import org.opendaylight.yangtools.yang.data.api.Node; public interface NodeWrapper> { + void setQname(QName name); + T unwrap(CompositeNode parent); URI getNamespace(); diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.xtend b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.xtend index b6426c65bf..b730754439 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.xtend +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.xtend @@ -1,10 +1,12 @@ package org.opendaylight.controller.sal.restconf.impl import java.util.List +import java.util.Set import javax.ws.rs.core.Response import org.opendaylight.controller.md.sal.common.api.TransactionStatus import org.opendaylight.controller.sal.rest.api.RestconfService import org.opendaylight.yangtools.yang.data.api.CompositeNode +import org.opendaylight.yangtools.yang.model.api.ChoiceNode import org.opendaylight.yangtools.yang.model.api.DataNodeContainer import org.opendaylight.yangtools.yang.model.api.DataSchemaNode @@ -77,7 +79,7 @@ class RestconfImpl implements RestconfService { override readConfigurationData(String identifier) { val instanceIdentifierWithSchemaNode = identifier.resolveInstanceIdentifier - val data = broker.readOperationalData(instanceIdentifierWithSchemaNode.getInstanceIdentifier); + val data = broker.readConfigurationData(instanceIdentifierWithSchemaNode.getInstanceIdentifier); return new StructuredData(data, instanceIdentifierWithSchemaNode.schemaNode) } @@ -112,16 +114,43 @@ class RestconfImpl implements RestconfService { } private def void addNamespaceToNodeFromSchemaRecursively(NodeWrapper nodeBuilder, DataSchemaNode schema) { - if (nodeBuilder.namespace === null) { - nodeBuilder.namespace = schema.QName.namespace + if (schema === null) { + throw new ResponseException(Response.Status.BAD_REQUEST, + "Data has bad format\n" + nodeBuilder.localName + " does not exist in yang schema."); + } + val moduleName = controllerContext.findModuleByNamespace(schema.QName.namespace); + if (nodeBuilder.namespace === null || nodeBuilder.namespace == schema.QName.namespace || + nodeBuilder.namespace.path == moduleName) { + nodeBuilder.qname = schema.QName + } else { + throw new ResponseException(Response.Status.BAD_REQUEST, + "Data has bad format\nIf data is in XML format then namespace for " + nodeBuilder.localName + + " should be " + schema.QName.namespace + ".\n If data is in Json format then module name for " + + nodeBuilder.localName + " should be " + moduleName + "."); } if (nodeBuilder instanceof CompositeNodeWrapper) { val List> children = (nodeBuilder as CompositeNodeWrapper).getValues for (child : children) { addNamespaceToNodeFromSchemaRecursively(child, - (schema as DataNodeContainer).childNodes.findFirst[n|n.QName.localName.equals(child.localName)]) + findFirstSchemaByLocalName(child.localName, (schema as DataNodeContainer).childNodes)) + } + } + } + + private def DataSchemaNode findFirstSchemaByLocalName(String localName, Set schemas) { + for (schema : schemas) { + if (schema instanceof ChoiceNode) { + for (caze : (schema as ChoiceNode).cases) { + val result = findFirstSchemaByLocalName(localName, caze.childNodes) + if (result !== null) { + return result + } + } + } else { + return schemas.findFirst[n|n.QName.localName.equals(localName)] } } + return null } } diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/SimpleNodeWrapper.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/SimpleNodeWrapper.java index 4600d0890b..1b103b43c2 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/SimpleNodeWrapper.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/SimpleNodeWrapper.java @@ -18,6 +18,7 @@ public final class SimpleNodeWrapper implements NodeWrapper>, Simp private String localName; private String value; private URI namespace; + private QName name; public SimpleNodeWrapper(String localName, String value) { this.localName = Preconditions.checkNotNull(localName); @@ -29,6 +30,12 @@ public final class SimpleNodeWrapper implements NodeWrapper>, Simp this.namespace = namespace; } + @Override + public void setQname(QName name) { + Preconditions.checkState(simpleNode == null, "Cannot change the object, due to data inconsistencies."); + this.name = name; + } + @Override public String getLocalName() { if (simpleNode != null) { @@ -54,12 +61,16 @@ public final class SimpleNodeWrapper implements NodeWrapper>, Simp @Override public SimpleNode unwrap(CompositeNode parent) { if (simpleNode == null) { - Preconditions.checkNotNull(namespace); - simpleNode = NodeFactory.createImmutableSimpleNode(new QName(namespace, localName), parent, value); + if (name == null) { + Preconditions.checkNotNull(namespace); + name = new QName(namespace, localName); + } + simpleNode = NodeFactory.createImmutableSimpleNode(name, parent, value); value = null; namespace = null; localName = null; + name = null; } return simpleNode; } diff --git a/opendaylight/netconf/config-persister-impl/pom.xml b/opendaylight/netconf/config-persister-impl/pom.xml index ce25de215e..dce858fce6 100644 --- a/opendaylight/netconf/config-persister-impl/pom.xml +++ b/opendaylight/netconf/config-persister-impl/pom.xml @@ -89,6 +89,9 @@ org.slf4j, org.w3c.dom, org.xml.sax, + javax.xml.namespace, + javax.xml.xpath, + org.opendaylight.controller.config.api diff --git a/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandler.java b/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandler.java index a569f90538..a777ea4099 100644 --- a/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandler.java +++ b/opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandler.java @@ -13,6 +13,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Sets; import io.netty.channel.EventLoopGroup; import io.netty.channel.nio.NioEventLoopGroup; +import org.opendaylight.controller.config.api.ConflictingVersionException; import org.opendaylight.controller.config.persist.api.ConfigSnapshotHolder; import org.opendaylight.controller.config.persist.api.Persister; import org.opendaylight.controller.netconf.api.NetconfMessage; @@ -21,6 +22,7 @@ import org.opendaylight.controller.netconf.api.jmx.DefaultCommitOperationMXBean; import org.opendaylight.controller.netconf.api.jmx.NetconfJMXNotification; import org.opendaylight.controller.netconf.client.NetconfClient; import org.opendaylight.controller.netconf.client.NetconfClientDispatcher; +import org.opendaylight.controller.netconf.util.xml.XMLNetconfUtil; import org.opendaylight.controller.netconf.util.xml.XmlElement; import org.opendaylight.controller.netconf.util.xml.XmlNetconfConstants; import org.opendaylight.controller.netconf.util.xml.XmlUtil; @@ -37,6 +39,8 @@ import javax.management.Notification; import javax.management.NotificationListener; import javax.management.ObjectName; import javax.net.ssl.SSLContext; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathExpression; import java.io.Closeable; import java.io.IOException; import java.io.InputStream; @@ -66,7 +70,7 @@ public class ConfigPersisterNotificationHandler implements NotificationListener, private final Persister persister; private final MBeanServerConnection mbeanServer; - private Long currentSessionId; + private final ObjectName on = DefaultCommitOperationMXBean.objectName; @@ -96,16 +100,25 @@ public class ConfigPersisterNotificationHandler implements NotificationListener, if (maybeConfig.isPresent()) { logger.debug("Last config found {}", persister); - - registerToNetconf(maybeConfig.get().getCapabilities()); - - final String configSnapshot = maybeConfig.get().getConfigSnapshot(); - logger.trace("Pushing following xml to netconf {}", configSnapshot); - try { - pushLastConfig(XmlUtil.readXmlToElement(configSnapshot)); - } catch (SAXException | IOException e) { - throw new IllegalStateException("Unable to load last config", e); + ConflictingVersionException lastException = null; + int maxAttempts = 30; + for(int i = 0 ; i < maxAttempts; i++) { + registerToNetconf(maybeConfig.get().getCapabilities()); + + final String configSnapshot = maybeConfig.get().getConfigSnapshot(); + logger.trace("Pushing following xml to netconf {}", configSnapshot); + try { + pushLastConfig(XmlUtil.readXmlToElement(configSnapshot)); + } catch(ConflictingVersionException e) { + closeClientAndDispatcher(netconfClient, netconfClientDispatcher); + lastException = e; + Thread.sleep(1000); + } catch (SAXException | IOException e) { + throw new IllegalStateException("Unable to load last config", e); + } } + throw new IllegalStateException("Failed to push configuration, maximum attempt count has been reached: " + + maxAttempts, lastException); } else { // this ensures that netconf is initialized, this is first @@ -130,9 +143,9 @@ public class ConfigPersisterNotificationHandler implements NotificationListener, int attempt = 0; - while (true) { + long deadline = pollingStart + timeout; + while (System.currentTimeMillis() < deadline) { attempt++; - netconfClientDispatcher = new NetconfClientDispatcher(Optional.absent(), nettyThreadgroup, nettyThreadgroup); try { netconfClient = new NetconfClient(this.toString(), address, delay, netconfClientDispatcher); @@ -146,14 +159,12 @@ public class ConfigPersisterNotificationHandler implements NotificationListener, if (isSubset(currentCapabilities, expectedCaps)) { logger.debug("Hello from netconf stable with {} capabilities", currentCapabilities); - currentSessionId = netconfClient.getSessionId(); + long currentSessionId = netconfClient.getSessionId(); logger.info("Session id received from netconf server: {}", currentSessionId); return currentSessionId; } - if (System.currentTimeMillis() > pollingStart + timeout) { - break; - } + logger.debug("Polling hello from netconf, attempt {}, capabilities {}", attempt, currentCapabilities); @@ -245,7 +256,7 @@ public class ConfigPersisterNotificationHandler implements NotificationListener, return maybeConfigElement; } - private synchronized void pushLastConfig(Element xmlToBePersisted) { + private synchronized void pushLastConfig(Element xmlToBePersisted) throws ConflictingVersionException { logger.info("Pushing last configuration to netconf"); StringBuilder response = new StringBuilder("editConfig response = {"); @@ -276,19 +287,26 @@ public class ConfigPersisterNotificationHandler implements NotificationListener, logger.trace("Detailed message {}", response); } - private void checkIsOk(XmlElement element, NetconfMessage responseMessage) { + static void checkIsOk(XmlElement element, NetconfMessage responseMessage) throws ConflictingVersionException { if (element.getName().equals(XmlNetconfConstants.OK)) { return; - } else { - if (element.getName().equals(XmlNetconfConstants.RPC_ERROR)) { - logger.warn("Can not load last configuration, operation failed"); - throw new IllegalStateException("Can not load last configuration, operation failed: " - + XmlUtil.toString(responseMessage.getDocument())); + } + + if (element.getName().equals(XmlNetconfConstants.RPC_ERROR)) { + logger.warn("Can not load last configuration, operation failed"); + // is it ConflictingVersionException ? + XPathExpression xPathExpression = XMLNetconfUtil.compileXPath("/netconf:rpc-reply/netconf:rpc-error/netconf:error-info/netconf:error"); + String error = (String) XmlUtil.evaluateXPath(xPathExpression, element.getDomElement(), XPathConstants.STRING); + if (error!=null && error.contains(ConflictingVersionException.class.getCanonicalName())) { + throw new ConflictingVersionException(error); } - logger.warn("Can not load last configuration. Operation failed."); - throw new IllegalStateException("Can not load last configuration. Operation failed: " + throw new IllegalStateException("Can not load last configuration, operation failed: " + XmlUtil.toString(responseMessage.getDocument())); } + + logger.warn("Can not load last configuration. Operation failed."); + throw new IllegalStateException("Can not load last configuration. Operation failed: " + + XmlUtil.toString(responseMessage.getDocument())); } private static NetconfMessage createEditConfigMessage(Element dataElement, String editConfigResourcename) { diff --git a/opendaylight/netconf/config-persister-impl/src/test/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandlerTest.java b/opendaylight/netconf/config-persister-impl/src/test/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandlerTest.java new file mode 100644 index 0000000000..6c45c9c011 --- /dev/null +++ b/opendaylight/netconf/config-persister-impl/src/test/java/org/opendaylight/controller/netconf/persist/impl/ConfigPersisterNotificationHandlerTest.java @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2013 Cisco Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.controller.netconf.persist.impl; + +import org.junit.Test; +import org.opendaylight.controller.config.api.ConflictingVersionException; +import org.opendaylight.controller.netconf.api.NetconfMessage; +import org.opendaylight.controller.netconf.util.xml.XmlElement; +import org.opendaylight.controller.netconf.util.xml.XmlUtil; +import org.w3c.dom.Document; + +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; +import static org.junit.matchers.JUnitMatchers.containsString; + +public class ConfigPersisterNotificationHandlerTest { + + @Test + public void testConflictingVersionDetection() throws Exception { + Document document = XmlUtil.readXmlToDocument(getClass().getResourceAsStream("/conflictingVersionResponse.xml")); + try{ + ConfigPersisterNotificationHandler.checkIsOk(XmlElement.fromDomDocument(document).getOnlyChildElement(), new NetconfMessage(document)); + fail(); + }catch(ConflictingVersionException e){ + assertThat(e.getMessage(), containsString("Optimistic lock failed. Expected parent version 21, was 18")); + } + } + +} diff --git a/opendaylight/netconf/config-persister-impl/src/test/resources/conflictingVersionResponse.xml b/opendaylight/netconf/config-persister-impl/src/test/resources/conflictingVersionResponse.xml new file mode 100644 index 0000000000..f65e0e0bc2 --- /dev/null +++ b/opendaylight/netconf/config-persister-impl/src/test/resources/conflictingVersionResponse.xml @@ -0,0 +1,13 @@ + + + application + operation-failed + error + + + + + org.opendaylight.controller.config.api.ConflictingVersionException: Optimistic lock failed. Expected parent version 21, was 18 + + +