From b185f37b7df100af3794264e1a8fb51b73191818 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 30 Oct 2023 19:19:11 +0100 Subject: [PATCH] Reduce StreamsConfiguration proliferation The indirection through configuration is causing us to allocate multiple instances of SubscribeToStreamUtil. Refactor instantiation so that we really are using a single configured instance. JIRA: NETCONF-1102 Change-Id: Ib6028aad570e4fac6d77540c6bece1f44ef77113 Signed-off-by: Robert Varga --- .../nb/rfc8040/RestconfApplication.java | 23 ++++++++++++------- .../RestconfInvokeOperationsServiceImpl.java | 11 +++------ ...estconfStreamsSubscriptionServiceImpl.java | 11 ++++----- .../services/impl/SubscribeToStreamUtil.java | 14 +++++------ ...stconfInvokeOperationsServiceImplTest.java | 3 +-- ...onfStreamsSubscriptionServiceImplTest.java | 19 ++++----------- .../AbstractNotificationListenerTest.java | 2 +- 7 files changed, 36 insertions(+), 47 deletions(-) diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/RestconfApplication.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/RestconfApplication.java index 2e79f502be..245ad33b55 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/RestconfApplication.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/RestconfApplication.java @@ -25,6 +25,7 @@ import org.opendaylight.restconf.nb.rfc8040.rests.services.impl.RestconfInvokeOp import org.opendaylight.restconf.nb.rfc8040.rests.services.impl.RestconfOperationsServiceImpl; import org.opendaylight.restconf.nb.rfc8040.rests.services.impl.RestconfSchemaServiceImpl; import org.opendaylight.restconf.nb.rfc8040.rests.services.impl.RestconfStreamsSubscriptionServiceImpl; +import org.opendaylight.restconf.nb.rfc8040.rests.services.impl.SubscribeToStreamUtil; import org.opendaylight.restconf.nb.rfc8040.streams.StreamsConfiguration; import org.opendaylight.restconf.nb.rfc8040.streams.listeners.ListenersBroker; @@ -34,27 +35,33 @@ public class RestconfApplication extends AbstractRestconfApplication { final DOMMountPointService mountPointService, final RestconfStreamsSubscriptionService streamSubscription, final DOMDataBroker dataBroker, final DOMActionService actionService, final DOMNotificationService notificationService, - final DOMSchemaService domSchemaService, final ListenersBroker listenersBroker, - final StreamsConfiguration configuration) { + final DOMSchemaService domSchemaService, final SubscribeToStreamUtil streamUtils) { super(databindProvider, List.of( streamSubscription, new RestconfDataServiceImpl(databindProvider, server, dataBroker, streamSubscription, actionService), - new RestconfInvokeOperationsServiceImpl(databindProvider, server, mountPointService, listenersBroker, - configuration), + new RestconfInvokeOperationsServiceImpl(databindProvider, server, mountPointService, streamUtils), new RestconfOperationsServiceImpl(databindProvider, server), new RestconfSchemaServiceImpl(domSchemaService, mountPointService), new RestconfImpl(databindProvider))); } + private RestconfApplication(final DatabindProvider databindProvider, final MdsalRestconfServer server, + final DOMMountPointService mountPointService, final DOMDataBroker dataBroker, + final DOMActionService actionService, final DOMNotificationService notificationService, + final DOMSchemaService domSchemaService, final SubscribeToStreamUtil streamUtils) { + this(databindProvider, server, mountPointService, + new RestconfStreamsSubscriptionServiceImpl(dataBroker, notificationService, databindProvider, streamUtils), + dataBroker, actionService, notificationService, domSchemaService, streamUtils); + } + @Inject public RestconfApplication(final DatabindProvider databindProvider, final MdsalRestconfServer server, final DOMMountPointService mountPointService, final DOMDataBroker dataBroker, final DOMRpcService rpcService, final DOMActionService actionService, final DOMNotificationService notificationService, final DOMSchemaService domSchemaService, final ListenersBroker listenersBroker, final StreamsConfiguration configuration) { - this(databindProvider, server, mountPointService, - new RestconfStreamsSubscriptionServiceImpl(dataBroker, notificationService, databindProvider, - listenersBroker, configuration), - dataBroker, actionService, notificationService, domSchemaService, listenersBroker, configuration); + this(databindProvider, server, mountPointService, dataBroker, actionService, notificationService, + domSchemaService, configuration.useSSE() ? SubscribeToStreamUtil.serverSentEvents(listenersBroker) + : SubscribeToStreamUtil.webSockets(listenersBroker)); } } diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfInvokeOperationsServiceImpl.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfInvokeOperationsServiceImpl.java index 8d2464103f..4df6c98d8d 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfInvokeOperationsServiceImpl.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfInvokeOperationsServiceImpl.java @@ -35,8 +35,6 @@ import org.opendaylight.restconf.nb.rfc8040.databind.OperationInputBody; import org.opendaylight.restconf.nb.rfc8040.databind.XmlOperationInputBody; import org.opendaylight.restconf.nb.rfc8040.legacy.InstanceIdentifierContext; import org.opendaylight.restconf.nb.rfc8040.legacy.NormalizedNodePayload; -import org.opendaylight.restconf.nb.rfc8040.streams.StreamsConfiguration; -import org.opendaylight.restconf.nb.rfc8040.streams.listeners.ListenersBroker; import org.opendaylight.yang.gen.v1.urn.opendaylight.device.notification.rev221106.SubscribeDeviceNotification; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.remote.rev140114.CreateDataChangeEventSubscription; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.remote.rev140114.CreateNotificationStream; @@ -59,17 +57,14 @@ public final class RestconfInvokeOperationsServiceImpl { @Deprecated(forRemoval = true) private final DOMMountPointService mountPointService; private final SubscribeToStreamUtil streamUtils; - private final ListenersBroker listenersBroker; public RestconfInvokeOperationsServiceImpl(final DatabindProvider databindProvider, final MdsalRestconfServer server, final DOMMountPointService mountPointService, - final ListenersBroker listenersBroker, final StreamsConfiguration configuration) { + final SubscribeToStreamUtil streamUtils) { this.databindProvider = requireNonNull(databindProvider); this.server = requireNonNull(server); this.mountPointService = requireNonNull(mountPointService); - this.listenersBroker = requireNonNull(listenersBroker); - streamUtils = configuration.useSSE() ? SubscribeToStreamUtil.serverSentEvents(listenersBroker) - : SubscribeToStreamUtil.webSockets(listenersBroker); + this.streamUtils = requireNonNull(streamUtils); } /** @@ -170,7 +165,7 @@ public final class RestconfInvokeOperationsServiceImpl { return CreateStreamUtil.createNotificationStream(streamUtils.listenersBroker(), input, localDatabind.modelContext()); } else if (SubscribeDeviceNotification.QNAME.equals(type)) { - return CreateStreamUtil.createDeviceNotificationListener(listenersBroker, input, + return CreateStreamUtil.createDeviceNotificationListener(streamUtils.listenersBroker(), input, streamUtils.prepareUriByStreamName(uriInfo, "").toString(), mountPointService); } } diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImpl.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImpl.java index 3a192290a3..822862a2e0 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImpl.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImpl.java @@ -7,6 +7,8 @@ */ package org.opendaylight.restconf.nb.rfc8040.rests.services.impl; +import static java.util.Objects.requireNonNull; + import java.net.URI; import javax.ws.rs.Path; import javax.ws.rs.core.Response; @@ -19,8 +21,6 @@ import org.opendaylight.restconf.nb.rfc8040.databind.jaxrs.QueryParams; import org.opendaylight.restconf.nb.rfc8040.legacy.NormalizedNodePayload; import org.opendaylight.restconf.nb.rfc8040.rests.services.api.RestconfStreamsSubscriptionService; import org.opendaylight.restconf.nb.rfc8040.rests.utils.RestconfStreamsConstants; -import org.opendaylight.restconf.nb.rfc8040.streams.StreamsConfiguration; -import org.opendaylight.restconf.nb.rfc8040.streams.listeners.ListenersBroker; import org.opendaylight.yang.gen.v1.subscribe.to.notification.rev161028.Notifi; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; @@ -47,14 +47,13 @@ public class RestconfStreamsSubscriptionServiceImpl implements RestconfStreamsSu * @param dataBroker {@link DOMDataBroker} * @param notificationService {@link DOMNotificationService} * @param databindProvider a {@link DatabindProvider} - * @param configuration configuration for RESTCONF {@link StreamsConfiguration}} + * @param streamUtils a {@link SubscribeToStreamUtil} */ public RestconfStreamsSubscriptionServiceImpl(final DOMDataBroker dataBroker, final DOMNotificationService notificationService, final DatabindProvider databindProvider, - final ListenersBroker listenersBroker, final StreamsConfiguration configuration) { + final SubscribeToStreamUtil streamUtils) { handlersHolder = new HandlersHolder(dataBroker, notificationService, databindProvider); - streamUtils = configuration.useSSE() ? SubscribeToStreamUtil.serverSentEvents(listenersBroker) - : SubscribeToStreamUtil.webSockets(listenersBroker); + this.streamUtils = requireNonNull(streamUtils); } @Override diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/SubscribeToStreamUtil.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/SubscribeToStreamUtil.java index ba6be4e46a..d8b68b5fd1 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/SubscribeToStreamUtil.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/SubscribeToStreamUtil.java @@ -42,13 +42,12 @@ import org.slf4j.LoggerFactory; /** * Subscribe to stream util class. */ -abstract class SubscribeToStreamUtil { +public abstract class SubscribeToStreamUtil { /** * Implementation of SubscribeToStreamUtil for Server-sent events. */ private static final class ServerSentEvents extends SubscribeToStreamUtil { - - private ServerSentEvents(final ListenersBroker listenersBroker) { + ServerSentEvents(final ListenersBroker listenersBroker) { super(listenersBroker); } @@ -64,8 +63,7 @@ abstract class SubscribeToStreamUtil { * Implementation of SubscribeToStreamUtil for Web sockets. */ private static final class WebSockets extends SubscribeToStreamUtil { - - private WebSockets(final ListenersBroker listenersBroker) { + WebSockets(final ListenersBroker listenersBroker) { super(listenersBroker); } @@ -92,15 +90,15 @@ abstract class SubscribeToStreamUtil { private final @NonNull ListenersBroker listenersBroker; - SubscribeToStreamUtil(final ListenersBroker listenersBroker) { + private SubscribeToStreamUtil(final ListenersBroker listenersBroker) { this.listenersBroker = requireNonNull(listenersBroker); } - static SubscribeToStreamUtil serverSentEvents(final ListenersBroker listenersBroker) { + public static @NonNull SubscribeToStreamUtil serverSentEvents(final ListenersBroker listenersBroker) { return new ServerSentEvents(listenersBroker); } - static SubscribeToStreamUtil webSockets(final ListenersBroker listenersBroker) { + public static @NonNull SubscribeToStreamUtil webSockets(final ListenersBroker listenersBroker) { return new WebSockets(listenersBroker); } diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfInvokeOperationsServiceImplTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfInvokeOperationsServiceImplTest.java index 5d160aaf02..67e50dfcd7 100644 --- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfInvokeOperationsServiceImplTest.java +++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfInvokeOperationsServiceImplTest.java @@ -44,7 +44,6 @@ import org.opendaylight.netconf.dom.api.NetconfDataTreeService; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; import org.opendaylight.restconf.nb.rfc8040.databind.DatabindContext; import org.opendaylight.restconf.nb.rfc8040.legacy.NormalizedNodePayload; -import org.opendaylight.restconf.nb.rfc8040.streams.StreamsConfiguration; import org.opendaylight.restconf.nb.rfc8040.streams.listeners.ListenersBroker; import org.opendaylight.yangtools.yang.common.ErrorTag; import org.opendaylight.yangtools.yang.common.ErrorType; @@ -90,7 +89,7 @@ public class RestconfInvokeOperationsServiceImplTest { public void setup() { server = new MdsalRestconfServer(dataBroker, rpcService, mountPointService); invokeOperationsService = new RestconfInvokeOperationsServiceImpl(() -> CONTEXT, server, mountPointService, - new ListenersBroker(), new StreamsConfiguration(0, 1, 0, false)); + SubscribeToStreamUtil.webSockets(new ListenersBroker())); } @Test diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImplTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImplTest.java index c48ca9ba76..7d430b5bb9 100644 --- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImplTest.java +++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImplTest.java @@ -12,7 +12,6 @@ import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; -import static org.opendaylight.restconf.nb.rfc8040.streams.listeners.AbstractNotificationListenerTest.MODEL_CONTEXT; import com.google.common.collect.ImmutableClassToInstanceMap; import java.net.URI; @@ -35,7 +34,6 @@ import org.opendaylight.restconf.common.errors.RestconfDocumentedException; import org.opendaylight.restconf.nb.rfc8040.URLConstants; import org.opendaylight.restconf.nb.rfc8040.databind.DatabindContext; import org.opendaylight.restconf.nb.rfc8040.databind.DatabindProvider; -import org.opendaylight.restconf.nb.rfc8040.streams.StreamsConfiguration; import org.opendaylight.restconf.nb.rfc8040.streams.listeners.AbstractNotificationListenerTest; import org.opendaylight.restconf.nb.rfc8040.streams.listeners.ListenersBroker; import org.opendaylight.restconf.nb.rfc8040.utils.parser.IdentifierCodec; @@ -60,10 +58,7 @@ public class RestconfStreamsSubscriptionServiceImplTest extends AbstractNotifica private DOMNotificationService notificationService; private final ListenersBroker listenersBroker = new ListenersBroker(); - private StreamsConfiguration configurationWs; - private StreamsConfiguration configurationSse; - private DatabindProvider databindProvider; - + private final DatabindProvider databindProvider = () -> DatabindContext.ofModel(MODEL_CONTEXT); @Before public void setUp() throws URISyntaxException { @@ -84,10 +79,6 @@ public class RestconfStreamsSubscriptionServiceImplTest extends AbstractNotifica doReturn(new MultivaluedHashMap<>()).when(uriInfo).getQueryParameters(); doReturn(UriBuilder.fromUri("http://localhost:8181")).when(uriInfo).getBaseUriBuilder(); doReturn(new URI("http://127.0.0.1/" + URI)).when(uriInfo).getAbsolutePath(); - - databindProvider = () -> DatabindContext.ofModel(MODEL_CONTEXT); - configurationWs = new StreamsConfiguration(0, 100, 10, false); - configurationSse = new StreamsConfiguration(0, 100, 10, true); } @Test @@ -96,7 +87,7 @@ public class RestconfStreamsSubscriptionServiceImplTest extends AbstractNotifica IdentifierCodec.deserialize("toaster:toaster/toasterStatus", MODEL_CONTEXT), Scope.ONE, NotificationOutputType.XML); final var streamsSubscriptionService = new RestconfStreamsSubscriptionServiceImpl(dataBroker, - notificationService, databindProvider, listenersBroker, configurationSse); + notificationService, databindProvider, SubscribeToStreamUtil.serverSentEvents(listenersBroker)); final var response = streamsSubscriptionService.subscribeToStream( "data-change-event-subscription/toaster:toaster/toasterStatus/datastore=OPERATIONAL/scope=ONE", uriInfo); assertEquals("http://localhost:8181/" + URLConstants.BASE_PATH + "/" + URLConstants.SSE_SUBPATH @@ -110,7 +101,7 @@ public class RestconfStreamsSubscriptionServiceImplTest extends AbstractNotifica IdentifierCodec.deserialize("toaster:toaster/toasterStatus", MODEL_CONTEXT), Scope.ONE, NotificationOutputType.XML); final var streamsSubscriptionService = new RestconfStreamsSubscriptionServiceImpl(dataBroker, - notificationService, databindProvider, listenersBroker, configurationWs); + notificationService, databindProvider, SubscribeToStreamUtil.webSockets(listenersBroker)); final var response = streamsSubscriptionService.subscribeToStream( "data-change-event-subscription/toaster:toaster/toasterStatus/datastore=OPERATIONAL/scope=ONE", uriInfo); assertEquals("ws://localhost:8181/" + URLConstants.BASE_PATH @@ -121,7 +112,7 @@ public class RestconfStreamsSubscriptionServiceImplTest extends AbstractNotifica @Test public void testSubscribeToStreamMissingDatastoreInPath() { final var streamsSubscriptionService = new RestconfStreamsSubscriptionServiceImpl(dataBroker, - notificationService, databindProvider, listenersBroker, configurationWs); + notificationService, databindProvider, SubscribeToStreamUtil.webSockets(listenersBroker)); final var errors = assertThrows(RestconfDocumentedException.class, () -> streamsSubscriptionService.subscribeToStream("toaster:toaster/toasterStatus/scope=ONE", uriInfo)) .getErrors(); @@ -135,7 +126,7 @@ public class RestconfStreamsSubscriptionServiceImplTest extends AbstractNotifica @Test public void testSubscribeToStreamMissingScopeInPath() { final var streamsSubscriptionService = new RestconfStreamsSubscriptionServiceImpl(dataBroker, - notificationService, databindProvider, listenersBroker, configurationWs); + notificationService, databindProvider, SubscribeToStreamUtil.webSockets(listenersBroker)); final var errors = assertThrows(RestconfDocumentedException.class, () -> streamsSubscriptionService.subscribeToStream("toaster:toaster/toasterStatus/datastore=OPERATIONAL", uriInfo)).getErrors(); diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/AbstractNotificationListenerTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/AbstractNotificationListenerTest.java index 19273353a9..cb82f9314d 100644 --- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/AbstractNotificationListenerTest.java +++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/streams/listeners/AbstractNotificationListenerTest.java @@ -16,6 +16,6 @@ import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils; public abstract class AbstractNotificationListenerTest { static final QNameModule MODULE = QNameModule.create(XMLNamespace.of("notifi:mod"), Revision.of("2016-11-23")); - public static final EffectiveModelContext MODEL_CONTEXT = + protected static final EffectiveModelContext MODEL_CONTEXT = YangParserTestUtils.parseYangResourceDirectory("/notifications"); } -- 2.36.6