From: Tom Pantelis Date: Wed, 17 Dec 2014 23:56:32 +0000 (+0000) Subject: Merge "BUG 2509 : Removing all journal entries from a Followers in-memory journal... X-Git-Tag: release/lithium~754 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=3591817114661bb7971d6d355186ff1b39636fcd;hp=dff59a3b6775db8e7215254d3c5da65388221949 Merge "BUG 2509 : Removing all journal entries from a Followers in-memory journal causes Leader to send an InstallSnapshot" --- diff --git a/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/Entity.java b/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/Entity.java index 64f4c7ef1e..f10c116755 100644 --- a/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/Entity.java +++ b/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/Entity.java @@ -165,8 +165,9 @@ public class Entity implements Comparable { public void setLastSeenTimestamp(Date lastSeenTimestamp) { if (activeSince == null || (activeSince.getTime() + ACTIVITY_TIMEOUT) < lastSeenTimestamp - .getTime()) + .getTime()) { this.activeSince = lastSeenTimestamp; + } this.lastSeenTimestamp = lastSeenTimestamp; } @@ -180,8 +181,9 @@ public class Entity implements Comparable { @Override public int hashCode() { - if (hashCode != 0) + if (hashCode != 0) { return hashCode; + } final int prime = 31; hashCode = 1; hashCode = prime * hashCode @@ -194,30 +196,40 @@ public class Entity implements Comparable { @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) + } + if (obj == null) { return false; - if (getClass() != obj.getClass()) + } + if (getClass() != obj.getClass()) { return false; + } Entity other = (Entity) obj; if (ipv4Address == null) { - if (other.ipv4Address != null) + if (other.ipv4Address != null) { return false; - } else if (!ipv4Address.equals(other.ipv4Address)) + } + } else if (!ipv4Address.equals(other.ipv4Address)) { return false; - if (macAddress != other.macAddress) + } + if (macAddress != other.macAddress) { return false; + } if (port == null) { - if (other.port != null) + if (other.port != null) { return false; - } else if (!port.equals(other.port)) + } + } else if (!port.equals(other.port)) { return false; + } if (vlan == null) { - if (other.vlan != null) + if (other.vlan != null) { return false; - } else if (!vlan.equals(other.vlan)) + } + } else if (!vlan.equals(other.vlan)) { return false; + } return true; } @@ -257,8 +269,9 @@ public class Entity implements Comparable { Comparable switchId = (Comparable) port.getNode().getID(); Comparable oswitchId = (Comparable) o.port.getNode().getID(); r = switchId.compareTo(oswitchId); - if (r != 0) + if (r != 0) { return r; + } Comparable portId = (Comparable) port.getID(); Comparable oportId = (Comparable) o.port.getID(); diff --git a/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/SwitchPort.java b/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/SwitchPort.java index e60f8b4b0e..892e1d6c79 100644 --- a/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/SwitchPort.java +++ b/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/SwitchPort.java @@ -127,20 +127,26 @@ public class SwitchPort { @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) + } + if (obj == null) { return false; - if (getClass() != obj.getClass()) + } + if (getClass() != obj.getClass()) { return false; + } SwitchPort other = (SwitchPort) obj; - if (errorStatus != other.errorStatus) + if (errorStatus != other.errorStatus) { return false; + } if (port == null) { - if (other.port != null) + if (other.port != null) { return false; - } else if (!port.equals(other.port)) + } + } else if (!port.equals(other.port)) { return false; + } return true; } diff --git a/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/hostAware/HostNodeConnector.java b/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/hostAware/HostNodeConnector.java index fe396ba92b..86376be5c5 100644 --- a/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/hostAware/HostNodeConnector.java +++ b/opendaylight/adsal/hosttracker_new/api/src/main/java/org/opendaylight/controller/hosttracker/hostAware/HostNodeConnector.java @@ -150,22 +150,29 @@ public class HostNodeConnector extends Host { @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (!super.equals(obj)) + } + if (!super.equals(obj)) { return false; - if (getClass() != obj.getClass()) + } + if (getClass() != obj.getClass()) { return false; + } HostNodeConnector other = (HostNodeConnector) obj; if (nodeConnector == null) { - if (other.nodeConnector != null) + if (other.nodeConnector != null) { return false; - } else if (!nodeConnector.equals(other.nodeConnector)) + } + } else if (!nodeConnector.equals(other.nodeConnector)) { return false; - if (staticHost != other.staticHost) + } + if (staticHost != other.staticHost) { return false; - if (vlan != other.vlan) + } + if (vlan != other.vlan) { return false; + } return true; } @@ -181,8 +188,9 @@ public class HostNodeConnector extends Host { EthernetAddress e = (EthernetAddress) getDataLayerAddress(); macaddr = e.getValue(); } - if (macaddr == null) + if (macaddr == null) { return false; + } return !Arrays.equals(emptyArray, macaddr); } diff --git a/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/IteratableTypeInfo.java b/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/IteratableTypeInfo.java index 3977837c7f..d0386ed2c5 100644 --- a/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/IteratableTypeInfo.java +++ b/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/IteratableTypeInfo.java @@ -29,7 +29,9 @@ import java.util.List; Object item = it.next(); for (TypeInfo child : _types.values()) { Object val = child.retrieve(item, query, index); - if (val != null) objects.add(val); + if (val != null) { + objects.add(val); + } } } return objects; @@ -38,7 +40,9 @@ import java.util.List; @Override public synchronized void explore() { - if (_explored) return; + if (_explored) { + return; + } if (LOGGER.isDebugEnabled()) { LOGGER.debug("exploring iteratable type: {} gtype: {}", _class, _accessor.getGenericType()); diff --git a/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/QueryContextImpl.java b/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/QueryContextImpl.java index 13d70b1cb0..8f83c147c8 100644 --- a/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/QueryContextImpl.java +++ b/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/QueryContextImpl.java @@ -16,17 +16,25 @@ import org.slf4j.LoggerFactory; @Override public Query createQuery(String queryString, Class type) throws QueryException { - if (queryString == null || queryString.trim().length() == 0) return null; + if (queryString == null || queryString.trim().length() == 0) { + return null; + } try { - if (LOGGER.isDebugEnabled()) LOGGER.debug("Processing query: {}", queryString); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Processing query: {}", queryString); + } // FiqlParser is a parser generated by javacc Expression expression = FiqlParser.parse(queryString); - if (LOGGER.isDebugEnabled()) LOGGER.debug("Query expression: {}", expression); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Query expression: {}", expression); + } // create Query and return; return new QueryImpl(type, expression); } catch (Exception ex) { - if (LOGGER.isDebugEnabled()) LOGGER.error("Query processing failed = {}", + if (LOGGER.isDebugEnabled()) { + LOGGER.error("Query processing failed = {}", queryString, ex); + } throw new QueryException("Unable to parse query.", ex); } } diff --git a/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/QueryImpl.java b/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/QueryImpl.java index a520f98fc0..cb77e04a59 100644 --- a/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/QueryImpl.java +++ b/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/QueryImpl.java @@ -158,7 +158,9 @@ import org.slf4j.LoggerFactory; } private boolean compare(Object valueToMatch, Object actualValue, OP operator) { - if (valueToMatch == null || actualValue == null) return false; + if (valueToMatch == null || actualValue == null) { + return false; + } if (ALLOW_OBJECT_STRING_COMPARE && (valueToMatch instanceof String) && !(actualValue instanceof String)) { actualValue = actualValue.toString(); @@ -203,7 +205,9 @@ import org.slf4j.LoggerFactory; } } private Object parse(String arg, Object value) { - if (value == null) return null; + if (value == null) { + return null; + } try { if (value instanceof String) { diff --git a/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/TypeInfo.java b/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/TypeInfo.java index 91f01d8ad7..6c0c5dc816 100644 --- a/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/TypeInfo.java +++ b/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/TypeInfo.java @@ -107,7 +107,9 @@ import org.slf4j.LoggerFactory; if (LOGGER.isDebugEnabled()) { LOGGER.debug("retrieve: {}/{} type:{}", index, query.length, target.getClass()); } - if (index >= query.length) return null; + if (index >= query.length) { + return null; + } explore(); if (!target.getClass().equals(_class)) { if (_class.isAssignableFrom(target.getClass())) { @@ -124,7 +126,9 @@ import org.slf4j.LoggerFactory; } } TypeInfo child = getChild(query[index]); - if (child == null) return null; + if (child == null) { + return null; + } target = child.getAccessor().getValue(target); if (index+1 == query.length) { // match found @@ -137,27 +141,35 @@ import org.slf4j.LoggerFactory; * Explore the type info for children. */ public synchronized void explore() { - if (_explored) return; + if (_explored) { + return; + } for (Class c = _class; c != null; c = c.getSuperclass()) { - if (c.equals(Object.class)) break; + if (c.equals(Object.class)) { + break; + } // Currently only fields and methods annotated with JAXB annotations are // considered as valid for search purposes. //check methods first for (Method m : c.getDeclaredMethods()) { String tn = getTypeName(m, _accessType); if (tn != null) { - if (LOGGER.isDebugEnabled()) LOGGER.debug( + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( "exploring type: {} name: {} method: {}", _class.getSimpleName(), tn, m); + } _types.put(tn, createTypeInfo(tn, new Accessor(m))); } } for (Field f : c.getDeclaredFields()) { String tn = getTypeName(f, _accessType); if (tn != null) { - if (LOGGER.isDebugEnabled()) LOGGER.debug( + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( "exploring type: {} name: {} field: {}", _class.getSimpleName(), tn, f); + } _types.put(tn, createTypeInfo(tn, new Accessor(f))); } } @@ -219,7 +231,9 @@ import org.slf4j.LoggerFactory; PropertyDescriptor[] props = info.getPropertyDescriptors(); for (PropertyDescriptor pd : props) { - if (m.equals(pd.getReadMethod())) return pd.getName(); + if (m.equals(pd.getReadMethod())) { + return pd.getName(); + } } } catch (IntrospectionException e) @@ -234,8 +248,12 @@ import org.slf4j.LoggerFactory; // root is always a composite type // FIXME assert its a JAXB type XmlRootElement root = clz.getAnnotation(XmlRootElement.class); - if (root == null) throw new IllegalArgumentException("Not a JAXB type: " + clz); - if (name == null) name = getRootName(clz); + if (root == null) { + throw new IllegalArgumentException("Not a JAXB type: " + clz); + } + if (name == null) { + name = getRootName(clz); + } return new TypeInfo(name, clz, null); } @@ -252,7 +270,9 @@ import org.slf4j.LoggerFactory; public static String getRootName(Class cls) { XmlRootElement root = cls.getAnnotation(XmlRootElement.class); - if (root == null) return null; + if (root == null) { + return null; + } String rootName = root.name(); if (DEFAULT_NAME.equals(rootName)) { String clsName = cls.getSimpleName(); @@ -281,7 +301,9 @@ import org.slf4j.LoggerFactory; return null; } } - if (DEFAULT_NAME.equals(name)) return dflt; + if (DEFAULT_NAME.equals(name)) { + return dflt; + } return name; } diff --git a/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/WrapperTypeInfo.java b/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/WrapperTypeInfo.java index a8172f2add..156942c61d 100644 --- a/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/WrapperTypeInfo.java +++ b/opendaylight/adsal/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/query/WrapperTypeInfo.java @@ -24,10 +24,14 @@ public class WrapperTypeInfo extends TypeInfo { if (LOGGER.isDebugEnabled()) { LOGGER.debug("retrieve collection: {}/{}", index, query.length); } - if (index >= query.length) return null; + if (index >= query.length) { + return null; + } explore(); TypeInfo child = getChild(query[index]); - if (child == null) return null; + if (child == null) { + return null; + } if (query.length == index+1) { // skipping this node return target; }else { // if list of list go to next node to get value @@ -37,7 +41,9 @@ public class WrapperTypeInfo extends TypeInfo { @Override public synchronized void explore() { - if (_explored) return; + if (_explored) { + return; + } if (LOGGER.isDebugEnabled()) { LOGGER.debug("exploring wrapper type: {} gtype: {}", _class, _accessor.getGenericType()); diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/JmxAttributeValidationException.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/JmxAttributeValidationException.java index 3380a10afe..b09017fe55 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/JmxAttributeValidationException.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/JmxAttributeValidationException.java @@ -84,7 +84,7 @@ public class JmxAttributeValidationException extends RuntimeException { public static void checkCondition(boolean condition, String message, JmxAttribute jmxAttribute) throws JmxAttributeValidationException { - if (condition == false) { + if (!condition) { throw new JmxAttributeValidationException( jmxAttribute.getAttributeName() + " " + message, jmxAttribute); diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/ObjectNameUtil.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/ObjectNameUtil.java index ee23206eeb..0c7478dca0 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/ObjectNameUtil.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/ObjectNameUtil.java @@ -128,7 +128,7 @@ public class ObjectNameUtil { if (quoted == null) { throw new IllegalArgumentException("Cannot find " + SERVICE_QNAME_KEY + " in " + objectName); } - if (quoted.startsWith("\"") == false || quoted.endsWith("\"") == false) { + if (!quoted.startsWith("\"") || !quoted.endsWith("\"")) { throw new IllegalArgumentException("Quotes not found in " + objectName); } String substring = quoted.substring(1); @@ -201,7 +201,7 @@ public class ObjectNameUtil { throw new IllegalArgumentException( "Expected ObjectName with transaction:" + inputON); } - if (ON_DOMAIN.equals(inputON.getDomain()) == false) { + if (!ON_DOMAIN.equals(inputON.getDomain())) { throw new IllegalArgumentException("Expected different domain: " + inputON); } @@ -263,7 +263,7 @@ public class ObjectNameUtil { Map allProperties = getAdditionalProperties(on); Map result = new HashMap<>(); for (Entry entry : allProperties.entrySet()) { - if (blacklist.contains(entry.getKey()) == false) { + if (!blacklist.contains(entry.getKey())) { result.put(entry.getKey(), entry.getValue()); } } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java index 93a6168ec2..96739fb822 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java @@ -196,7 +196,6 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe * {@inheritDoc} */ @Override - @SuppressWarnings("PMD.AvoidCatchingThrowable") public synchronized CommitStatus commitConfig(ObjectName transactionControllerON) throws ConflictingVersionException, ValidationException { final String transactionName = ObjectNameUtil diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AbstractDynamicWrapper.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AbstractDynamicWrapper.java index d868f7c332..de367eaf9c 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AbstractDynamicWrapper.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AbstractDynamicWrapper.java @@ -49,8 +49,6 @@ import org.opendaylight.controller.config.spi.Module; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static java.lang.String.format; - /** * Contains common code for readable/rw dynamic mbean wrappers. Routes all * requests (getAttribute, setAttribute, invoke) into the actual instance, but diff --git a/opendaylight/config/yang-jmx-generator/pom.xml b/opendaylight/config/yang-jmx-generator/pom.xml index 979b39688b..bfeb3f0f7a 100644 --- a/opendaylight/config/yang-jmx-generator/pom.xml +++ b/opendaylight/config/yang-jmx-generator/pom.xml @@ -34,6 +34,10 @@ org.opendaylight.yangtools binding-type-provider + + org.opendaylight.yangtools + yang-model-util + org.opendaylight.yangtools diff --git a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java index 80db46df2b..ed727c9a13 100644 --- a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java +++ b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java @@ -374,7 +374,7 @@ final class ModuleMXBeanEntryBuilder { final String javaNamePrefix) { return RuntimeBeanEntry.extractClassNameToRuntimeBeanMap(packageName, dataNodeContainer, moduleLocalNameFromXPath, - typeProviderWrapper, javaNamePrefix, currentModule).values(); + typeProviderWrapper, javaNamePrefix, currentModule, schemaContext).values(); } diff --git a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntry.java b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntry.java index b6ed824321..74981a9582 100644 --- a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntry.java +++ b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntry.java @@ -11,8 +11,14 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.base.Optional; +import com.google.common.base.Preconditions; +import com.google.common.collect.Collections2; +import com.google.common.collect.HashMultimap; import com.google.common.collect.Lists; +import com.google.common.collect.Multimap; +import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -20,6 +26,7 @@ import java.util.Comparator; import java.util.Deque; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -36,15 +43,16 @@ import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode; import org.opendaylight.yangtools.yang.model.api.DataNodeContainer; import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; -import org.opendaylight.yangtools.yang.model.api.IdentitySchemaNode; import org.opendaylight.yangtools.yang.model.api.LeafListSchemaNode; import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode; import org.opendaylight.yangtools.yang.model.api.ListSchemaNode; import org.opendaylight.yangtools.yang.model.api.Module; import org.opendaylight.yangtools.yang.model.api.RpcDefinition; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.opendaylight.yangtools.yang.model.api.SchemaNode; import org.opendaylight.yangtools.yang.model.api.UnknownSchemaNode; import org.opendaylight.yangtools.yang.model.api.UsesNode; +import org.opendaylight.yangtools.yang.model.util.SchemaContextUtil; /** * Holds information about runtime bean to be generated. There are two kinds of @@ -56,6 +64,21 @@ import org.opendaylight.yangtools.yang.model.api.UsesNode; * lined via children so that a tree with all beans can be created. */ public class RuntimeBeanEntry { + + private static final Function QNAME_FROM_NODE = new Function() { + @Override + public QName apply(final SchemaNode input) { + return input.getQName(); + } + }; + + private static final Function UNKNOWN_NODE_TO_STRING = new Function() { + @Override + public String apply(final UnknownSchemaNode input) { + return input.getQName().getLocalName() + input.getNodeParameter(); + } + }; + private final String packageName; private final String yangName, javaNamePrefix; private final boolean isRoot; @@ -112,13 +135,12 @@ public class RuntimeBeanEntry { public static Map extractClassNameToRuntimeBeanMap( final String packageName, final DataNodeContainer container, final String moduleYangName, final TypeProviderWrapper typeProviderWrapper, - final String javaNamePrefix, final Module currentModule) { + final String javaNamePrefix, final Module currentModule, final SchemaContext schemaContext) { - Map> identitiesToRpcs = getIdentitiesToRpcs(currentModule); AttributesRpcsAndRuntimeBeans attributesRpcsAndRuntimeBeans = extractSubtree( packageName, container, typeProviderWrapper, currentModule, - identitiesToRpcs); + schemaContext); Map result = new HashMap<>(); List attributes; @@ -150,52 +172,41 @@ public class RuntimeBeanEntry { return result; } - private static Map> getIdentitiesToRpcs( - final Module currentModule) { - // currently only looks for local identities (found in currentModule) - Map> result = new HashMap<>(); - for (IdentitySchemaNode identity : currentModule.getIdentities()) { - // add all - result.put(identity.getQName(), new HashSet()); - } + private static Multimap getIdentitiesToRpcs( + final SchemaContext schemaCtx) { + Multimap result = HashMultimap.create(); + for (Module currentModule : schemaCtx.getModules()) { - for (RpcDefinition rpc : currentModule.getRpcs()) { - ContainerSchemaNode input = rpc.getInput(); - if (input != null) { - for (UsesNode uses : input.getUses()) { + // Find all identities in current module for later identity->rpc mapping + Set allIdentitiesInModule = Sets.newHashSet(Collections2.transform(currentModule.getIdentities(), QNAME_FROM_NODE)); - if (uses.getGroupingPath().getPath().size() != 1) { - continue; - } + for (RpcDefinition rpc : currentModule.getRpcs()) { + ContainerSchemaNode input = rpc.getInput(); + if (input != null) { + for (UsesNode uses : input.getUses()) { - // check grouping path - QName qname = uses.getGroupingPath().getPath().get(0); - if (false == qname - .equals(ConfigConstants.RPC_CONTEXT_REF_GROUPING_QNAME)) { - continue; - } + // Check if the rpc is config rpc by looking for input argument rpc-context-ref + Iterator pathFromRoot = uses.getGroupingPath().getPathFromRoot().iterator(); + if (!pathFromRoot.hasNext() || + !pathFromRoot.next().equals(ConfigConstants.RPC_CONTEXT_REF_GROUPING_QNAME)) { + continue; + } - for (SchemaNode refinedNode : uses.getRefines().values()) { - - for (UnknownSchemaNode unknownSchemaNode : refinedNode - .getUnknownSchemaNodes()) { - if (ConfigConstants.RPC_CONTEXT_INSTANCE_EXTENSION_QNAME - .equals(unknownSchemaNode.getNodeType())) { - String localIdentityName = unknownSchemaNode - .getNodeParameter(); - QName identityQName = QName.create( - currentModule.getNamespace(), - currentModule.getRevision(), - localIdentityName); - Set rpcDefinitions = result - .get(identityQName); - if (rpcDefinitions == null) { - throw new IllegalArgumentException( - "Identity referenced by rpc not found. Identity:" - + localIdentityName + " , rpc " - + rpc); + for (SchemaNode refinedNode : uses.getRefines().values()) { + for (UnknownSchemaNode unknownSchemaNode : refinedNode + .getUnknownSchemaNodes()) { + if (ConfigConstants.RPC_CONTEXT_INSTANCE_EXTENSION_QNAME + .equals(unknownSchemaNode.getNodeType())) { + String localIdentityName = unknownSchemaNode + .getNodeParameter(); + QName identityQName = QName.create( + currentModule.getNamespace(), + currentModule.getRevision(), + localIdentityName); + Preconditions.checkArgument(allIdentitiesInModule.contains(identityQName), + "Identity referenced by rpc not found. Identity: %s, rpc: %s", localIdentityName, rpc); + result.put(identityQName, rpc); } - rpcDefinitions.add(rpc); } } } @@ -212,7 +223,9 @@ public class RuntimeBeanEntry { private static AttributesRpcsAndRuntimeBeans extractSubtree( final String packageName, final DataNodeContainer subtree, final TypeProviderWrapper typeProviderWrapper, final Module currentModule, - final Map> identitiesToRpcs) { + final SchemaContext ctx) { + + Multimap identitiesToRpcs = getIdentitiesToRpcs(ctx); List attributes = Lists.newArrayList(); List runtimeBeanEntries = new ArrayList<>(); @@ -234,7 +247,7 @@ public class RuntimeBeanEntry { ListSchemaNode listSchemaNode = (ListSchemaNode) child; RuntimeBeanEntry hierarchicalChild = createHierarchical( packageName, listSchemaNode, typeProviderWrapper, - currentModule, identitiesToRpcs); + currentModule, ctx); runtimeBeanEntries.add(hierarchicalChild); } else /* ordinary list attribute */{ ListAttribute listAttribute = ListAttribute.create( @@ -258,18 +271,11 @@ public class RuntimeBeanEntry { if (ConfigConstants.RPC_CONTEXT_INSTANCE_EXTENSION_QNAME .equals(unknownSchemaNode.getNodeType())) { String localIdentityName = unknownSchemaNode.getNodeParameter(); - QName identityQName = QName.create(currentModule.getNamespace(), - currentModule.getRevision(), localIdentityName); - Set rpcDefinitions = identitiesToRpcs - .get(identityQName); - if (rpcDefinitions == null) { - throw new IllegalArgumentException("Cannot find identity " - + localIdentityName + " to be used as " - + "context reference when resolving " - + unknownSchemaNode); - } + QName identityQName = unknownSchemaNode.isAddedByUses() ? + findQNameFromGrouping(subtree, ctx, unknownSchemaNode, localIdentityName) : + QName.create(currentModule.getNamespace(), currentModule.getRevision(), localIdentityName); // convert RpcDefinition to Rpc - for (RpcDefinition rpcDefinition : rpcDefinitions) { + for (RpcDefinition rpcDefinition : identitiesToRpcs.get(identityQName)) { String name = TypeProviderWrapper .findJavaParameter(rpcDefinition); AttributeIfc returnType; @@ -310,6 +316,22 @@ public class RuntimeBeanEntry { attributes, rpcs); } + /** + * Find "proper" qname of unknown node in case it comes from a grouping + */ + private static QName findQNameFromGrouping(final DataNodeContainer subtree, final SchemaContext ctx, final UnknownSchemaNode unknownSchemaNode, final String localIdentityName) { + QName identityQName = null; + for (UsesNode usesNode : subtree.getUses()) { + SchemaNode dataChildByName = SchemaContextUtil.findDataSchemaNode(ctx, usesNode.getGroupingPath()); + Module m = SchemaContextUtil.findParentModule(ctx, dataChildByName); + List unknownSchemaNodes = dataChildByName.getUnknownSchemaNodes(); + if(Collections2.transform(unknownSchemaNodes, UNKNOWN_NODE_TO_STRING).contains(UNKNOWN_NODE_TO_STRING.apply(unknownSchemaNode))) { + identityQName = QName.create(dataChildByName.getQName(), localIdentityName); + } + } + return identityQName; + } + private static AttributeIfc getReturnTypeAttribute(final DataSchemaNode child, final TypeProviderWrapper typeProviderWrapper, final String packageName) { if (child instanceof LeafSchemaNode) { @@ -353,13 +375,13 @@ public class RuntimeBeanEntry { private static RuntimeBeanEntry createHierarchical(final String packageName, final ListSchemaNode listSchemaNode, final TypeProviderWrapper typeProviderWrapper, final Module currentModule, - final Map> identitiesToRpcs) { + final SchemaContext ctx) { // supported are numeric types, strings, enums // get all attributes AttributesRpcsAndRuntimeBeans attributesRpcsAndRuntimeBeans = extractSubtree( packageName, listSchemaNode, typeProviderWrapper, - currentModule, identitiesToRpcs); + currentModule, ctx); Optional keyYangName; if (listSchemaNode.getKeyDefinition().isEmpty()) { diff --git a/opendaylight/config/yang-jmx-generator/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntryTest.java b/opendaylight/config/yang-jmx-generator/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntryTest.java index 1503b84d64..98b1f752af 100644 --- a/opendaylight/config/yang-jmx-generator/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntryTest.java +++ b/opendaylight/config/yang-jmx-generator/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntryTest.java @@ -52,7 +52,7 @@ public class RuntimeBeanEntryTest extends AbstractYangTest { .getUnknownSchemaNodes(); Map runtimeBeans = RuntimeBeanEntry .extractClassNameToRuntimeBeanMap(PACKAGE_NAME, caseNode, "test-name", new TypeProviderWrapper(new - TypeProviderImpl(context)), "test", jmxImplModule); + TypeProviderImpl(context)), "test", jmxImplModule, context); assertEquals(1, runtimeBeans.size()); RuntimeBeanEntry runtimeMXBean = runtimeBeans.get("testRuntimeMXBean"); assertTrue(runtimeMXBean.isRoot()); diff --git a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/util/NetconfTestImplModuleUtil.java b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/util/NetconfTestImplModuleUtil.java index 5e37f5afcf..2428b10941 100644 --- a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/util/NetconfTestImplModuleUtil.java +++ b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/util/NetconfTestImplModuleUtil.java @@ -11,6 +11,7 @@ package org.opendaylight.controller.config.yang.test.util; import com.google.common.collect.Lists; +import java.math.BigInteger; import java.util.List; import org.opendaylight.controller.config.yang.test.impl.Asdf; import org.opendaylight.controller.config.yang.test.impl.Deep2; @@ -43,11 +44,36 @@ public class NetconfTestImplModuleUtil { return asdf; } + @Override + public BigInteger getCommonStat() { + return new BigInteger("54"); + } + @Override public String noArg(final String arg1) { return arg1.toUpperCase(); } + @Override + public Long commonRpcTwo() { + return 1L; + } + + @Override + public String commonRpcThree() { + return "true"; + } + + @Override + public Boolean commonRpc() { + return true; + } + + @Override + public void netconfImplRpcFromGrouping() { + // rpc from grouping within same yang module + } + }); for (int i = 0; i < module.getSimpleShort(); i++) { diff --git a/opendaylight/config/yang-test/src/main/yang/config-test-impl.yang b/opendaylight/config/yang-test/src/main/yang/config-test-impl.yang index e7aa64d7a6..093d7b3f13 100644 --- a/opendaylight/config/yang-test/src/main/yang/config-test-impl.yang +++ b/opendaylight/config/yang-test/src/main/yang/config-test-impl.yang @@ -8,6 +8,7 @@ module config-test-impl { import ietf-inet-types { prefix inet; revision-date 2010-09-24;} import rpc-context { prefix rpcx; revision-date 2013-06-17; } import test-types { prefix tt; revision-date 2013-11-27; } + import test-groups { prefix tg; revision-date 2014-12-08; } description "Testing IMPL"; @@ -347,6 +348,22 @@ module config-test-impl { } } + grouping netconf-impl-rpc { + rpcx:rpc-context-instance netconf-impl-rpc-ctx; + } + + identity netconf-impl-rpc-ctx; + + rpc netconf-impl-rpc-from-grouping { + input { + uses rpcx:rpc-context-ref { + refine context-instance { + rpcx:rpc-context-instance "netconf-impl-rpc-ctx"; + } + } + } + } + augment "/config:modules/config:module/config:state" { case impl-netconf { when "/config:modules/config:module/config:type = 'impl-netconf'"; @@ -354,6 +371,11 @@ module config-test-impl { // rpc rpcx:rpc-context-instance "test-rpc"; + // add some stats + rpc from groupings outside this module + uses tt:common-operational; + uses tg:common-operational-rpc; + uses netconf-impl-rpc; + // root runtime bean leaf created-sessions { type uint32; diff --git a/opendaylight/config/yang-test/src/main/yang/types/test-groups.yang b/opendaylight/config/yang-test/src/main/yang/types/test-groups.yang new file mode 100644 index 0000000000..00f704bed6 --- /dev/null +++ b/opendaylight/config/yang-test/src/main/yang/types/test-groups.yang @@ -0,0 +1,52 @@ +module test-groups { + yang-version 1; + namespace "urn:opendaylight:params:xml:ns:yang:controller:config:test:groups"; + prefix "tg"; + + import rpc-context { prefix rpcx; revision-date 2013-06-17; } + + description + "Groupings generated for testing"; + + revision "2014-12-08"; + + grouping common-operational-rpc { + rpcx:rpc-context-instance common-rpc-ctx; + rpcx:rpc-context-instance common-rpc-ctx-two; + } + + identity common-rpc-ctx; + identity common-rpc-ctx-two; + + rpc common-rpc { + input { + uses rpcx:rpc-context-ref { + refine context-instance { + rpcx:rpc-context-instance "common-rpc-ctx"; + } + } + } + + output { + leaf output { + type boolean; + } + } + } + + rpc common-rpc-two { + input { + uses rpcx:rpc-context-ref { + refine context-instance { + rpcx:rpc-context-instance "common-rpc-ctx-two"; + } + } + } + + output { + leaf output { + type uint32; + } + } + } +} diff --git a/opendaylight/config/yang-test/src/main/yang/types/test-types.yang b/opendaylight/config/yang-test/src/main/yang/types/test-types.yang index df5387be2c..ee466b4034 100644 --- a/opendaylight/config/yang-test/src/main/yang/types/test-types.yang +++ b/opendaylight/config/yang-test/src/main/yang/types/test-types.yang @@ -3,6 +3,8 @@ module test-types { namespace "urn:opendaylight:params:xml:ns:yang:controller:config:test:types"; prefix "tt"; + import rpc-context { prefix rpcx; revision-date 2013-06-17; } + description "Types generated for testing"; @@ -40,4 +42,34 @@ module test-types { identity test-identity2 { base test-identity1; } + + grouping common-operational { + leaf common-stat { + type uint64; + } + // This would not work, since it clashes with identity common-rpc-ctx from test-groups + // Both grouping add the same unknown node "rpcx:rpc-context-instance common-rpc-ctx-three;" + // and we cannot match the unknown node to the grouping that added it + //rpcx:rpc-context-instance common-rpc-ctx-three; + rpcx:rpc-context-instance common-rpc-ctx-three; + } + + //identity common-rpc-ctx; + identity common-rpc-ctx-three; + + rpc common-rpc-three { + input { + uses rpcx:rpc-context-ref { + refine context-instance { + rpcx:rpc-context-instance "common-rpc-ctx-three"; + } + } + } + + output { + leaf output { + type string; + } + } + } } diff --git a/opendaylight/md-sal/benchmark-data-store/src/main/java/org/opendaylight/controller/md/sal/dom/store/benchmark/AbstractInMemoryBrokerWriteTransactionBenchmark.java b/opendaylight/md-sal/benchmark-data-store/src/main/java/org/opendaylight/controller/md/sal/dom/store/benchmark/AbstractInMemoryBrokerWriteTransactionBenchmark.java index fdd715e54f..7765d1e480 100644 --- a/opendaylight/md-sal/benchmark-data-store/src/main/java/org/opendaylight/controller/md/sal/dom/store/benchmark/AbstractInMemoryBrokerWriteTransactionBenchmark.java +++ b/opendaylight/md-sal/benchmark-data-store/src/main/java/org/opendaylight/controller/md/sal/dom/store/benchmark/AbstractInMemoryBrokerWriteTransactionBenchmark.java @@ -8,9 +8,10 @@ package org.opendaylight.controller.md.sal.dom.store.benchmark; import java.util.concurrent.TimeUnit; + import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.dom.api.DOMDataReadWriteTransaction; -import org.opendaylight.controller.md.sal.dom.broker.impl.DOMDataBrokerImpl; +import org.opendaylight.controller.md.sal.dom.broker.impl.SerializedDOMDataBroker; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Measurement; @@ -21,7 +22,7 @@ import org.openjdk.jmh.annotations.Warmup; */ public abstract class AbstractInMemoryBrokerWriteTransactionBenchmark extends AbstractInMemoryWriteTransactionBenchmark { - protected DOMDataBrokerImpl domBroker; + protected SerializedDOMDataBroker domBroker; protected void initTestNode() throws Exception { final YangInstanceIdentifier testPath = YangInstanceIdentifier.builder(BenchmarkModel.TEST_PATH) diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRouterCodegenInstance.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRouterCodegenInstance.java index 783e5c0cd4..d69aeed352 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRouterCodegenInstance.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRouterCodegenInstance.java @@ -8,11 +8,14 @@ package org.opendaylight.controller.sal.binding.codegen.impl; import static org.opendaylight.controller.sal.binding.codegen.RuntimeCodeHelper.setRoutingTable; - +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.Set; - +import javax.annotation.concurrent.GuardedBy; import org.opendaylight.controller.md.sal.common.api.routing.RouteChange; import org.opendaylight.controller.md.sal.common.api.routing.RouteChangeListener; import org.opendaylight.controller.sal.binding.api.BindingAwareBroker.RoutedRpcRegistration; @@ -29,9 +32,6 @@ import org.opendaylight.yangtools.yang.binding.RpcService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; - public class RpcRouterCodegenInstance implements // RpcRouter, RouteChangeListener, InstanceIdentifier> { @@ -133,7 +133,14 @@ RpcRouter, RouteChangeListener, InstanceIdentif return new DefaultRpcImplementationRegistration(service); } - private class RoutedRpcRegistrationImpl extends AbstractObjectRegistration implements RoutedRpcRegistration { + private final class RoutedRpcRegistrationImpl extends AbstractObjectRegistration implements RoutedRpcRegistration { + /* + * FIXME: retaining this collection is not completely efficient. We really should be storing + * a reference to this registration, as a particular listener may be registered multiple + * times -- and then this goes kaboom in various aspects. + */ + @GuardedBy("this") + private final Collection> contexts = new ArrayList<>(1); public RoutedRpcRegistrationImpl(final T instance) { super(instance); @@ -145,32 +152,49 @@ RpcRouter, RouteChangeListener, InstanceIdentif } @Override - public void registerPath(final Class context, final InstanceIdentifier path) { + public synchronized void registerPath(final Class context, final InstanceIdentifier path) { + if (isClosed()) { + LOG.debug("Closed registration of {} ignoring new path {}", getInstance(), path); + return; + } + routingTables.get(context).updateRoute(path, getInstance()); + contexts.add(context); } @Override - public void unregisterPath(final Class context, final InstanceIdentifier path) { + public synchronized void unregisterPath(final Class context, final InstanceIdentifier path) { + if (isClosed()) { + LOG.debug("Closed unregistration of {} ignoring new path {}", getInstance(), path); + return; + } + routingTables.get(context).removeRoute(path, getInstance()); + contexts.remove(context); } + @Deprecated @Override public void registerInstance(final Class context, final InstanceIdentifier instance) { registerPath(context, instance); } + @Deprecated @Override public void unregisterInstance(final Class context, final InstanceIdentifier instance) { unregisterPath(context, instance); } @Override - protected void removeRegistration() { - + protected synchronized void removeRegistration() { + for (Class ctx : contexts) { + routingTables.get(ctx).removeAllReferences(getInstance()); + } + contexts.clear(); } } - private class DefaultRpcImplementationRegistration extends AbstractObjectRegistration implements RpcRegistration { + private final class DefaultRpcImplementationRegistration extends AbstractObjectRegistration implements RpcRegistration { protected DefaultRpcImplementationRegistration(final T instance) { diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRoutingTableImpl.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRoutingTableImpl.java index ce159b8f3e..78fa88bb1b 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRoutingTableImpl.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/RpcRoutingTableImpl.java @@ -8,10 +8,10 @@ package org.opendaylight.controller.sal.binding.codegen.impl; import java.util.Collections; +import java.util.Iterator; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; - import org.opendaylight.controller.md.sal.common.api.routing.RouteChangeListener; import org.opendaylight.controller.md.sal.common.api.routing.RouteChangePublisher; import org.opendaylight.controller.md.sal.common.impl.routing.RoutingUtils; @@ -25,8 +25,7 @@ import org.opendaylight.yangtools.yang.binding.RpcService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -class RpcRoutingTableImpl // -implements // +final class RpcRoutingTableImpl implements Mutable, // RpcRoutingTable, // RouteChangePublisher, InstanceIdentifier> { @@ -42,7 +41,7 @@ implements // private RouteChangeListener, InstanceIdentifier> listener; private S defaultRoute; - public RpcRoutingTableImpl(String routerName,Class contextType, Class serviceType) { + public RpcRoutingTableImpl(final String routerName,final Class contextType, final Class serviceType) { super(); this.routerName = routerName; this.serviceType = serviceType; @@ -52,7 +51,7 @@ implements // } @Override - public void setDefaultRoute(S target) { + public void setDefaultRoute(final S target) { defaultRoute = target; } @@ -63,7 +62,7 @@ implements // @Override public , InstanceIdentifier>> ListenerRegistration registerRouteChangeListener( - L listener) { + final L listener) { return new SingletonListenerRegistration(listener); } @@ -74,7 +73,7 @@ implements // @Override @SuppressWarnings("unchecked") - public void updateRoute(InstanceIdentifier path, S service) { + public void updateRoute(final InstanceIdentifier path, final S service) { S previous = this.routes.put(path, service); LOGGER.debug("Route {} updated to {} in routing table {}",path,service,this); @@ -88,7 +87,7 @@ implements // @Override @SuppressWarnings("unchecked") - public void removeRoute(InstanceIdentifier path) { + public void removeRoute(final InstanceIdentifier path) { S previous = this.routes.remove(path); LOGGER.debug("Route {} to {} removed in routing table {}",path,previous,this); @SuppressWarnings("rawtypes") @@ -98,7 +97,7 @@ implements // } } - public void removeRoute(InstanceIdentifier path, S service) { + void removeRoute(final InstanceIdentifier path, final S service) { @SuppressWarnings("rawtypes") RouteChangeListener listenerCapture = listener; if (routes.remove(path, service) && listenerCapture != null) { @@ -108,7 +107,7 @@ implements // } @Override - public S getRoute(InstanceIdentifier nodeInstance) { + public S getRoute(final InstanceIdentifier nodeInstance) { S route = routes.get(nodeInstance); if (route != null) { return route; @@ -121,25 +120,28 @@ implements // return unmodifiableRoutes; } - protected void removeAllReferences(S service) { - + void removeAllReferences(final S service) { + // FIXME: replace this via properly-synchronized BiMap (or something) + final Iterator it = routes.values().iterator(); + while (it.hasNext()) { + final S s = it.next(); + if (service.equals(s)) { + it.remove(); + } + } } - - @Override public String toString() { return "RpcRoutingTableImpl [router=" + routerName + ", service=" + serviceType.getSimpleName() + ", context=" + contextType.getSimpleName() + "]"; } - - private class SingletonListenerRegistration, InstanceIdentifier>> extends AbstractObjectRegistration implements ListenerRegistration { - public SingletonListenerRegistration(L instance) { + public SingletonListenerRegistration(final L instance) { super(instance); listener = instance; } diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/RpcProviderRegistryImpl.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/RpcProviderRegistryImpl.java index 13a9f1cc10..0949d3d761 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/RpcProviderRegistryImpl.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/RpcProviderRegistryImpl.java @@ -8,7 +8,13 @@ package org.opendaylight.controller.sal.binding.impl; import static com.google.common.base.Preconditions.checkState; - +import com.google.common.base.Preconditions; +import com.google.common.base.Throwables; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; +import com.google.common.util.concurrent.UncheckedExecutionException; import java.util.EventListener; import java.util.HashMap; import java.util.Map; @@ -17,7 +23,6 @@ import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicBoolean; - import org.opendaylight.controller.md.sal.common.api.routing.RouteChange; import org.opendaylight.controller.md.sal.common.api.routing.RouteChangeListener; import org.opendaylight.controller.md.sal.common.api.routing.RouteChangePublisher; @@ -40,13 +45,6 @@ import org.opendaylight.yangtools.yang.binding.RpcService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Throwables; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; -import com.google.common.util.concurrent.UncheckedExecutionException; - public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChangePublisher> { private RuntimeCodeGenerator rpcFactory = SingletonHolder.RPC_GENERATOR_IMPL; @@ -91,8 +89,7 @@ public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChange } @Override - public final RpcRegistration addRpcImplementation(final Class type, final T implementation) - throws IllegalStateException { + public final RpcRegistration addRpcImplementation(final Class type, final T implementation) { // FIXME: This should be well documented - addRpcImplementation for // routed RPCs @@ -222,11 +219,10 @@ public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChange } - private class RouteChangeForwarder implements RouteChangeListener, InstanceIdentifier> { - + private final class RouteChangeForwarder implements RouteChangeListener, InstanceIdentifier> { private final Class type; - public RouteChangeForwarder(final Class type) { + RouteChangeForwarder(final Class type) { this.type = type; } @@ -256,15 +252,14 @@ public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChange } } - public static class RpcProxyRegistration extends AbstractObjectRegistration implements RpcRegistration { - + private static final class RpcProxyRegistration extends AbstractObjectRegistration implements RpcRegistration { + private final RpcProviderRegistryImpl registry; private final Class serviceType; - private RpcProviderRegistryImpl registry; - public RpcProxyRegistration(final Class type, final T service, final RpcProviderRegistryImpl registry) { + RpcProxyRegistration(final Class type, final T service, final RpcProviderRegistryImpl registry) { super(service); + this.registry = Preconditions.checkNotNull(registry); this.serviceType = type; - this.registry = registry; } @Override @@ -274,13 +269,10 @@ public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChange @Override protected void removeRegistration() { - if (registry != null) { - T publicProxy = registry.getRpcService(serviceType); - RpcService currentDelegate = RuntimeCodeHelper.getDelegate(publicProxy); - if (currentDelegate == getInstance()) { - RuntimeCodeHelper.setDelegate(publicProxy, null); - } - registry = null; + T publicProxy = registry.getRpcService(serviceType); + RpcService currentDelegate = RuntimeCodeHelper.getDelegate(publicProxy); + if (currentDelegate == getInstance()) { + RuntimeCodeHelper.setDelegate(publicProxy, null); } } } diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DomToBindingRpcForwarder.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DomToBindingRpcForwarder.java index 6be5f2d481..9bff0e9609 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DomToBindingRpcForwarder.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DomToBindingRpcForwarder.java @@ -2,13 +2,11 @@ package org.opendaylight.controller.sal.binding.impl.connect.dom; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; - import com.google.common.base.Function; -import com.google.common.collect.FluentIterable; +import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; - import java.lang.ref.WeakReference; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; @@ -18,11 +16,10 @@ import java.util.Map; import java.util.Set; import java.util.WeakHashMap; import java.util.concurrent.Callable; - import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry; import org.opendaylight.controller.sal.binding.api.rpc.RpcRouter; import org.opendaylight.controller.sal.binding.impl.RpcProviderRegistryImpl; -import org.opendaylight.controller.sal.core.api.Broker; +import org.opendaylight.controller.sal.core.api.Broker.RoutedRpcRegistration; import org.opendaylight.controller.sal.core.api.RpcImplementation; import org.opendaylight.controller.sal.core.api.RpcProvisionRegistry; import org.opendaylight.yangtools.concepts.CompositeObjectRegistration; @@ -37,6 +34,7 @@ import org.opendaylight.yangtools.yang.binding.util.BindingReflections; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.common.RpcResult; import org.opendaylight.yangtools.yang.data.api.CompositeNode; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.impl.codec.BindingIndependentMappingService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,7 +45,7 @@ class DomToBindingRpcForwarder implements RpcImplementation, InvocationHandler { private final Set supportedRpcs; private final WeakReference> rpcServiceType; - private Set registrations; + private Set registrations; private final Map strategiesByQName = new HashMap<>(); private final WeakHashMap strategiesByMethod = new WeakHashMap<>(); private final RpcService proxy; @@ -58,7 +56,7 @@ class DomToBindingRpcForwarder implements RpcImplementation, InvocationHandler { private final RpcProviderRegistry baRpcRegistry; private final RpcProviderRegistryImpl baRpcRegistryImpl; - private final Function, org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier> toDOMInstanceIdentifier; + private final Function, YangInstanceIdentifier> toDOMInstanceIdentifier; private final static Method EQUALS_METHOD; @@ -75,10 +73,9 @@ class DomToBindingRpcForwarder implements RpcImplementation, InvocationHandler { this.rpcServiceType = new WeakReference>(service); this.supportedRpcs = mappingService.getRpcQNamesFor(service); - toDOMInstanceIdentifier = new Function, org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier>() { - + this.toDOMInstanceIdentifier = new Function, YangInstanceIdentifier>() { @Override - public org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier apply(final InstanceIdentifier input) { + public YangInstanceIdentifier apply(final InstanceIdentifier input) { return mappingService.toDataDom(input); } }; @@ -105,7 +102,7 @@ class DomToBindingRpcForwarder implements RpcImplementation, InvocationHandler { final RpcProvisionRegistry biRpcRegistry, final RpcProviderRegistry baRpcRegistry, final RpcProviderRegistryImpl registryImpl) { this(service, mappingService, biRpcRegistry, baRpcRegistry,registryImpl); - final ImmutableSet.Builder registrationsBuilder = ImmutableSet.builder(); + final ImmutableSet.Builder registrationsBuilder = ImmutableSet.builder(); try { for (QName rpc : supportedRpcs) { registrationsBuilder.add(biRpcRegistry.addRoutedRpcImplementation(rpc, this)); @@ -162,9 +159,8 @@ class DomToBindingRpcForwarder implements RpcImplementation, InvocationHandler { public void registerPaths(final Class context, final Class service, final Set> set) { QName ctx = BindingReflections.findQName(context); - for (org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier path : FluentIterable.from(set).transform( - toDOMInstanceIdentifier)) { - for (org.opendaylight.controller.sal.core.api.Broker.RoutedRpcRegistration reg : registrations) { + for (YangInstanceIdentifier path : Collections2.transform(set, toDOMInstanceIdentifier)) { + for (RoutedRpcRegistration reg : registrations) { reg.registerPath(ctx, path); } } @@ -188,9 +184,8 @@ class DomToBindingRpcForwarder implements RpcImplementation, InvocationHandler { public void removePaths(final Class context, final Class service, final Set> set) { QName ctx = BindingReflections.findQName(context); - for (org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier path : FluentIterable.from(set).transform( - toDOMInstanceIdentifier)) { - for (org.opendaylight.controller.sal.core.api.Broker.RoutedRpcRegistration reg : registrations) { + for (YangInstanceIdentifier path : Collections2.transform(set, toDOMInstanceIdentifier)) { + for (RoutedRpcRegistration reg : registrations) { reg.unregisterPath(ctx, path); } } diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DomToBindingRpcForwardingManager.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DomToBindingRpcForwardingManager.java index 63d4b71210..b6bc488c20 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DomToBindingRpcForwardingManager.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/connect/dom/DomToBindingRpcForwardingManager.java @@ -1,9 +1,10 @@ package org.opendaylight.controller.sal.binding.impl.connect.dom; +import com.google.common.base.Optional; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.WeakHashMap; - import org.opendaylight.controller.md.sal.common.api.routing.RouteChange; import org.opendaylight.controller.md.sal.common.api.routing.RouteChangeListener; import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry; @@ -18,8 +19,6 @@ import org.opendaylight.yangtools.yang.binding.RpcService; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.impl.codec.BindingIndependentMappingService; -import com.google.common.base.Optional; - /** * Manager responsible for instantiating forwarders responsible for * forwarding of RPC invocations from DOM Broker to Binding Aware Broker @@ -69,16 +68,21 @@ class DomToBindingRpcForwardingManager implements @Override public void onRouteChange(final RouteChange> change) { - for (Map.Entry>> entry : change.getAnnouncements().entrySet()) { - bindingRoutesAdded(entry); + // Process removals first + for (Entry>> entry : change.getRemovals().entrySet()) { + final Class context = entry.getKey().getRoutingContext(); + if (context != null) { + final Class service = entry.getKey().getRpcService(); + getRpcForwarder(service, context).removePaths(context, service, entry.getValue()); + } } - } - private void bindingRoutesAdded(final Map.Entry>> entry) { - Class context = entry.getKey().getRoutingContext(); - Class service = entry.getKey().getRpcService(); - if (context != null) { - getRpcForwarder(service, context).registerPaths(context, service, entry.getValue()); + for (Entry>> entry : change.getAnnouncements().entrySet()) { + final Class context = entry.getKey().getRoutingContext(); + if (context != null) { + final Class service = entry.getKey().getRpcService(); + getRpcForwarder(service, context).registerPaths(context, service, entry.getValue()); + } } } diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/raft/protobuff/client/messages/CompositeModificationByteStringPayload.java b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/raft/protobuff/client/messages/CompositeModificationByteStringPayload.java new file mode 100644 index 0000000000..99de5dde35 --- /dev/null +++ b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/raft/protobuff/client/messages/CompositeModificationByteStringPayload.java @@ -0,0 +1,119 @@ +/* + * Copyright (c) 2014 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.cluster.raft.protobuff.client.messages; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.protobuf.ByteString; +import com.google.protobuf.GeneratedMessage; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.UnknownFieldSet; +import java.io.IOException; +import java.io.Serializable; +import java.lang.ref.SoftReference; +import java.util.HashMap; +import java.util.Map; +import org.opendaylight.controller.protobuff.messages.cluster.raft.AppendEntriesMessages; +import org.opendaylight.controller.protobuff.messages.persistent.PersistentMessages; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class CompositeModificationByteStringPayload extends Payload implements + Serializable { + private static final long serialVersionUID = 1L; + + private ByteString byteString; + private SoftReference modificationReference; + private static final Logger LOG = LoggerFactory.getLogger(CompositeModificationByteStringPayload.class); + + public CompositeModificationByteStringPayload(){ + byteString = null; + } + public CompositeModificationByteStringPayload(Object modification){ + this(((PersistentMessages.CompositeModification) modification).toByteString()); + this.modificationReference = new SoftReference<>((PersistentMessages.CompositeModification) modification); + } + + private CompositeModificationByteStringPayload(ByteString byteString){ + this.byteString = Preconditions.checkNotNull(byteString, "byteString should not be null"); + } + + + @Override + public Map encode() { + Preconditions.checkState(byteString!=null); + Map map = new HashMap<>(); + map.put(org.opendaylight.controller.protobuff.messages.shard.CompositeModificationPayload.modification, + getModificationInternal()); + return map; + } + + @Override + public Payload decode( + AppendEntriesMessages.AppendEntries.ReplicatedLogEntry.Payload payload) { + PersistentMessages.CompositeModification modification = payload + .getExtension( + org.opendaylight.controller.protobuff.messages.shard.CompositeModificationPayload.modification); + + // The extension was put in the unknown field. + // This is because extensions need to be registered + // see org.opendaylight.controller.mdsal.CompositeModificationPayload.registerAllExtensions + // also see https://developers.google.com/protocol-buffers/docs/reference/java/com/google/protobuf/ExtensionRegistry + // If that is not done then on the other end the extension shows up as an unknown field + // Need to figure out a better way to do this + if(payload.getUnknownFields().hasField(2)){ + UnknownFieldSet.Field field = + payload.getUnknownFields().getField(2); + + return new CompositeModificationByteStringPayload(field.getLengthDelimitedList().get(0)); + } + + return new CompositeModificationByteStringPayload(modification); + } + + public Object getModification(){ + return getModificationInternal(); + } + + private PersistentMessages.CompositeModification getModificationInternal(){ + if(this.modificationReference != null && this.modificationReference.get() != null){ + return this.modificationReference.get(); + } + try { + PersistentMessages.CompositeModification compositeModification = PersistentMessages.CompositeModification.parseFrom(this.byteString); + this.modificationReference = new SoftReference<>(compositeModification); + return compositeModification; + } catch (InvalidProtocolBufferException e) { + LOG.error("Unexpected exception occurred when parsing byteString to CompositeModification", e); + } + + return null; + } + + public int size(){ + return byteString.size(); + } + + private void writeObject(java.io.ObjectOutputStream stream) + throws IOException { + byteString.writeTo(stream); + } + + private void readObject(java.io.ObjectInputStream stream) + throws IOException, ClassNotFoundException { + byteString = ByteString.readFrom(stream); + } + + @VisibleForTesting + public void clearModificationReference(){ + if(this.modificationReference != null) { + this.modificationReference.clear(); + } + } +} \ No newline at end of file diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java index a22e535fad..7d6dde9c8a 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java @@ -69,6 +69,7 @@ import org.opendaylight.controller.cluster.notifications.RoleChangeNotifier; import org.opendaylight.controller.cluster.raft.RaftActor; import org.opendaylight.controller.cluster.raft.ReplicatedLogEntry; import org.opendaylight.controller.cluster.raft.base.messages.CaptureSnapshotReply; +import org.opendaylight.controller.cluster.raft.protobuff.client.messages.CompositeModificationByteStringPayload; import org.opendaylight.controller.cluster.raft.protobuff.client.messages.CompositeModificationPayload; import org.opendaylight.controller.cluster.raft.protobuff.client.messages.Payload; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeListener; @@ -321,7 +322,7 @@ public class Shard extends RaftActor { cohortEntry.getCohort().preCommit().get(); Shard.this.persistData(getSender(), transactionID, - new CompositeModificationPayload(cohortEntry.getModification().toSerializable())); + new CompositeModificationByteStringPayload(cohortEntry.getModification().toSerializable())); } catch (InterruptedException | ExecutionException e) { LOG.error(e, "An exception occurred while preCommitting transaction {}", cohortEntry.getTransactionID()); @@ -679,6 +680,8 @@ public class Shard extends RaftActor { protected void appendRecoveredLogEntry(final Payload data) { if (data instanceof CompositeModificationPayload) { currentLogRecoveryBatch.add(((CompositeModificationPayload) data).getModification()); + } else if (data instanceof CompositeModificationByteStringPayload) { + currentLogRecoveryBatch.add(((CompositeModificationByteStringPayload) data).getModification()); } else { LOG.error("Unknown state received {} during recovery", data); } @@ -755,19 +758,12 @@ public class Shard extends RaftActor { if (data instanceof CompositeModificationPayload) { Object modification = ((CompositeModificationPayload) data).getModification(); - if(modification == null) { - LOG.error( - "modification is null - this is very unexpected, clientActor = {}, identifier = {}", - identifier, clientActor != null ? clientActor.path().toString() : null); - } else if(clientActor == null) { - // There's no clientActor to which to send a commit reply so we must be applying - // replicated state from the leader. - commitWithNewTransaction(MutableCompositeModification.fromSerializable( - modification, schemaContext)); - } else { - // This must be the OK to commit after replication consensus. - finishCommit(clientActor, identifier); - } + applyModificationToState(clientActor, identifier, modification); + } else if(data instanceof CompositeModificationByteStringPayload ){ + Object modification = ((CompositeModificationByteStringPayload) data).getModification(); + + applyModificationToState(clientActor, identifier, modification); + } else { LOG.error("Unknown state received {} Class loader = {} CompositeNodeMod.ClassLoader = {}", data, data.getClass().getClassLoader(), @@ -778,6 +774,22 @@ public class Shard extends RaftActor { } + private void applyModificationToState(ActorRef clientActor, String identifier, Object modification) { + if(modification == null) { + LOG.error( + "modification is null - this is very unexpected, clientActor = {}, identifier = {}", + identifier, clientActor != null ? clientActor.path().toString() : null); + } else if(clientActor == null) { + // There's no clientActor to which to send a commit reply so we must be applying + // replicated state from the leader. + commitWithNewTransaction(MutableCompositeModification.fromSerializable( + modification, schemaContext)); + } else { + // This must be the OK to commit after replication consensus. + finishCommit(clientActor, identifier); + } + } + private void updateJournalStats() { ReplicatedLogEntry lastLogEntry = getLastLogEntry(); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/CompositeModificationByteStringPayloadTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/CompositeModificationByteStringPayloadTest.java new file mode 100644 index 0000000000..db9f3d1801 --- /dev/null +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/CompositeModificationByteStringPayloadTest.java @@ -0,0 +1,83 @@ +/* + * Copyright (c) 2014 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.cluster.datastore; + +import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.assertTrue; +import java.util.ArrayList; +import java.util.List; +import org.apache.commons.lang.SerializationUtils; +import org.junit.Test; +import org.opendaylight.controller.cluster.datastore.modification.Modification; +import org.opendaylight.controller.cluster.datastore.modification.MutableCompositeModification; +import org.opendaylight.controller.cluster.datastore.modification.WriteModification; +import org.opendaylight.controller.cluster.raft.ReplicatedLogEntry; +import org.opendaylight.controller.cluster.raft.ReplicatedLogImplEntry; +import org.opendaylight.controller.cluster.raft.messages.AppendEntries; +import org.opendaylight.controller.cluster.raft.protobuff.client.messages.CompositeModificationByteStringPayload; +import org.opendaylight.controller.md.cluster.datastore.model.TestModel; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; + +public class CompositeModificationByteStringPayloadTest { + + private static final SchemaContext SCHEMA_CONTEXT = TestModel.createTestContext(); + + @Test + public void testSerialization(){ + WriteModification writeModification = + new WriteModification(TestModel.TEST_PATH, ImmutableNodes + .containerNode(TestModel.TEST_QNAME), + TestModel.createTestContext()); + + MutableCompositeModification compositeModification = + new MutableCompositeModification(); + + compositeModification.addModification(writeModification); + + CompositeModificationByteStringPayload compositeModificationByteStringPayload + = new CompositeModificationByteStringPayload(compositeModification.toSerializable()); + + byte[] bytes = SerializationUtils.serialize(compositeModificationByteStringPayload); + + Object deserialize = SerializationUtils.deserialize(bytes); + + assertTrue(deserialize instanceof CompositeModificationByteStringPayload); + + } + + @Test + public void testAppendEntries(){ + List entries = new ArrayList<>(); + + CompositeModificationByteStringPayload payload = newByteStringPayload( + new WriteModification(TestModel.OUTER_LIST_PATH, + ImmutableNodes.mapNodeBuilder(TestModel.OUTER_LIST_QNAME).build(), + SCHEMA_CONTEXT)); + + payload.clearModificationReference(); + + entries.add(new ReplicatedLogImplEntry(0, 1, payload)); + + + assertNotNull(new AppendEntries(10, "foobar", 10, 10, entries, 10).toSerializable()); + } + + + + private CompositeModificationByteStringPayload newByteStringPayload(final Modification... mods) { + MutableCompositeModification compMod = new MutableCompositeModification(); + for(Modification mod: mods) { + compMod.addModification(mod); + } + + return new CompositeModificationByteStringPayload(compMod.toSerializable()); + } + +} diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java index 926cef6ba5..2792342ab2 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java @@ -75,6 +75,7 @@ import org.opendaylight.controller.cluster.raft.base.messages.CaptureSnapshot; import org.opendaylight.controller.cluster.raft.base.messages.ElectionTimeout; import org.opendaylight.controller.cluster.raft.client.messages.FindLeader; import org.opendaylight.controller.cluster.raft.client.messages.FindLeaderReply; +import org.opendaylight.controller.cluster.raft.protobuff.client.messages.CompositeModificationByteStringPayload; import org.opendaylight.controller.cluster.raft.protobuff.client.messages.CompositeModificationPayload; import org.opendaylight.controller.cluster.raft.protobuff.client.messages.Payload; import org.opendaylight.controller.md.cluster.datastore.model.SchemaContextHelper; @@ -432,9 +433,9 @@ public class ShardTest extends AbstractActorTest { ImmutableNodes.mapNodeBuilder(TestModel.OUTER_LIST_QNAME).build(), SCHEMA_CONTEXT)))); - int nListEntries = 11; + int nListEntries = 16; Set listEntryKeys = new HashSet<>(); - for(int i = 1; i <= nListEntries; i++) { + for(int i = 1; i <= nListEntries-5; i++) { listEntryKeys.add(Integer.valueOf(i)); YangInstanceIdentifier path = YangInstanceIdentifier.builder(TestModel.OUTER_LIST_PATH) .nodeWithKey(TestModel.OUTER_LIST_QNAME, TestModel.ID_QNAME, i).build(); @@ -445,6 +446,19 @@ public class ShardTest extends AbstractActorTest { newPayload(mod))); } + // Add some of the new CompositeModificationByteStringPayload + for(int i = 11; i <= nListEntries; i++) { + listEntryKeys.add(Integer.valueOf(i)); + YangInstanceIdentifier path = YangInstanceIdentifier.builder(TestModel.OUTER_LIST_PATH) + .nodeWithKey(TestModel.OUTER_LIST_QNAME, TestModel.ID_QNAME, i).build(); + Modification mod = new MergeModification(path, + ImmutableNodes.mapEntry(TestModel.OUTER_LIST_QNAME, TestModel.ID_QNAME, i), + SCHEMA_CONTEXT); + InMemoryJournal.addEntry(shardID.toString(), i, new ReplicatedLogImplEntry(i, 1, + newByteStringPayload(mod))); + } + + InMemoryJournal.addEntry(shardID.toString(), nListEntries + 1, new ApplyLogEntries(nListEntries)); @@ -516,6 +530,16 @@ public class ShardTest extends AbstractActorTest { return new CompositeModificationPayload(compMod.toSerializable()); } + private CompositeModificationByteStringPayload newByteStringPayload(final Modification... mods) { + MutableCompositeModification compMod = new MutableCompositeModification(); + for(Modification mod: mods) { + compMod.addModification(mod); + } + + return new CompositeModificationByteStringPayload(compMod.toSerializable()); + } + + private DOMStoreThreePhaseCommitCohort setupMockWriteTransaction(final String cohortName, final InMemoryDOMDataStore dataStore, final YangInstanceIdentifier path, final NormalizedNode data, final MutableCompositeModification modification) { diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/DomBrokerImplModule.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/DomBrokerImplModule.java index f1f16cd635..d299afa057 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/DomBrokerImplModule.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/DomBrokerImplModule.java @@ -84,6 +84,7 @@ public final class DomBrokerImplModule extends org.opendaylight.controller.confi return new BrokerImpl(router, services); } + @Deprecated private DataProviderService createLegacyDataService(final DataStore legacyStore, final SchemaService schemaService) { YangInstanceIdentifier rootPath = YangInstanceIdentifier.builder().toInstance(); DataBrokerImpl dataService = new DataBrokerImpl(); diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java index 85134f1fd4..deddd6938a 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java @@ -244,7 +244,7 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype implements D // For debugging purposes, allow dumping of the modification. Coupled with the above // precondition log, it should allow us to understand what went on. - LOG.trace("Store Tx: {} modifications: {}", modification); + LOG.trace("Store Tx: {} modifications: {} tree: {}", modification, dataTree); return Futures.immediateFailedFuture(new TransactionCommitFailedException("Data did not pass validation.", e)); } catch (Exception e) { diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfDocumentedExceptionMapper.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfDocumentedExceptionMapper.java index 10201ab6f5..16b3ee6708 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfDocumentedExceptionMapper.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfDocumentedExceptionMapper.java @@ -16,7 +16,6 @@ import static org.opendaylight.controller.sal.rest.api.Draft02.RestConfModule.ER import static org.opendaylight.controller.sal.rest.api.Draft02.RestConfModule.ERROR_TAG_QNAME; import static org.opendaylight.controller.sal.rest.api.Draft02.RestConfModule.ERROR_TYPE_QNAME; import static org.opendaylight.controller.sal.rest.api.Draft02.RestConfModule.NAMESPACE; - import com.google.common.base.Charsets; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; @@ -69,6 +68,7 @@ import org.xml.sax.InputSource; public class RestconfDocumentedExceptionMapper implements ExceptionMapper { private final static Logger LOG = LoggerFactory.getLogger(RestconfDocumentedExceptionMapper.class); + private static final TransformerFactory TRANSFORMER_FACTORY = TransformerFactory.newInstance(); @Context private HttpHeaders headers; @@ -178,8 +178,7 @@ public class RestconfDocumentedExceptionMapper implements ExceptionMapper read(InputStream entityStream) throws XMLStreamException, diff --git a/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java b/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java index 5ed528e9bf..beb3365f1c 100644 --- a/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java +++ b/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java @@ -712,7 +712,7 @@ public class NetconfMappingTest extends AbstractConfigTest { private List getYangs() throws FileNotFoundException { List paths = Arrays.asList("/META-INF/yang/config.yang", "/META-INF/yang/rpc-context.yang", "/META-INF/yang/config-test.yang", "/META-INF/yang/config-test-impl.yang", "/META-INF/yang/test-types.yang", - "/META-INF/yang/ietf-inet-types.yang"); + "/META-INF/yang/test-groups.yang", "/META-INF/yang/ietf-inet-types.yang"); final Collection yangDependencies = new ArrayList<>(); for (String path : paths) { final InputStream is = Preconditions diff --git a/opendaylight/netconf/netconf-api/src/main/java/org/opendaylight/controller/netconf/api/NetconfDocumentedException.java b/opendaylight/netconf/netconf-api/src/main/java/org/opendaylight/controller/netconf/api/NetconfDocumentedException.java index e5f32653c5..e1e932b55a 100644 --- a/opendaylight/netconf/netconf-api/src/main/java/org/opendaylight/controller/netconf/api/NetconfDocumentedException.java +++ b/opendaylight/netconf/netconf-api/src/main/java/org/opendaylight/controller/netconf/api/NetconfDocumentedException.java @@ -45,6 +45,15 @@ public class NetconfDocumentedException extends Exception { static { BUILDER_FACTORY = DocumentBuilderFactory.newInstance(); + try { + BUILDER_FACTORY.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + BUILDER_FACTORY.setFeature("http://xml.org/sax/features/external-general-entities", false); + BUILDER_FACTORY.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + BUILDER_FACTORY.setXIncludeAware(false); + BUILDER_FACTORY.setExpandEntityReferences(false); + } catch (ParserConfigurationException e) { + throw new ExceptionInInitializerError(e); + } BUILDER_FACTORY.setNamespaceAware(true); BUILDER_FACTORY.setCoalescing(true); BUILDER_FACTORY.setIgnoringElementContentWhitespace(true); diff --git a/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/AbstractNetconfConfigTest.java b/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/AbstractNetconfConfigTest.java index a724d1d9c5..65810a6bda 100644 --- a/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/AbstractNetconfConfigTest.java +++ b/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/AbstractNetconfConfigTest.java @@ -187,6 +187,7 @@ public abstract class AbstractNetconfConfigTest extends AbstractConfigTest { "/META-INF/yang/config-test.yang", "/META-INF/yang/config-test-impl.yang", "/META-INF/yang/test-types.yang", + "/META-INF/yang/test-groups.yang", "/META-INF/yang/ietf-inet-types.yang"); final Collection yangDependencies = new ArrayList<>(); diff --git a/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/NetconfConfigPersisterITTest.java b/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/NetconfConfigPersisterITTest.java index cc170358dd..92c96d92f2 100644 --- a/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/NetconfConfigPersisterITTest.java +++ b/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/NetconfConfigPersisterITTest.java @@ -112,8 +112,8 @@ public class NetconfConfigPersisterITTest extends AbstractNetconfConfigTest { } notificationVerifier.assertNotificationCount(2); - notificationVerifier.assertNotificationContent(0, 0, 0, 8); - notificationVerifier.assertNotificationContent(1, 4, 3, 8); + notificationVerifier.assertNotificationContent(0, 0, 0, 9); + notificationVerifier.assertNotificationContent(1, 4, 3, 9); mockedAggregator.assertSnapshotCount(2); // Capabilities are stripped for persister diff --git a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXICodec.java b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXICodec.java index 98baef0f85..30867bcd18 100644 --- a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXICodec.java +++ b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXICodec.java @@ -8,6 +8,9 @@ import org.openexi.proc.common.GrammarOptions; import org.openexi.proc.grammars.GrammarCache; import org.openexi.sax.EXIReader; import org.openexi.sax.Transmogrifier; +import org.openexi.sax.TransmogrifierException; +import org.xml.sax.EntityResolver; +import org.xml.sax.InputSource; public final class NetconfEXICodec { /** @@ -16,6 +19,17 @@ public final class NetconfEXICodec { * of the stream. This is really useful, so let's output it now. */ private static final boolean OUTPUT_EXI_COOKIE = true; + /** + * OpenEXI does not allow us to directly prevent resolution of external entities. In order + * to prevent XXE attacks, we reuse a single no-op entity resolver. + */ + private static final EntityResolver ENTITY_RESOLVER = new EntityResolver() { + @Override + public InputSource resolveEntity(final String publicId, final String systemId) { + return new InputSource(); + } + }; + private final EXIOptions exiOptions; public NetconfEXICodec(final EXIOptions exiOptions) { @@ -44,16 +58,18 @@ public final class NetconfEXICodec { final EXIReader r = new EXIReader(); r.setPreserveLexicalValues(exiOptions.getPreserveLexicalValues()); r.setGrammarCache(getGrammarCache()); + r.setEntityResolver(ENTITY_RESOLVER); return r; } - Transmogrifier getTransmogrifier() throws EXIOptionsException { + Transmogrifier getTransmogrifier() throws EXIOptionsException, TransmogrifierException { final Transmogrifier transmogrifier = new Transmogrifier(); transmogrifier.setAlignmentType(exiOptions.getAlignmentType()); transmogrifier.setBlockSize(exiOptions.getBlockSize()); transmogrifier.setGrammarCache(getGrammarCache()); transmogrifier.setOutputCookie(OUTPUT_EXI_COOKIE); transmogrifier.setOutputOptions(HeaderOptionsOutputType.all); + transmogrifier.setResolveExternalGeneralEntities(false); return transmogrifier; } } diff --git a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java index 55dcd9daba..e90bc7916d 100644 --- a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java +++ b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java @@ -14,22 +14,18 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.MessageToByteEncoder; import java.io.IOException; import java.io.OutputStream; -import javax.xml.transform.Transformer; import javax.xml.transform.TransformerException; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.sax.SAXResult; -import javax.xml.transform.sax.SAXTransformerFactory; import org.opendaylight.controller.netconf.api.NetconfMessage; import org.openexi.proc.common.EXIOptionsException; import org.openexi.sax.Transmogrifier; +import org.openexi.sax.TransmogrifierException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public final class NetconfMessageToEXIEncoder extends MessageToByteEncoder { - private static final Logger LOG = LoggerFactory.getLogger(NetconfMessageToEXIEncoder.class); - - private static final SAXTransformerFactory saxTransformerFactory = (SAXTransformerFactory)SAXTransformerFactory.newInstance(); private final NetconfEXICodec codec; public NetconfMessageToEXIEncoder(final NetconfEXICodec codec) { @@ -37,15 +33,14 @@ public final class NetconfMessageToEXIEncoder extends MessageToByteEncoder { private static final Logger LOG = LoggerFactory.getLogger(NetconfMessageToXMLEncoder.class); - private static final TransformerFactory FACTORY = TransformerFactory.newInstance(); private final Optional clientId; @@ -38,13 +34,13 @@ public class NetconfMessageToXMLEncoder extends MessageToByteEncoderabsent()); } - public NetconfMessageToXMLEncoder(Optional clientId) { + public NetconfMessageToXMLEncoder(final Optional clientId) { this.clientId = clientId; } @Override @VisibleForTesting - public void encode(ChannelHandlerContext ctx, NetconfMessage msg, ByteBuf out) throws IOException, TransformerException { + public void encode(final ChannelHandlerContext ctx, final NetconfMessage msg, final ByteBuf out) throws IOException, TransformerException { LOG.trace("Sent to encode : {}", msg); if (clientId.isPresent()) { @@ -53,14 +49,10 @@ public class NetconfMessageToXMLEncoder extends MessageToByteEncoder DEFAULT_TRANSFORMER = new ThreadLocal() { + @Override + protected Transformer initialValue() { + try { + return FACTORY.newTransformer(); + } catch (TransformerConfigurationException | TransformerFactoryConfigurationError e) { + throw new IllegalStateException("Unexpected error while instantiating a Transformer", e); + } + }; + + @Override + public void set(final Transformer value) { + throw new UnsupportedOperationException(); + }; + }; + + private static final ThreadLocal PRETTY_TRANSFORMER = new ThreadLocal() { + @Override + protected Transformer initialValue() { + final Transformer ret; + + try { + ret = FACTORY.newTransformer(); + } catch (TransformerConfigurationException | TransformerFactoryConfigurationError e) { + throw new IllegalStateException("Unexpected error while instantiating a Transformer", e); + } + + ret.setOutputProperty(OutputKeys.INDENT, "yes"); + ret.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes"); + return ret; + }; + + @Override + public void set(final Transformer value) { + throw new UnsupportedOperationException(); + }; + }; + + private ThreadLocalTransformers() { + throw new UnsupportedOperationException("Utility class"); + } + + /** + * Get the transformer with default configuration. + * + * @return A transformer with default configuration based on the default implementation. + */ + public static Transformer getDefaultTransformer() { + return DEFAULT_TRANSFORMER.get(); + } + + /** + * Get the transformer with default configuration, but with automatic indentation + * and the XML declaration removed. + * + * @return A transformer with human-friendly configuration. + */ + public static Transformer getPrettyTransformer() { + return PRETTY_TRANSFORMER.get(); + } +} diff --git a/opendaylight/netconf/netconf-testtool/src/main/java/org/opendaylight/controller/netconf/test/tool/NetconfDeviceSimulator.java b/opendaylight/netconf/netconf-testtool/src/main/java/org/opendaylight/controller/netconf/test/tool/NetconfDeviceSimulator.java index c6cad90355..287ff2dca7 100644 --- a/opendaylight/netconf/netconf-testtool/src/main/java/org/opendaylight/controller/netconf/test/tool/NetconfDeviceSimulator.java +++ b/opendaylight/netconf/netconf-testtool/src/main/java/org/opendaylight/controller/netconf/test/tool/NetconfDeviceSimulator.java @@ -200,7 +200,7 @@ public class NetconfDeviceSimulator implements Closeable { server = dispatcher.createLocalServer(tcpLocalAddress); try { final SshProxyServer sshServer = new SshProxyServer(minaTimerExecutor, nettyThreadgroup, nioExecutor); - sshServer.bind(getSshConfiguration(bindingAddress, tcpLocalAddress)); + sshServer.bind(getSshConfiguration(bindingAddress, tcpLocalAddress, keyPairProvider)); sshWrappers.add(sshServer); } catch (final BindException e) { LOG.warn("Cannot start simulated device on {}, port already in use. Skipping.", address); @@ -259,7 +259,7 @@ public class NetconfDeviceSimulator implements Closeable { return openDevices; } - private SshProxyServerConfiguration getSshConfiguration(final InetSocketAddress bindingAddress, final LocalAddress tcpLocalAddress) throws IOException { + private SshProxyServerConfiguration getSshConfiguration(final InetSocketAddress bindingAddress, final LocalAddress tcpLocalAddress, final PEMGeneratorHostKeyProvider keyPairProvider) throws IOException { return new SshProxyServerConfigurationBuilder() .setBindingAddress(bindingAddress) .setLocalAddress(tcpLocalAddress) @@ -269,7 +269,7 @@ public class NetconfDeviceSimulator implements Closeable { return true; } }) - .setKeyPairProvider(new PEMGeneratorHostKeyProvider(Files.createTempFile("prefix", "suffix").toAbsolutePath().toString())) + .setKeyPairProvider(keyPairProvider) .setIdleTimeout(Integer.MAX_VALUE) .createSshProxyServerConfiguration(); } diff --git a/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/xml/XMLNetconfUtil.java b/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/xml/XMLNetconfUtil.java index eaaf320b02..6569dba714 100644 --- a/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/xml/XMLNetconfUtil.java +++ b/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/xml/XMLNetconfUtil.java @@ -8,6 +8,7 @@ package org.opendaylight.controller.netconf.util.xml; +import javax.xml.namespace.NamespaceContext; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathExpression; import javax.xml.xpath.XPathExpressionException; @@ -15,14 +16,17 @@ import javax.xml.xpath.XPathFactory; import org.opendaylight.controller.netconf.api.xml.XmlNetconfConstants; public final class XMLNetconfUtil { + private static final XPathFactory FACTORY = XPathFactory.newInstance(); + private static final NamespaceContext NS_CONTEXT = new HardcodedNamespaceResolver("netconf", + XmlNetconfConstants.URN_IETF_PARAMS_XML_NS_NETCONF_BASE_1_0); - private XMLNetconfUtil() {} + private XMLNetconfUtil() { + throw new UnsupportedOperationException("Utility class"); + } - public static XPathExpression compileXPath(String xPath) { - final XPathFactory xPathfactory = XPathFactory.newInstance(); - final XPath xpath = xPathfactory.newXPath(); - xpath.setNamespaceContext(new HardcodedNamespaceResolver("netconf", - XmlNetconfConstants.URN_IETF_PARAMS_XML_NS_NETCONF_BASE_1_0)); + public static XPathExpression compileXPath(final String xPath) { + final XPath xpath = FACTORY.newXPath(); + xpath.setNamespaceContext(NS_CONTEXT); try { return xpath.compile(xPath); } catch (final XPathExpressionException e) { diff --git a/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/xml/XmlUtil.java b/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/xml/XmlUtil.java index 68c4d9fdab..4ae65f31f7 100644 --- a/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/xml/XmlUtil.java +++ b/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/xml/XmlUtil.java @@ -43,40 +43,53 @@ public final class XmlUtil { public static final String XMLNS_ATTRIBUTE_KEY = "xmlns"; public static final String XMLNS_URI = "http://www.w3.org/2000/xmlns/"; - private static final DocumentBuilderFactory BUILDERFACTORY; + private static final DocumentBuilderFactory BUILDER_FACTORY; + private static final TransformerFactory TRANSFORMER_FACTORY = TransformerFactory.newInstance(); + private static final SchemaFactory SCHEMA_FACTORY = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); static { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + try { + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setXIncludeAware(false); + factory.setExpandEntityReferences(false); + } catch (ParserConfigurationException e) { + throw new ExceptionInInitializerError(e); + } factory.setNamespaceAware(true); factory.setCoalescing(true); factory.setIgnoringElementContentWhitespace(true); factory.setIgnoringComments(true); - BUILDERFACTORY = factory; + BUILDER_FACTORY = factory; } - private XmlUtil() {} + private XmlUtil() { + throw new UnsupportedOperationException("Utility class"); + } - public static Element readXmlToElement(String xmlContent) throws SAXException, IOException { + public static Element readXmlToElement(final String xmlContent) throws SAXException, IOException { Document doc = readXmlToDocument(xmlContent); return doc.getDocumentElement(); } - public static Element readXmlToElement(InputStream xmlContent) throws SAXException, IOException { + public static Element readXmlToElement(final InputStream xmlContent) throws SAXException, IOException { Document doc = readXmlToDocument(xmlContent); return doc.getDocumentElement(); } - public static Document readXmlToDocument(String xmlContent) throws SAXException, IOException { + public static Document readXmlToDocument(final String xmlContent) throws SAXException, IOException { return readXmlToDocument(new ByteArrayInputStream(xmlContent.getBytes(Charsets.UTF_8))); } // TODO improve exceptions throwing // along with XmlElement - public static Document readXmlToDocument(InputStream xmlContent) throws SAXException, IOException { + public static Document readXmlToDocument(final InputStream xmlContent) throws SAXException, IOException { DocumentBuilder dBuilder; try { - dBuilder = BUILDERFACTORY.newDocumentBuilder(); + dBuilder = BUILDER_FACTORY.newDocumentBuilder(); } catch (ParserConfigurationException e) { throw new IllegalStateException("Failed to parse XML document", e); } @@ -86,20 +99,20 @@ public final class XmlUtil { return doc; } - public static Element readXmlToElement(File xmlFile) throws SAXException, IOException { + public static Element readXmlToElement(final File xmlFile) throws SAXException, IOException { return readXmlToDocument(new FileInputStream(xmlFile)).getDocumentElement(); } public static Document newDocument() { try { - DocumentBuilder builder = BUILDERFACTORY.newDocumentBuilder(); + DocumentBuilder builder = BUILDER_FACTORY.newDocumentBuilder(); return builder.newDocument(); } catch (ParserConfigurationException e) { throw new IllegalStateException("Failed to create document", e); } } - public static Element createElement(final Document document, String qName, Optional namespaceURI) { + public static Element createElement(final Document document, final String qName, final Optional namespaceURI) { if(namespaceURI.isPresent()) { final Element element = document.createElementNS(namespaceURI.get(), qName); String name = XMLNS_ATTRIBUTE_KEY; @@ -112,20 +125,20 @@ public final class XmlUtil { return document.createElement(qName); } - public static Element createTextElement(Document document, String qName, String content, Optional namespaceURI) { + public static Element createTextElement(final Document document, final String qName, final String content, final Optional namespaceURI) { Element typeElement = createElement(document, qName, namespaceURI); typeElement.appendChild(document.createTextNode(content)); return typeElement; } - public static Element createTextElementWithNamespacedContent(Document document, String qName, String prefix, - String namespace, String contentWithoutPrefix) { + public static Element createTextElementWithNamespacedContent(final Document document, final String qName, final String prefix, + final String namespace, final String contentWithoutPrefix) { return createTextElementWithNamespacedContent(document, qName, prefix, namespace, contentWithoutPrefix, Optional.absent()); } - public static Element createTextElementWithNamespacedContent(Document document, String qName, String prefix, - String namespace, String contentWithoutPrefix, Optional namespaceURI) { + public static Element createTextElementWithNamespacedContent(final Document document, final String qName, final String prefix, + final String namespace, final String contentWithoutPrefix, final Optional namespaceURI) { String content = createPrefixedValue(XmlNetconfConstants.PREFIX, contentWithoutPrefix); Element element = createTextElement(document, qName, content, namespaceURI); @@ -134,25 +147,25 @@ public final class XmlUtil { return element; } - public static String createPrefixedValue(String prefix, String value) { + public static String createPrefixedValue(final String prefix, final String value) { return prefix + ":" + value; } - public static String toString(Document document) { + public static String toString(final Document document) { return toString(document.getDocumentElement()); } - public static String toString(Element xml) { + public static String toString(final Element xml) { return toString(xml, false); } - public static String toString(XmlElement xmlElement) { + public static String toString(final XmlElement xmlElement) { return toString(xmlElement.getDomElement(), false); } - public static String toString(Element xml, boolean addXmlDeclaration) { + public static String toString(final Element xml, final boolean addXmlDeclaration) { try { - Transformer transformer = TransformerFactory.newInstance().newTransformer(); + Transformer transformer = TRANSFORMER_FACTORY.newTransformer(); transformer.setOutputProperty(OutputKeys.INDENT, "yes"); transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, addXmlDeclaration ? "no" : "yes"); @@ -166,26 +179,25 @@ public final class XmlUtil { } } - public static String toString(Document doc, boolean addXmlDeclaration) { + public static String toString(final Document doc, final boolean addXmlDeclaration) { return toString(doc.getDocumentElement(), addXmlDeclaration); } - public static Schema loadSchema(InputStream... fromStreams) { + public static Schema loadSchema(final InputStream... fromStreams) { Source[] sources = new Source[fromStreams.length]; int i = 0; for (InputStream stream : fromStreams) { sources[i++] = new StreamSource(stream); } - final SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); try { - return schemaFactory.newSchema(sources); + return SCHEMA_FACTORY.newSchema(sources); } catch (SAXException e) { throw new IllegalStateException("Failed to instantiate XML schema", e); } } - public static Object evaluateXPath(XPathExpression expr, Object rootNode, QName returnType) { + public static Object evaluateXPath(final XPathExpression expr, final Object rootNode, final QName returnType) { try { return expr.evaluate(rootNode, returnType); } catch (XPathExpressionException e) { @@ -193,7 +205,7 @@ public final class XmlUtil { } } - public static Document createDocumentCopy(Document original) { + public static Document createDocumentCopy(final Document original) { final Document copiedDocument = newDocument(); final Node copiedRoot = copiedDocument.importNode(original.getDocumentElement(), true); copiedDocument.appendChild(copiedRoot); diff --git a/opendaylight/netconf/netconf-util/src/test/java/org/opendaylight/controller/netconf/util/xml/XmlUtilTest.java b/opendaylight/netconf/netconf-util/src/test/java/org/opendaylight/controller/netconf/util/xml/XmlUtilTest.java index 3796dd996a..79aa565df9 100644 --- a/opendaylight/netconf/netconf-util/src/test/java/org/opendaylight/controller/netconf/util/xml/XmlUtilTest.java +++ b/opendaylight/netconf/netconf-util/src/test/java/org/opendaylight/controller/netconf/util/xml/XmlUtilTest.java @@ -61,6 +61,18 @@ public class XmlUtilTest { } + @Test(expected = SAXParseException.class) + public void testXXEFlaw() throws Exception { + XmlUtil.readXmlToDocument("\n" + + "]>\n" + + "\n" + + " \n" + + " urn:ietf:params:netconf:base:1.0 &xxe;\n" + + " \n" + + " ]]>]]>"); + } + @Test public void testXPath() throws Exception { final XPathExpression correctXPath = XMLNetconfUtil.compileXPath("/top/innerText");