From a0128f82bcc2a3ac2f8611049f7c9ab4dfeda985 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 24 Oct 2021 14:17:27 +0200 Subject: [PATCH] Cleanup AbstractQueryParams We have a chunk of dead code here, which would be visible if we had the proper structure. XPath checking is being done in a completely different place now. JIRA: NETCONF-773 Change-Id: I7a79deeb9108d3374663b4e83135e7ddc00a398c Signed-off-by: Robert Varga --- .../listeners/AbstractQueryParams.java | 70 ++----------------- .../streams/listeners/ListenerAdapter.java | 12 ++-- .../NotificationListenerAdapter.java | 12 ++-- 3 files changed, 17 insertions(+), 77 deletions(-) diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/AbstractQueryParams.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/AbstractQueryParams.java index ab644b406f..1e7ad2b5df 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/AbstractQueryParams.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/AbstractQueryParams.java @@ -10,47 +10,16 @@ package org.opendaylight.restconf.nb.rfc8040.streams.listeners; import static java.util.Objects.requireNonNull; import com.google.common.annotations.VisibleForTesting; -import java.io.StringReader; import java.time.Instant; -import javax.xml.XMLConstants; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.xpath.XPath; -import javax.xml.xpath.XPathConstants; -import javax.xml.xpath.XPathFactory; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; -import org.w3c.dom.Document; -import org.xml.sax.InputSource; /** * Features of query parameters part of both notifications. */ abstract class AbstractQueryParams extends AbstractNotificationsData { - // FIXME: BUG-7956: switch to using UntrustedXML - private static final DocumentBuilderFactory DBF; - - static { - final DocumentBuilderFactory f = DocumentBuilderFactory.newInstance(); - f.setCoalescing(true); - f.setExpandEntityReferences(false); - f.setIgnoringElementContentWhitespace(true); - f.setIgnoringComments(true); - f.setXIncludeAware(false); - try { - f.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - f.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - f.setFeature("http://xml.org/sax/features/external-general-entities", false); - f.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - } catch (final ParserConfigurationException e) { - throw new ExceptionInInitializerError(e); - } - DBF = f; - } - // FIXME: these should be final private Instant startTime = null; private Instant stopTime = null; - private String filter = null; private boolean leafNodesOnly = false; private boolean skipNotificationData = false; @@ -67,12 +36,14 @@ abstract class AbstractQueryParams extends AbstractNotificationsData { * @param filter Indicates which subset of all possible events are of interest. * @param leafNodesOnly If TRUE, notifications will contain changes of leaf nodes only. */ + public abstract void setQueryParams(Instant startTime, Instant stopTime, String filter, + boolean leafNodesOnly, boolean skipNotificationData); + @SuppressWarnings("checkstyle:hiddenField") - public void setQueryParams(final Instant startTime, final Instant stopTime, final String filter, - final boolean leafNodesOnly, final boolean skipNotificationData) { + final void setQueryParams(final Instant startTime, final Instant stopTime, final boolean leafNodesOnly, + final boolean skipNotificationData) { this.startTime = requireNonNull(startTime); this.stopTime = stopTime; - this.filter = filter; this.leafNodesOnly = leafNodesOnly; this.skipNotificationData = skipNotificationData; } @@ -118,35 +89,4 @@ abstract class AbstractQueryParams extends AbstractNotificationsData { } return false; } - - /** - * Check if is filter used and then prepare and post data do client. - * - * @param xml XML data of notification. - */ - @SuppressWarnings("checkstyle:IllegalCatch") - // FIXME: this method is never called, have we lost functionality compared to bierman02? - boolean checkFilter(final String xml) { - if (filter == null) { - return true; - } - try { - return parseFilterParam(xml); - } catch (final Exception e) { - throw new RestconfDocumentedException("Problem while parsing filter.", e); - } - } - - /** - * Parse and evaluate filter statement by XML format. - * - * @return {@code true} or {@code false} depending on filter expression and data of notification. - * @throws Exception If operation fails. - */ - private boolean parseFilterParam(final String xml) throws Exception { - final Document docOfXml = DBF.newDocumentBuilder().parse(new InputSource(new StringReader(xml))); - final XPath xPath = XPathFactory.newInstance().newXPath(); - // FIXME: BUG-7956: xPath.setNamespaceContext(nsContext); - return (boolean) xPath.compile(filter).evaluate(docOfXml, XPathConstants.BOOLEAN); - } } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/ListenerAdapter.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/ListenerAdapter.java index 7cc6a90d87..416a4c6271 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/ListenerAdapter.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/ListenerAdapter.java @@ -84,11 +84,11 @@ public class ListenerAdapter extends AbstractCommonSubscriber implements Cluster } @Override - public void setQueryParams(final Instant start, final Instant stop, final String filter, + public void setQueryParams(final Instant startTime, final Instant stopTime, final String filter, final boolean leafNodesOnly, final boolean skipNotificationData) { - super.setQueryParams(start, stop, filter, leafNodesOnly, skipNotificationData); + setQueryParams(startTime, stopTime, leafNodesOnly, skipNotificationData); try { - this.formatter = getFormatter(filter); + formatter = getFormatter(filter); } catch (final XPathExpressionException e) { throw new IllegalArgumentException("Failed to get filter", e); } @@ -129,12 +129,12 @@ public class ListenerAdapter extends AbstractCommonSubscriber implements Cluster */ @Override public String getStreamName() { - return this.streamName; + return streamName; } @Override public String getOutputType() { - return this.outputType.getName(); + return outputType.getName(); } /** @@ -143,7 +143,7 @@ public class ListenerAdapter extends AbstractCommonSubscriber implements Cluster * @return Path pointed to data in data store. */ public YangInstanceIdentifier getPath() { - return this.path; + return path; } /** diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/NotificationListenerAdapter.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/NotificationListenerAdapter.java index 806ee31d47..ce76d77ed5 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/NotificationListenerAdapter.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/NotificationListenerAdapter.java @@ -58,7 +58,7 @@ public class NotificationListenerAdapter extends AbstractCommonSubscriber implem this.path = requireNonNull(path); this.streamName = requireNonNull(streamName); checkArgument(!streamName.isEmpty()); - this.formatter = getFormatterFactory().getFormatter(); + formatter = getFormatterFactory().getFormatter(); LOG.debug("output type: {}, {}", outputType, this.outputType); } @@ -82,9 +82,9 @@ public class NotificationListenerAdapter extends AbstractCommonSubscriber implem @Override public void setQueryParams(final Instant startTime, final Instant stopTime, final String filter, final boolean leafNodesOnly, final boolean skipNotificationData) { - super.setQueryParams(startTime, stopTime, filter, leafNodesOnly, skipNotificationData); + setQueryParams(startTime, stopTime, leafNodesOnly, skipNotificationData); try { - this.formatter = getFormatter(filter); + formatter = getFormatter(filter); } catch (XPathExpressionException e) { throw new IllegalArgumentException("Failed to get filter", e); } @@ -97,7 +97,7 @@ public class NotificationListenerAdapter extends AbstractCommonSubscriber implem */ @Override public String getOutputType() { - return this.outputType.getName(); + return outputType.getName(); } @Override @@ -128,7 +128,7 @@ public class NotificationListenerAdapter extends AbstractCommonSubscriber implem */ @Override public String getStreamName() { - return this.streamName; + return streamName; } /** @@ -137,7 +137,7 @@ public class NotificationListenerAdapter extends AbstractCommonSubscriber implem * @return The configured schema path that points to observing YANG notification schema node. */ public Absolute getSchemaPath() { - return this.path; + return path; } public final synchronized void listen(final DOMNotificationService notificationService) { -- 2.36.6