From: Robert Varga Date: Sat, 23 Oct 2021 09:11:21 +0000 (+0200) Subject: Cleanup NormalizedNodeContext lifecycle X-Git-Tag: v2.0.6~39 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=d152b6b85cf24a9eb48c3a3c45c0c7c493f9c426;p=netconf.git Cleanup NormalizedNodeContext lifecycle We have headers as a non-final field with a leaking HashMap. As it turns out, this is only used for stream listers, which can use ImmutableMap. Cleanup constructors to express this, so we know what gets filled in. Change-Id: I43f3b77c8c2a9640b6edbd815ce72bc3637819ba Signed-off-by: Robert Varga --- diff --git a/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/context/NormalizedNodeContext.java b/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/context/NormalizedNodeContext.java index b22e163514..a4348b458e 100644 --- a/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/context/NormalizedNodeContext.java +++ b/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/context/NormalizedNodeContext.java @@ -7,49 +7,40 @@ */ package org.opendaylight.restconf.common.context; -import java.util.HashMap; -import java.util.Map; +import static java.util.Objects.requireNonNull; + +import com.google.common.collect.ImmutableMap; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.model.api.SchemaNode; public class NormalizedNodeContext { - private final InstanceIdentifierContext context; + private final ImmutableMap headers; private final WriterParameters writerParameters; private final NormalizedNode data; - private Map headers = new HashMap<>(); - public NormalizedNodeContext(final InstanceIdentifierContext context, - final NormalizedNode data, final WriterParameters writerParameters) { + final NormalizedNode data, final WriterParameters writerParameters, + final ImmutableMap headers) { this.context = context; this.data = data; this.writerParameters = writerParameters; + this.headers = requireNonNull(headers); } public NormalizedNodeContext(final InstanceIdentifierContext context, - final NormalizedNode data, final WriterParameters writerParameters, final Map headers) { - this.context = context; - this.data = data; - this.writerParameters = writerParameters; - this.headers = headers; + final NormalizedNode data, final WriterParameters writerParameters) { + this(context, data, writerParameters, ImmutableMap.of()); } public NormalizedNodeContext(final InstanceIdentifierContext context, final NormalizedNode data) { - this.context = context; - this.data = data; - // default writer parameters - writerParameters = WriterParameters.EMPTY; + this(context, data, WriterParameters.EMPTY, ImmutableMap.of()); } public NormalizedNodeContext(final InstanceIdentifierContext context, - final NormalizedNode data, final Map headers) { - this.context = context; - this.data = data; - // default writer parameters - writerParameters = WriterParameters.EMPTY; - this.headers = headers; + final NormalizedNode data, final ImmutableMap headers) { + this(context, data, WriterParameters.EMPTY, headers); } public InstanceIdentifierContext getInstanceIdentifierContext() { @@ -69,7 +60,7 @@ public class NormalizedNodeContext { * * @return map of headers */ - public Map getNewHeaders() { + public ImmutableMap getNewHeaders() { return headers; } } diff --git a/restconf/restconf-nb-bierman02/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfImpl.java b/restconf/restconf-nb-bierman02/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfImpl.java index d1d1add8c8..54891164e7 100644 --- a/restconf/restconf-nb-bierman02/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfImpl.java +++ b/restconf/restconf-nb-bierman02/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfImpl.java @@ -17,6 +17,7 @@ import com.google.common.base.Predicates; import com.google.common.base.Splitter; import com.google.common.base.Strings; import com.google.common.base.Throwables; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.util.concurrent.FluentFuture; @@ -210,10 +211,10 @@ public final class RestconfImpl implements RestconfService { public NormalizedNodeContext getModules(final UriInfo uriInfo) { final MapNode allModuleMap = makeModuleMapNode(controllerContext.getAllModules()); - final EffectiveModelContext schemaContext = this.controllerContext.getGlobalSchema(); + final EffectiveModelContext schemaContext = controllerContext.getGlobalSchema(); final Module restconfModule = getRestconfModule(); - final DataSchemaNode modulesSchemaNode = this.controllerContext.getRestconfModuleRestConfSchemaNode( + final DataSchemaNode modulesSchemaNode = controllerContext.getRestconfModuleRestConfSchemaNode( restconfModule, Draft02.RestConfModule.MODULES_CONTAINER_SCHEMA_NODE); checkState(modulesSchemaNode instanceof ContainerSchemaNode); @@ -239,12 +240,12 @@ public final class RestconfImpl implements RestconfService { } final InstanceIdentifierContext mountPointIdentifier = - this.controllerContext.toMountPointIdentifier(identifier); + controllerContext.toMountPointIdentifier(identifier); final DOMMountPoint mountPoint = mountPointIdentifier.getMountPoint(); final MapNode mountPointModulesMap = makeModuleMapNode(controllerContext.getAllModules(mountPoint)); final Module restconfModule = getRestconfModule(); - final DataSchemaNode modulesSchemaNode = this.controllerContext.getRestconfModuleRestConfSchemaNode( + final DataSchemaNode modulesSchemaNode = controllerContext.getRestconfModuleRestConfSchemaNode( restconfModule, Draft02.RestConfModule.MODULES_CONTAINER_SCHEMA_NODE); checkState(modulesSchemaNode instanceof ContainerSchemaNode); @@ -254,7 +255,7 @@ public final class RestconfImpl implements RestconfService { return new NormalizedNodeContext( new InstanceIdentifierContext<>(null, modulesSchemaNode, mountPoint, - this.controllerContext.getGlobalSchema()), + controllerContext.getGlobalSchema()), moduleContainerBuilder.build(), QueryParametersParser.parseWriterParameters(uriInfo)); } @@ -267,14 +268,14 @@ public final class RestconfImpl implements RestconfService { final EffectiveModelContext schemaContext; if (identifier.contains(ControllerContext.MOUNT)) { final InstanceIdentifierContext mountPointIdentifier = - this.controllerContext.toMountPointIdentifier(identifier); + controllerContext.toMountPointIdentifier(identifier); mountPoint = mountPointIdentifier.getMountPoint(); - module = this.controllerContext.findModuleByNameAndRevision(mountPoint, nameRev.getKey(), + module = controllerContext.findModuleByNameAndRevision(mountPoint, nameRev.getKey(), nameRev.getValue()); schemaContext = modelContext(mountPoint); } else { - module = this.controllerContext.findModuleByNameAndRevision(nameRev.getKey(), nameRev.getValue()); - schemaContext = this.controllerContext.getGlobalSchema(); + module = controllerContext.findModuleByNameAndRevision(nameRev.getKey(), nameRev.getValue()); + schemaContext = controllerContext.getGlobalSchema(); } if (module == null) { @@ -287,7 +288,7 @@ public final class RestconfImpl implements RestconfService { final Set modules = Collections.singleton(module); final MapNode moduleMap = makeModuleMapNode(modules); - final DataSchemaNode moduleSchemaNode = this.controllerContext + final DataSchemaNode moduleSchemaNode = controllerContext .getRestconfModuleRestConfSchemaNode(restconfModule, Draft02.RestConfModule.MODULE_LIST_SCHEMA_NODE); checkState(moduleSchemaNode instanceof ListSchemaNode); @@ -299,10 +300,10 @@ public final class RestconfImpl implements RestconfService { @Override @Deprecated public NormalizedNodeContext getAvailableStreams(final UriInfo uriInfo) { - final EffectiveModelContext schemaContext = this.controllerContext.getGlobalSchema(); + final EffectiveModelContext schemaContext = controllerContext.getGlobalSchema(); final Set availableStreams = Notificator.getStreamNames(); final Module restconfModule = getRestconfModule(); - final DataSchemaNode streamSchemaNode = this.controllerContext + final DataSchemaNode streamSchemaNode = controllerContext .getRestconfModuleRestConfSchemaNode(restconfModule, Draft02.RestConfModule.STREAM_LIST_SCHEMA_NODE); checkState(streamSchemaNode instanceof ListSchemaNode); @@ -313,7 +314,7 @@ public final class RestconfImpl implements RestconfService { listStreamsBuilder.withChild(toStreamEntryNode(streamName, streamSchemaNode)); } - final DataSchemaNode streamsContainerSchemaNode = this.controllerContext.getRestconfModuleRestConfSchemaNode( + final DataSchemaNode streamsContainerSchemaNode = controllerContext.getRestconfModuleRestConfSchemaNode( restconfModule, Draft02.RestConfModule.STREAMS_CONTAINER_SCHEMA_NODE); checkState(streamsContainerSchemaNode instanceof ContainerSchemaNode); @@ -349,13 +350,13 @@ public final class RestconfImpl implements RestconfService { } final InstanceIdentifierContext mountPointIdentifier = - this.controllerContext.toMountPointIdentifier(identifier); + controllerContext.toMountPointIdentifier(identifier); final DOMMountPoint mountPoint = mountPointIdentifier.getMountPoint(); return OperationsResourceUtils.contextForModelContext(modelContext(mountPoint), mountPoint); } private Module getRestconfModule() { - final Module restconfModule = this.controllerContext.getRestconfModule(); + final Module restconfModule = controllerContext.getRestconfModule(); if (restconfModule == null) { LOG.debug("ietf-restconf module was not found."); throw new RestconfDocumentedException("ietf-restconf module was not found.", ErrorType.APPLICATION, @@ -432,9 +433,9 @@ public final class RestconfImpl implements RestconfService { throw new RestconfDocumentedException(msg, ErrorType.RPC, ErrorTag.OPERATION_NOT_SUPPORTED); } } else { - response = this.broker.invokeRpc(schema.getQName(), input); + response = broker.invokeRpc(schema.getQName(), input); } - schemaContext = this.controllerContext.getGlobalSchema(); + schemaContext = controllerContext.getGlobalSchema(); } final DOMRpcResult result = checkRpcResponse(response); @@ -465,7 +466,7 @@ public final class RestconfImpl implements RestconfService { final EffectiveModelContext schemaContext; if (identifier.contains(ControllerContext.MOUNT)) { // mounted RPC call - look up mount instance. - final InstanceIdentifierContext mountPointId = this.controllerContext.toMountPointIdentifier(identifier); + final InstanceIdentifierContext mountPointId = controllerContext.toMountPointIdentifier(identifier); mountPoint = mountPointId.getMountPoint(); schemaContext = modelContext(mountPoint); final int startOfRemoteRpcName = @@ -476,7 +477,7 @@ public final class RestconfImpl implements RestconfService { } else if (identifier.indexOf('/') == CHAR_NOT_FOUND) { identifierEncoded = identifier; mountPoint = null; - schemaContext = this.controllerContext.getGlobalSchema(); + schemaContext = controllerContext.getGlobalSchema(); } else { LOG.debug("Identifier {} cannot contain slash character (/).", identifier); throw new RestconfDocumentedException(String.format("Identifier %n%s%ncan\'t contain slash character (/).%n" @@ -487,7 +488,7 @@ public final class RestconfImpl implements RestconfService { final String identifierDecoded = ControllerContext.urlPathArgDecode(identifierEncoded); final RpcDefinition rpc; if (mountPoint == null) { - rpc = this.controllerContext.getRpcDefinition(identifierDecoded); + rpc = controllerContext.getRpcDefinition(identifierDecoded); } else { rpc = findRpc(modelContext(mountPoint), identifierDecoded); } @@ -512,7 +513,7 @@ public final class RestconfImpl implements RestconfService { } response = mountRpcServices.get().invokeRpc(rpc.getQName(), input); } else { - response = this.broker.invokeRpc(rpc.getQName(), input); + response = broker.invokeRpc(rpc.getQName(), input); } final NormalizedNode result = checkRpcResponse(response).getResult(); @@ -609,7 +610,7 @@ public final class RestconfImpl implements RestconfService { NotificationOutputType outputType = null; if (!pathIdentifier.isEmpty()) { final String fullRestconfIdentifier = - DATA_SUBSCR + this.controllerContext.toFullRestconfIdentifier(pathIdentifier, null); + DATA_SUBSCR + controllerContext.toFullRestconfIdentifier(pathIdentifier, null); LogicalDatastoreType datastore = parseEnumTypeParameter(value, LogicalDatastoreType.class, DATASTORE_PARAM_NAME); @@ -696,14 +697,14 @@ public final class RestconfImpl implements RestconfService { } } - final InstanceIdentifierContext iiWithData = this.controllerContext.toInstanceIdentifier(identifier); + final InstanceIdentifierContext iiWithData = controllerContext.toInstanceIdentifier(identifier); final DOMMountPoint mountPoint = iiWithData.getMountPoint(); NormalizedNode data = null; final YangInstanceIdentifier normalizedII = iiWithData.getInstanceIdentifier(); if (mountPoint != null) { - data = this.broker.readConfigurationData(mountPoint, normalizedII, withDefa); + data = broker.readConfigurationData(mountPoint, normalizedII, withDefa); } else { - data = this.broker.readConfigurationData(normalizedII, withDefa); + data = broker.readConfigurationData(normalizedII, withDefa); } if (data == null) { throw dataMissing(identifier); @@ -714,14 +715,14 @@ public final class RestconfImpl implements RestconfService { @Override public NormalizedNodeContext readOperationalData(final String identifier, final UriInfo uriInfo) { - final InstanceIdentifierContext iiWithData = this.controllerContext.toInstanceIdentifier(identifier); + final InstanceIdentifierContext iiWithData = controllerContext.toInstanceIdentifier(identifier); final DOMMountPoint mountPoint = iiWithData.getMountPoint(); NormalizedNode data = null; final YangInstanceIdentifier normalizedII = iiWithData.getInstanceIdentifier(); if (mountPoint != null) { - data = this.broker.readOperationalData(mountPoint, normalizedII); + data = broker.readOperationalData(mountPoint, normalizedII); } else { - data = this.broker.readOperationalData(normalizedII); + data = broker.readOperationalData(normalizedII); } if (data == null) { throw dataMissing(identifier); @@ -807,10 +808,10 @@ public final class RestconfImpl implements RestconfService { while (true) { if (mountPoint != null) { - result = this.broker.commitMountPointDataPut(mountPoint, normalizedII, payload.getData(), insert, + result = broker.commitMountPointDataPut(mountPoint, normalizedII, payload.getData(), insert, point); } else { - result = this.broker.commitConfigurationDataPut(this.controllerContext.getGlobalSchema(), normalizedII, + result = broker.commitConfigurationDataPut(controllerContext.getGlobalSchema(), normalizedII, payload.getData(), insert, point); } @@ -963,10 +964,10 @@ public final class RestconfImpl implements RestconfService { FluentFuture future; if (mountPoint != null) { - future = this.broker.commitConfigurationDataPost(mountPoint, normalizedII, payload.getData(), insert, + future = broker.commitConfigurationDataPost(mountPoint, normalizedII, payload.getData(), insert, point); } else { - future = this.broker.commitConfigurationDataPost(this.controllerContext.getGlobalSchema(), normalizedII, + future = broker.commitConfigurationDataPost(controllerContext.getGlobalSchema(), normalizedII, payload.getData(), insert, point); } @@ -1003,7 +1004,7 @@ public final class RestconfImpl implements RestconfService { final UriBuilder uriBuilder = uriInfo.getBaseUriBuilder(); uriBuilder.path("config"); try { - uriBuilder.path(this.controllerContext.toFullRestconfIdentifier(normalizedII, mountPoint)); + uriBuilder.path(controllerContext.toFullRestconfIdentifier(normalizedII, mountPoint)); } catch (final Exception e) { LOG.info("Location for instance identifier {} was not created", normalizedII, e); return null; @@ -1013,15 +1014,15 @@ public final class RestconfImpl implements RestconfService { @Override public Response deleteConfigurationData(final String identifier) { - final InstanceIdentifierContext iiWithData = this.controllerContext.toInstanceIdentifier(identifier); + final InstanceIdentifierContext iiWithData = controllerContext.toInstanceIdentifier(identifier); final DOMMountPoint mountPoint = iiWithData.getMountPoint(); final YangInstanceIdentifier normalizedII = iiWithData.getInstanceIdentifier(); final FluentFuture future; if (mountPoint != null) { - future = this.broker.commitConfigurationDataDelete(mountPoint, normalizedII); + future = broker.commitConfigurationDataDelete(mountPoint, normalizedII); } else { - future = this.broker.commitConfigurationDataDelete(normalizedII); + future = broker.commitConfigurationDataDelete(normalizedII); } try { @@ -1135,10 +1136,7 @@ public final class RestconfImpl implements RestconfService { NodeIdentifier.create(QName.create("subscribe:to:notification", "2016-10-28", "location"))); // prepare new header with location - final Map headers = new HashMap<>(); - headers.put("Location", response); - - return new NormalizedNodeContext(iid, builder.build(), headers); + return new NormalizedNodeContext(iid, builder.build(), ImmutableMap.of("Location", response)); } final String msg = "Bad type of notification of sal-remote"; @@ -1206,7 +1204,7 @@ public final class RestconfImpl implements RestconfService { } for (final NotificationListenerAdapter listener : listeners) { - this.broker.registerToListenNotification(listener); + broker.registerToListenNotification(listener); listener.setQueryParams(start, Optional.ofNullable(stop), Optional.ofNullable(filter), false, false); } @@ -1273,7 +1271,7 @@ public final class RestconfImpl implements RestconfService { ErrorType.APPLICATION, ErrorTag.MISSING_ATTRIBUTE); } - this.broker.registerToListenDataChanges(datastore, scope, listener); + broker.registerToListenDataChanges(datastore, scope, listener); final UriBuilder uriBuilder = uriInfo.getAbsolutePathBuilder(); @@ -1294,7 +1292,7 @@ public final class RestconfImpl implements RestconfService { } try { - return this.broker.patchConfigurationDataWithinTransaction(context); + return broker.patchConfigurationDataWithinTransaction(context); } catch (final Exception e) { LOG.debug("Patch transaction failed", e); throw new RestconfDocumentedException(e.getMessage(), e); @@ -1309,7 +1307,7 @@ public final class RestconfImpl implements RestconfService { } try { - return this.broker.patchConfigurationDataWithinTransaction(context); + return broker.patchConfigurationDataWithinTransaction(context); } catch (final Exception e) { LOG.debug("Patch transaction failed", e); throw new RestconfDocumentedException(e.getMessage(), e); @@ -1388,7 +1386,7 @@ public final class RestconfImpl implements RestconfService { private MapNode makeModuleMapNode(final Collection modules) { requireNonNull(modules); final Module restconfModule = getRestconfModule(); - final DataSchemaNode moduleSchemaNode = this.controllerContext + final DataSchemaNode moduleSchemaNode = controllerContext .getRestconfModuleRestConfSchemaNode(restconfModule, Draft02.RestConfModule.MODULE_LIST_SCHEMA_NODE); checkState(moduleSchemaNode instanceof ListSchemaNode); diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImpl.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImpl.java index fe8fa79ee8..7306c70ddb 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImpl.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfStreamsSubscriptionServiceImpl.java @@ -9,6 +9,7 @@ package org.opendaylight.restconf.nb.rfc8040.rests.services.impl; import static com.google.common.base.Preconditions.checkState; +import com.google.common.collect.ImmutableMap; import java.net.URI; import java.time.Instant; import java.time.format.DateTimeFormatter; @@ -16,9 +17,7 @@ import java.time.format.DateTimeFormatterBuilder; import java.time.format.DateTimeParseException; import java.time.temporal.ChronoField; import java.time.temporal.TemporalAccessor; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Map.Entry; import java.util.Optional; import javax.ws.rs.Path; @@ -37,7 +36,7 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types. import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; -import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableLeafNodeBuilder; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode; import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; import org.opendaylight.yangtools.yang.model.api.Module; @@ -75,7 +74,7 @@ public class RestconfStreamsSubscriptionServiceImpl implements RestconfStreamsSu public RestconfStreamsSubscriptionServiceImpl(final DOMDataBroker dataBroker, final DOMNotificationService notificationService, final SchemaContextHandler schemaHandler, final Configuration configuration) { - this.handlersHolder = new HandlersHolder(dataBroker, notificationService, schemaHandler); + handlersHolder = new HandlersHolder(dataBroker, notificationService, schemaHandler); streamUtils = configuration.isUseSSE() ? SubscribeToStreamUtil.serverSentEvents() : SubscribeToStreamUtil.webSockets(); } @@ -95,16 +94,9 @@ public class RestconfStreamsSubscriptionServiceImpl implements RestconfStreamsSu throw new RestconfDocumentedException(msg); } - // prepare new header with location - final Map headers = new HashMap<>(); - headers.put("Location", response); - // prepare node with value of location return new NormalizedNodeContext(prepareIIDSubsStreamOutput(handlersHolder.getSchemaHandler()), - ImmutableLeafNodeBuilder.create() - .withNodeIdentifier(LOCATION_NODEID) - .withValue(response.toString()) - .build(), headers); + ImmutableNodes.leafNode(LOCATION_NODEID, response.toString()), ImmutableMap.of("Location", response)); } /** @@ -146,7 +138,7 @@ public class RestconfStreamsSubscriptionServiceImpl implements RestconfStreamsSu * @return the dataBroker */ public DOMDataBroker getDataBroker() { - return this.dataBroker; + return dataBroker; } /** @@ -155,7 +147,7 @@ public class RestconfStreamsSubscriptionServiceImpl implements RestconfStreamsSu * @return the notificationService */ public DOMNotificationService getNotificationServiceHandler() { - return this.notificationService; + return notificationService; } /** @@ -164,7 +156,7 @@ public class RestconfStreamsSubscriptionServiceImpl implements RestconfStreamsSu * @return the schemaHandler */ public SchemaContextHandler getSchemaHandler() { - return this.schemaHandler; + return schemaHandler; } }