From efe35f12ce37107c814a5d8bb245e5f9ed9ba073 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 25 Apr 2023 22:13:01 +0200 Subject: [PATCH] Clean up NotificationMessage Use java.time.Instant to carry an immutable event time -- addressing two SpotBugs warnings and making integration with DOMEvent/EventInstantAware much mode sealmess. JIRA: NETCONF-1000 Change-Id: I043c21abf7eb96368acfd73a6fddbc036e3ed7d3 Signed-off-by: Robert Varga --- .../notification/impl/CreateSubscription.java | 5 +- .../impl/NotificationsTransformUtil.java | 9 +--- .../impl/NetconfNotificationManagerTest.java | 12 ++--- .../impl/NotificationsTransformUtilTest.java | 10 ++-- .../util/NetconfMessageTransformUtil.java | 4 +- .../NetconfNestedNotificationTest.java | 2 +- .../netconf/NetconfToNotificationTest.java | 2 +- .../api/messages/NotificationMessage.java | 47 ++++++++----------- .../api/messages/NotificationMessageTest.java | 10 ++-- 9 files changed, 41 insertions(+), 60 deletions(-) diff --git a/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscription.java b/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscription.java index 697c81f55f..fc6db7fe8a 100644 --- a/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscription.java +++ b/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscription.java @@ -11,7 +11,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; import java.util.ArrayList; -import java.util.Date; import java.util.List; import java.util.Optional; import org.opendaylight.netconf.api.DocumentedException; @@ -139,8 +138,8 @@ public class CreateSubscription extends AbstractSingletonNetconfOperation final Optional filtered = SubtreeFilter.applySubtreeNotificationFilter(filter, notification.getDocument()); if (filtered.isPresent()) { - final Date eventTime = notification.getEventTime(); - currentSession.sendMessage(new NotificationMessage(filtered.orElseThrow(), eventTime)); + currentSession.sendMessage(new NotificationMessage(filtered.orElseThrow(), + notification.getEventTime())); } } catch (DocumentedException e) { LOG.warn("Failed to process notification {}", notification, e); diff --git a/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationsTransformUtil.java b/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationsTransformUtil.java index 26eac2aa90..9c71f4430b 100644 --- a/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationsTransformUtil.java +++ b/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationsTransformUtil.java @@ -10,11 +10,9 @@ package org.opendaylight.netconf.mdsal.notification.impl; import static com.google.common.base.Verify.verify; import java.io.IOException; -import java.util.Date; import javax.xml.stream.XMLStreamException; import javax.xml.transform.dom.DOMResult; import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.mdsal.binding.dom.codec.api.BindingNormalizedNodeSerializer; import org.opendaylight.mdsal.binding.dom.codec.spi.BindingDOMCodecFactory; import org.opendaylight.mdsal.binding.runtime.api.BindingRuntimeGenerator; @@ -54,12 +52,7 @@ public final class NotificationsTransformUtil { * Transform base notification for capabilities into NetconfNotification. */ public @NonNull NotificationMessage transform(final Notification notification, final Absolute path) { - return transform(notification, - notification instanceof EventInstantAware aware ? Date.from(aware.eventInstant()) : null, path); - } - - public @NonNull NotificationMessage transform(final Notification notification, final @Nullable Date eventTime, - final Absolute path) { + final var eventTime = notification instanceof EventInstantAware aware ? aware.eventInstant() : null; final var containerNode = serializer.toNormalizedNodeNotification(notification); final var result = new DOMResult(XmlUtil.newDocument()); try { diff --git a/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManagerTest.java b/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManagerTest.java index a554c0d7f6..ba7e270c35 100644 --- a/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManagerTest.java +++ b/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManagerTest.java @@ -88,21 +88,21 @@ public class NetconfNotificationManagerTest { "2015-10-23T23:59:60.20001Z", "1985-04-12T23:59:60.52Z" )) { - final Date apply = NotificationMessage.RFC3339_DATE_PARSER.apply(time); - final Date parse = new SimpleDateFormat(RFC3339_DATE_FORMAT_WITH_MILLIS_BLUEPRINT).parse(iterator.next()); - assertEquals(parse.getTime(), apply.getTime()); + final var apply = NotificationMessage.RFC3339_DATE_PARSER.apply(time); + final var parse = new SimpleDateFormat(RFC3339_DATE_FORMAT_WITH_MILLIS_BLUEPRINT).parse(iterator.next()); + assertEquals(parse, Date.from(apply)); // Testing that we're consistent from formatting to parsing. final String dateString = NotificationMessage.RFC3339_DATE_FORMATTER.apply(apply); - final Date date1 = NotificationMessage.RFC3339_DATE_PARSER.apply(dateString); + final var date1 = NotificationMessage.RFC3339_DATE_PARSER.apply(dateString); final String dateString1 = NotificationMessage.RFC3339_DATE_FORMATTER.apply(date1); assertEquals(apply, date1); assertEquals(dateString, dateString1); } // Testing that we're consistent from formatting to parsing. - final Date date0 = Date.from(Instant.ofEpochMilli(0)); + final var date0 = Instant.ofEpochMilli(0); final String dateString0 = NotificationMessage.RFC3339_DATE_FORMATTER.apply(date0); - final Date date1 = NotificationMessage.RFC3339_DATE_PARSER.apply(dateString0); + final var date1 = NotificationMessage.RFC3339_DATE_PARSER.apply(dateString0); final String dateString1 = NotificationMessage.RFC3339_DATE_FORMATTER.apply(date1); assertEquals(date0, date1); assertEquals(dateString0, dateString1); diff --git a/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationsTransformUtilTest.java b/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationsTransformUtilTest.java index efbbb54f03..9056ac2c4a 100644 --- a/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationsTransformUtilTest.java +++ b/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationsTransformUtilTest.java @@ -12,7 +12,7 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.withSettings; -import java.util.Date; +import java.time.Instant; import java.util.Set; import org.junit.BeforeClass; import org.junit.Test; @@ -32,7 +32,7 @@ import org.xmlunit.diff.DefaultNodeMatcher; import org.xmlunit.diff.ElementSelectors; public class NotificationsTransformUtilTest { - private static final Date DATE = new Date(); + private static final Instant EVENT_TIME = Instant.now(); private static final String INNER_NOTIFICATION = """ uri4 @@ -44,7 +44,7 @@ public class NotificationsTransformUtilTest { private static final String EXPECTED_NOTIFICATION = "\n" + INNER_NOTIFICATION - + " " + NotificationMessage.RFC3339_DATE_FORMATTER.apply(DATE) + "\n" + + " " + NotificationMessage.RFC3339_DATE_FORMATTER.apply(EVENT_TIME) + "\n" + "\n"; private static NotificationsTransformUtil UTIL; @@ -61,7 +61,7 @@ public class NotificationsTransformUtilTest { withSettings().extraInterfaces(EventInstantAware.class).defaultAnswer(Answers.CALLS_REAL_METHODS)); doReturn(Set.of(new Uri("uri1"))).when(capabilityChange).getAddedCapability(); doReturn(Set.of(new Uri("uri4"), new Uri("uri3"))).when(capabilityChange).getDeletedCapability(); - doReturn(DATE.toInstant()).when((EventInstantAware) capabilityChange).eventInstant(); + doReturn(EVENT_TIME).when((EventInstantAware) capabilityChange).eventInstant(); final var notification = UTIL.transform(capabilityChange, Absolute.of(NetconfCapabilityChange.QNAME)); @@ -70,7 +70,7 @@ public class NotificationsTransformUtilTest { @Test public void testTransformFromDOM() throws Exception { - final var notification = new NotificationMessage(XmlUtil.readXmlToDocument(INNER_NOTIFICATION), DATE); + final var notification = new NotificationMessage(XmlUtil.readXmlToDocument(INNER_NOTIFICATION), EVENT_TIME); compareXml(EXPECTED_NOTIFICATION, notification.toString()); } diff --git a/plugins/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfMessageTransformUtil.java b/plugins/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfMessageTransformUtil.java index 5c788fd2b4..9c6d58ee95 100644 --- a/plugins/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfMessageTransformUtil.java +++ b/plugins/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfMessageTransformUtil.java @@ -454,7 +454,7 @@ public final class NetconfMessageTransformUtil { try { return Map.entry( - NotificationMessage.RFC3339_DATE_PARSER.apply(eventTimeElement.getTextContent()).toInstant(), + NotificationMessage.RFC3339_DATE_PARSER.apply(eventTimeElement.getTextContent()), notificationElement); } catch (final DocumentedException e) { throw new IllegalArgumentException("Notification payload does not contain " + EVENT_TIME + " " + message, @@ -462,7 +462,7 @@ public final class NetconfMessageTransformUtil { } catch (final DateTimeParseException e) { LOG.warn("Unable to parse event time from {}. Setting time to {}", eventTimeElement, NotificationMessage.UNKNOWN_EVENT_TIME, e); - return Map.entry(NotificationMessage.UNKNOWN_EVENT_TIME.toInstant(), notificationElement); + return Map.entry(NotificationMessage.UNKNOWN_EVENT_TIME, notificationElement); } } diff --git a/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfNestedNotificationTest.java b/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfNestedNotificationTest.java index 89ed25bb2f..abcc300507 100644 --- a/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfNestedNotificationTest.java +++ b/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfNestedNotificationTest.java @@ -52,7 +52,7 @@ public class NetconfNestedNotificationTest extends AbstractBaseSchemasTest { assertNotNull(root); assertEquals(1, root.body().size()); assertEquals("interface-enabled", root.getIdentifier().getNodeType().getLocalName()); - assertEquals(NotificationMessage.RFC3339_DATE_PARSER.apply("2008-07-08T00:01:00Z").toInstant(), + assertEquals(NotificationMessage.RFC3339_DATE_PARSER.apply("2008-07-08T00:01:00Z"), ((DOMEvent) domNotification).getEventInstant()); assertEquals(Absolute.of(INTERFACES_QNAME, INTERFACE_QNAME, INTERFACE_ENABLED_NOTIFICATION_QNAME), domNotification.getType()); diff --git a/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfToNotificationTest.java b/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfToNotificationTest.java index ebb2a93b65..2e3c7c744a 100644 --- a/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfToNotificationTest.java +++ b/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/NetconfToNotificationTest.java @@ -80,7 +80,7 @@ public class NetconfToNotificationTest extends AbstractBaseSchemasTest { assertNotNull(root); assertEquals(6, root.body().size()); assertEquals("user-visited-page", root.getIdentifier().getNodeType().getLocalName()); - assertEquals(NotificationMessage.RFC3339_DATE_PARSER.apply("2015-10-23T09:42:27.67175+00:00").toInstant(), + assertEquals(NotificationMessage.RFC3339_DATE_PARSER.apply("2015-10-23T09:42:27.67175+00:00"), ((DOMEvent) domNotification).getEventInstant()); } } diff --git a/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/messages/NotificationMessage.java b/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/messages/NotificationMessage.java index f06db1a26a..dc7eed4729 100644 --- a/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/messages/NotificationMessage.java +++ b/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/messages/NotificationMessage.java @@ -9,7 +9,6 @@ package org.opendaylight.netconf.api.messages; import static java.util.Objects.requireNonNull; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.text.ParsePosition; import java.time.Instant; import java.time.LocalDateTime; @@ -20,8 +19,8 @@ import java.time.format.DateTimeFormatter; import java.time.format.DateTimeParseException; import java.time.temporal.ChronoField; import java.time.temporal.TemporalAccessor; -import java.util.Date; import java.util.function.Function; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.netconf.api.NetconfMessage; import org.opendaylight.netconf.api.xml.XmlNetconfConstants; import org.slf4j.Logger; @@ -40,8 +39,8 @@ public final class NotificationMessage extends NetconfMessage { /** * Used for unknown/un-parse-able event-times. */ - public static final Date UNKNOWN_EVENT_TIME = new Date(0); - + // FIXME: we should differentiate unknown and invalid event times + public static final Instant UNKNOWN_EVENT_TIME = Instant.EPOCH; /** * The ISO-like date-time formatter that formats or parses a date-time with @@ -51,14 +50,14 @@ public final class NotificationMessage extends NetconfMessage { private static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ISO_DATE_TIME; /** - * Provide a {@link String} representation of a {@link Date} object, + * Provide a {@link String} representation of a {@link Instant} object, * using the time-zone offset for UTC, {@code ZoneOffset.UTC}. */ - public static final Function RFC3339_DATE_FORMATTER = date -> - DATE_TIME_FORMATTER.format(date.toInstant().atOffset(ZoneOffset.UTC)); + public static final Function RFC3339_DATE_FORMATTER = date -> + DATE_TIME_FORMATTER.format(date.atOffset(ZoneOffset.UTC)); /** - * Parse a {@link String} object into a {@link Date} using the time-zone + * Parse a {@link String} object into a {@link Instant} using the time-zone * offset for UTC, {@code ZoneOffset.UTC}, and the system default time-zone, * {@code ZoneId.systemDefault()}. *

@@ -69,7 +68,7 @@ public final class NotificationMessage extends NetconfMessage { * caller. *

*/ - public static final Function RFC3339_DATE_PARSER = time -> { + public static final Function RFC3339_DATE_PARSER = time -> { try { final ZonedDateTime localDateTime = ZonedDateTime.parse(time, DATE_TIME_FORMATTER); final int startAt = 0; @@ -95,9 +94,9 @@ public final class NotificationMessage extends NetconfMessage { reminderBuilder.toString()); } - return Date.from(Instant.from(localDateTime)); + return Instant.from(localDateTime); } catch (DateTimeParseException exception) { - Date res = handlePotentialLeapSecond(time); + final var res = handlePotentialLeapSecond(time); if (res != null) { return res; } @@ -111,10 +110,10 @@ public final class NotificationMessage extends NetconfMessage { * conversion is applied, replacing the second-of-minute of 60 with 59. * * @param time {@link String} representation of a time - * @return {@code null} if time isn't ISO compliant or if the time doesn't have a leap second - * else a {@link Date} as per as the RFC3339_DATE_PARSER. + * @return {@code null} if time isn't ISO compliant or if the time doesn't have a leap second else an + * {@link Instant} as per as the RFC3339_DATE_PARSER. */ - private static Date handlePotentialLeapSecond(final String time) { + private static Instant handlePotentialLeapSecond(final String time) { // Parse the string from offset 0, so we get the whole value. final int offset = 0; final TemporalAccessor parsed = DATE_TIME_FORMATTER.parseUnresolved(time, new ParsePosition(offset)); @@ -166,22 +165,21 @@ public final class NotificationMessage extends NetconfMessage { return accessor.isSupported(field) ? (int) accessor.getLong(field) : 0; } - private final Date eventTime; + private final @NonNull Instant eventTime; /** * Create new notification and capture the timestamp in the constructor. */ public NotificationMessage(final Document notificationContent) { - this(notificationContent, new Date()); + this(notificationContent, Instant.now()); } /** * Create new notification with provided timestamp. */ - @SuppressFBWarnings("EI_EXPOSE_REP2") // stores a reference to an externally mutable Date object - public NotificationMessage(final Document notificationContent, final Date eventTime) { + public NotificationMessage(final Document notificationContent, final Instant eventTime) { super(wrapNotification(notificationContent, eventTime)); - this.eventTime = eventTime; + this.eventTime = requireNonNull(eventTime); } /** @@ -189,12 +187,11 @@ public final class NotificationMessage extends NetconfMessage { * * @return notification event time */ - @SuppressFBWarnings("EI_EXPOSE_REP") - public Date getEventTime() { + public @NonNull Instant getEventTime() { return eventTime; } - private static Document wrapNotification(final Document notificationContent, final Date eventTime) { + private static Document wrapNotification(final Document notificationContent, final Instant eventTime) { requireNonNull(notificationContent); requireNonNull(eventTime); @@ -205,14 +202,10 @@ public final class NotificationMessage extends NetconfMessage { final Element eventTimeElement = notificationContent.createElementNS( XmlNetconfConstants.URN_IETF_PARAMS_NETCONF_CAPABILITY_NOTIFICATION_1_0, EVENT_TIME_TAG); - eventTimeElement.setTextContent(getSerializedEventTime(eventTime)); + eventTimeElement.setTextContent(RFC3339_DATE_FORMATTER.apply(eventTime)); entireNotification.appendChild(eventTimeElement); notificationContent.appendChild(entireNotification); return notificationContent; } - - private static String getSerializedEventTime(final Date eventTime) { - return RFC3339_DATE_FORMATTER.apply(eventTime); - } } diff --git a/protocol/netconf-api/src/test/java/org/opendaylight/netconf/api/messages/NotificationMessageTest.java b/protocol/netconf-api/src/test/java/org/opendaylight/netconf/api/messages/NotificationMessageTest.java index cf4f4248e2..f8a9ee8f15 100644 --- a/protocol/netconf-api/src/test/java/org/opendaylight/netconf/api/messages/NotificationMessageTest.java +++ b/protocol/netconf-api/src/test/java/org/opendaylight/netconf/api/messages/NotificationMessageTest.java @@ -5,13 +5,12 @@ * 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.netconf.api.messages; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import java.util.Date; +import java.time.Instant; import org.junit.Test; import org.opendaylight.yangtools.util.xml.UntrustedXML; import org.w3c.dom.Document; @@ -26,8 +25,7 @@ public class NotificationMessageTest { final Element rootElement = document.createElement("test-root"); document.appendChild(rootElement); - final Date eventTime = new Date(); - eventTime.setTime(10000000); + final Instant eventTime = Instant.ofEpochMilli(10_000_000); final NotificationMessage netconfNotification = new NotificationMessage(document, eventTime); final Document resultDoc = netconfNotification.getDocument(); @@ -48,9 +46,7 @@ public class NotificationMessageTest { final Element eventTimeElement = (Element) childNodes.item(0); - assertEquals(eventTime.getTime(), NotificationMessage.RFC3339_DATE_PARSER - .apply(eventTimeElement.getTextContent()).getTime()); - + assertEquals(eventTime, NotificationMessage.RFC3339_DATE_PARSER.apply(eventTimeElement.getTextContent())); assertEquals(eventTime, netconfNotification.getEventTime()); } } -- 2.36.6