diff --git a/SESSION_0_FSM_REVIEW.md b/SESSION_0_FSM_REVIEW.md new file mode 100644 index 0000000000..bb014dd720 --- /dev/null +++ b/SESSION_0_FSM_REVIEW.md @@ -0,0 +1,8 @@ + +## Discussions for later +- We need to take care of modifications of the configuration file (Global lock) + +## Tasks +- the state machine to functions within transitions (fully unit tested) +- (?) Add a testing protocol adapter for testing (let's see if we need it, mockito might be) +- new PR Epic!! -> diff --git a/hivemq-edge/src/main/java/com/hivemq/HiveMQEdgeMain.java b/hivemq-edge/src/main/java/com/hivemq/HiveMQEdgeMain.java index aad3bf4dcf..c9a7797417 100644 --- a/hivemq-edge/src/main/java/com/hivemq/HiveMQEdgeMain.java +++ b/hivemq-edge/src/main/java/com/hivemq/HiveMQEdgeMain.java @@ -21,35 +21,32 @@ import com.hivemq.bootstrap.LoggingBootstrap; import com.hivemq.bootstrap.ioc.Injector; import com.hivemq.bootstrap.ioc.Persistences; -import com.hivemq.bootstrap.services.AfterHiveMQStartBootstrapService; import com.hivemq.bootstrap.services.AfterHiveMQStartBootstrapServiceImpl; import com.hivemq.common.shutdown.ShutdownHooks; import com.hivemq.configuration.info.SystemInformation; import com.hivemq.configuration.info.SystemInformationImpl; -import com.hivemq.configuration.service.ApiConfigurationService; import com.hivemq.configuration.service.ConfigurationService; import com.hivemq.edge.modules.ModuleLoader; import com.hivemq.embedded.EmbeddedExtension; import com.hivemq.exceptions.HiveMQEdgeStartupException; +import com.hivemq.http.JaxrsHttpServer; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import com.hivemq.http.JaxrsHttpServer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.Objects; import java.util.concurrent.TimeUnit; +import static java.util.Objects.requireNonNull; + public class HiveMQEdgeMain { - private static final Logger log = LoggerFactory.getLogger(HiveMQEdgeMain.class); + private static final @NotNull Logger log = LoggerFactory.getLogger(HiveMQEdgeMain.class); - private @Nullable ConfigurationService configService; private final @NotNull ModuleLoader moduleLoader; private final @NotNull MetricRegistry metricRegistry; private final @NotNull SystemInformation systemInformation; - + private @Nullable ConfigurationService configService; private @Nullable JaxrsHttpServer jaxrsServer; - private @Nullable Injector injector; private @Nullable Thread shutdownThread; @@ -64,22 +61,30 @@ public HiveMQEdgeMain( this.moduleLoader = moduleLoader; } - public void bootstrap() throws HiveMQEdgeStartupException { - // Already bootstrapped. - if (injector != null) { - return; + public static void main(final String @NotNull [] args) throws Exception { + log.info("Starting HiveMQ Edge..."); + final long startTime = System.nanoTime(); + final SystemInformationImpl systemInformation = new SystemInformationImpl(true); + final ModuleLoader moduleLoader = new ModuleLoader(systemInformation); + final HiveMQEdgeMain server = new HiveMQEdgeMain(systemInformation, new MetricRegistry(), null, moduleLoader); + try { + server.start(null); + log.info("Started HiveMQ Edge in {}ms", TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTime)); + } catch (final HiveMQEdgeStartupException e) { + log.error("HiveMQ Edge start was aborted with error.", e); } - final HiveMQEdgeBootstrap bootstrap = - new HiveMQEdgeBootstrap(metricRegistry, systemInformation, moduleLoader, configService); - + } - injector = bootstrap.bootstrap(); - if (configService == null) { - configService = injector.configurationService(); + public void bootstrap() throws HiveMQEdgeStartupException { + if (injector == null) { + injector = + new HiveMQEdgeBootstrap(metricRegistry, systemInformation, moduleLoader, configService).bootstrap(); + if (configService == null) { + configService = injector.configurationService(); + } } } - protected void startGateway(final @Nullable EmbeddedExtension embeddedExtension) throws HiveMQEdgeStartupException { if (injector == null) { throw new HiveMQEdgeStartupException("invalid startup state"); @@ -90,9 +95,7 @@ protected void startGateway(final @Nullable EmbeddedExtension embeddedExtension) throw new HiveMQEdgeStartupException("User aborted."); } - final HiveMQEdgeGateway instance = injector.edgeGateway(); - instance.start(embeddedExtension); - + injector.edgeGateway().start(embeddedExtension); initializeApiServer(injector); startApiServer(); } @@ -102,11 +105,9 @@ protected void stopGateway() { return; } final ShutdownHooks shutdownHooks = injector.shutdownHooks(); - // Already shutdown. if (shutdownHooks.isShuttingDown()) { return; } - shutdownHooks.runShutdownHooks(); //clear metrics @@ -114,13 +115,11 @@ protected void stopGateway() { //Stop the API Webserver stopApiServer(); - LoggingBootstrap.resetLogging(); } protected void initializeApiServer(final @NotNull Injector injector) { - final ApiConfigurationService config = Objects.requireNonNull(configService).apiConfiguration(); - if (jaxrsServer == null && config.isEnabled()) { + if (jaxrsServer == null && requireNonNull(configService).apiConfiguration().isEnabled()) { jaxrsServer = injector.apiServer(); } else { log.info("API is DISABLED by configuration"); @@ -142,7 +141,6 @@ protected void stopApiServer() { protected void afterStart() { afterHiveMQStartBootstrap(); - //hook method } private void afterHiveMQStartBootstrap() { @@ -150,13 +148,11 @@ private void afterHiveMQStartBootstrap() { final Persistences persistences = injector.persistences(); Preconditions.checkNotNull(persistences); Preconditions.checkNotNull(configService); - try { - final AfterHiveMQStartBootstrapService afterHiveMQStartBootstrapService = - AfterHiveMQStartBootstrapServiceImpl.decorate(injector.completeBootstrapService(), + injector.commercialModuleLoaderDiscovery() + .afterHiveMQStart(AfterHiveMQStartBootstrapServiceImpl.decorate(injector.completeBootstrapService(), injector.protocolAdapterManager(), - injector.services().modulesAndExtensionsService()); - injector.commercialModuleLoaderDiscovery().afterHiveMQStart(afterHiveMQStartBootstrapService); + injector.services().modulesAndExtensionsService())); } catch (final Exception e) { log.warn("Error on bootstrapping modules:", e); throw new HiveMQEdgeStartupException(e); @@ -174,31 +170,16 @@ public void start(final @Nullable EmbeddedExtension embeddedExtension) public void stop() { stopGateway(); - try { - Runtime.getRuntime().removeShutdownHook(shutdownThread); - } catch (final IllegalStateException ignored) { - //ignore - } - } - - public static void main(final String @NotNull [] args) throws Exception { - log.info("Starting HiveMQ Edge..."); - final long startTime = System.nanoTime(); - final SystemInformationImpl systemInformation = new SystemInformationImpl(true); - final ModuleLoader moduleLoader = new ModuleLoader(systemInformation); - final HiveMQEdgeMain server = - new HiveMQEdgeMain(systemInformation, new MetricRegistry(), null, moduleLoader); - try { - server.start(null); - log.info("Started HiveMQ Edge in {}ms", TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTime)); - } catch (final HiveMQEdgeStartupException e) { - log.error("HiveMQ Edge start was aborted with error.", e); + if (shutdownThread != null) { + try { + Runtime.getRuntime().removeShutdownHook(shutdownThread); + } catch (final IllegalStateException ignored) { + //ignore + } } } public @Nullable Injector getInjector() { return injector; } - - } diff --git a/hivemq-edge/src/main/java/com/hivemq/api/model/adapters/ProtocolAdapter.java b/hivemq-edge/src/main/java/com/hivemq/api/model/adapters/ProtocolAdapter.java index cc87cc3c5d..40f0ae5bb8 100644 --- a/hivemq-edge/src/main/java/com/hivemq/api/model/adapters/ProtocolAdapter.java +++ b/hivemq-edge/src/main/java/com/hivemq/api/model/adapters/ProtocolAdapter.java @@ -42,28 +42,28 @@ public class ProtocolAdapter { private final @NotNull String name; @JsonProperty("description") @Schema(description = "The description") - private final @NotNull String description; + private final @Nullable String description; @JsonProperty("url") @Schema(description = "The url of the adapter") - private final @NotNull String url; + private final @Nullable String url; @JsonProperty("version") @Schema(description = "The installed version of the adapter") private final @NotNull String version; @JsonProperty("logoUrl") @Schema(description = "The logo of the adapter") - private final @NotNull String logoUrl; + private final @Nullable String logoUrl; @JsonProperty("provisioningUrl") @Schema(description = "The provisioning url of the adapter") - private final @NotNull String provisioningUrl; + private final @Nullable String provisioningUrl; @JsonProperty("author") @Schema(description = "The author of the adapter") private final @NotNull String author; @JsonProperty("installed") @Schema(description = "Is the adapter installed?") - private final @NotNull Boolean installed; + private final @Nullable Boolean installed; @JsonProperty("category") @Schema(description = "The category of the adapter") - private final @NotNull ProtocolAdapterCategory category; + private final @Nullable ProtocolAdapterCategory category; @JsonProperty("tags") @Schema(description = "The search tags associated with this adapter") private final @NotNull List tags; @@ -72,27 +72,27 @@ public class ProtocolAdapter { private final @NotNull Set capabilities; @JsonProperty("configSchema") @Schema(description = "JSONSchema in the 'https://json-schema.org/draft/2020-12/schema' format, which describes the configuration requirements for the adapter.") - private final @NotNull JsonNode configSchema; + private final @Nullable JsonNode configSchema; @JsonProperty("uiSchema") @Schema(description = "UISchema (see https://rjsf-team.github.io/react-jsonschema-form/docs/api-reference/uiSchema/), which describes the UI rendering of the configuration for the adapter.") - private final @NotNull JsonNode uiSchema; + private final @Nullable JsonNode uiSchema; public ProtocolAdapter( @JsonProperty("id") final @NotNull String id, @JsonProperty("protocol") final @NotNull String protocol, @JsonProperty("name") final @NotNull String name, - @JsonProperty("description") final @NotNull String description, - @JsonProperty("url") final @NotNull String url, + @JsonProperty("description") final @Nullable String description, + @JsonProperty("url") final @Nullable String url, @JsonProperty("version") final @NotNull String version, - @JsonProperty("logoUrl") final @NotNull String logoUrl, + @JsonProperty("logoUrl") final @Nullable String logoUrl, @JsonProperty("provisioningUrl") final @Nullable String provisioningUrl, @JsonProperty("author") final @NotNull String author, @JsonProperty("installed") final @Nullable Boolean installed, @JsonProperty("capabilities") final @NotNull Set capabilities, @JsonProperty("category") final @Nullable ProtocolAdapterCategory category, - @JsonProperty("tags") final @Nullable List tags, - @JsonProperty("configSchema") final @NotNull JsonNode configSchema, - @JsonProperty("uiSchema") final @NotNull JsonNode uiSchema) { + @JsonProperty("tags") final @NotNull List tags, + @JsonProperty("configSchema") final @Nullable JsonNode configSchema, + @JsonProperty("uiSchema") final @Nullable JsonNode uiSchema) { this.id = id; this.protocol = protocol; this.name = name; @@ -122,11 +122,11 @@ public ProtocolAdapter( return name; } - public @NotNull String getDescription() { + public @Nullable String getDescription() { return description; } - public @NotNull String getUrl() { + public @Nullable String getUrl() { return url; } @@ -134,7 +134,7 @@ public ProtocolAdapter( return version; } - public @NotNull String getLogoUrl() { + public @Nullable String getLogoUrl() { return logoUrl; } @@ -146,7 +146,7 @@ public ProtocolAdapter( return author; } - public @NotNull JsonNode getConfigSchema() { + public @Nullable JsonNode getConfigSchema() { return configSchema; } @@ -158,7 +158,7 @@ public ProtocolAdapter( return installed; } - public @Nullable List getTags() { + public @NotNull List getTags() { return tags; } @@ -172,7 +172,7 @@ public ProtocolAdapter( @Override public boolean equals(final @Nullable Object o) { - return this == o || o instanceof final ProtocolAdapter that && Objects.equals(id, that.id); + return this == o || (o instanceof final ProtocolAdapter that && Objects.equals(id, that.id)); } @Override diff --git a/hivemq-edge/src/main/java/com/hivemq/api/model/adapters/ProtocolAdapterCategory.java b/hivemq-edge/src/main/java/com/hivemq/api/model/adapters/ProtocolAdapterCategory.java index d99ad75415..131dff87bf 100644 --- a/hivemq-edge/src/main/java/com/hivemq/api/model/adapters/ProtocolAdapterCategory.java +++ b/hivemq-edge/src/main/java/com/hivemq/api/model/adapters/ProtocolAdapterCategory.java @@ -17,15 +17,13 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.hivemq.edge.HiveMQEdgeConstants; +import io.swagger.v3.oas.annotations.media.Schema; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import io.swagger.v3.oas.annotations.media.Schema; /** * A category is a unique entity and represents a curated grouping of a protocol adapter. A protocol adapter * maybe in 1 category. - * - * @author Simon L Johnson */ public class ProtocolAdapterCategory { @@ -34,7 +32,7 @@ public class ProtocolAdapterCategory { description = "The unique name of the category to be used in API communication.", format = "string", minLength = 1, - required = true, + requiredMode = Schema.RequiredMode.REQUIRED, maxLength = HiveMQEdgeConstants.MAX_NAME_LEN, pattern = HiveMQEdgeConstants.NAME_REGEX) private final @NotNull String name; @@ -44,20 +42,16 @@ public class ProtocolAdapterCategory { description = "The display name of the category to be used in HCIs.", format = "string", minLength = 1, - required = true) + requiredMode = Schema.RequiredMode.REQUIRED) private final @NotNull String displayName; @JsonProperty("description") - @Schema(name = "description", - description = "The description associated with the category.", - format = "string") - private final @NotNull String description; + @Schema(name = "description", description = "The description associated with the category.", format = "string") + private final @Nullable String description; @JsonProperty("image") - @Schema(name = "image", - description = "The image associated with the category.", - format = "string") - private final @NotNull String image; + @Schema(name = "image", description = "The image associated with the category.", format = "string") + private final @Nullable String image; public ProtocolAdapterCategory( @JsonProperty("name") final @NotNull String name, @@ -70,19 +64,19 @@ public ProtocolAdapterCategory( this.image = image; } - public String getName() { + public @NotNull String getName() { return name; } - public String getDisplayName() { + public @NotNull String getDisplayName() { return displayName; } - public String getDescription() { + public @Nullable String getDescription() { return description; } - public String getImage() { + public @Nullable String getImage() { return image; } } diff --git a/hivemq-edge/src/main/java/com/hivemq/api/model/components/Module.java b/hivemq-edge/src/main/java/com/hivemq/api/model/components/Module.java index 37aa8cba59..4f2493fec6 100644 --- a/hivemq-edge/src/main/java/com/hivemq/api/model/components/Module.java +++ b/hivemq-edge/src/main/java/com/hivemq/api/model/components/Module.java @@ -18,16 +18,14 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; +import io.swagger.v3.oas.annotations.media.Schema; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import io.swagger.v3.oas.annotations.media.Schema; import java.util.Objects; /** * Bean to transport module details across the API - * - * @author Simon L Johnson */ @JsonIgnoreProperties(ignoreUnknown = true) public class Module { @@ -148,10 +146,7 @@ public Module( @Override public boolean equals(final @Nullable Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Module extension = (Module) o; - return Objects.equals(id, extension.id); + return this == o || (o instanceof final Module that && Objects.equals(id, that.id)); } @Override @@ -161,17 +156,30 @@ public int hashCode() { @Override public @NotNull String toString() { - final StringBuilder sb = new StringBuilder("Module{"); - sb.append("id='").append(id).append('\''); - sb.append(", version='").append(version).append('\''); - sb.append(", name='").append(name).append('\''); - sb.append(", description='").append(description).append('\''); - sb.append(", author='").append(author).append('\''); - sb.append(", priority=").append(priority); - sb.append(", installed=").append(installed); - sb.append(", documentationLink=").append(documentationLink); - sb.append(", provisioningLink=").append(provisioningLink); - sb.append('}'); - return sb.toString(); + return "Module{" + + "id='" + + id + + '\'' + + ", version='" + + version + + '\'' + + ", name='" + + name + + '\'' + + ", description='" + + description + + '\'' + + ", author='" + + author + + '\'' + + ", priority=" + + priority + + ", installed=" + + installed + + ", documentationLink=" + + documentationLink + + ", provisioningLink=" + + provisioningLink + + '}'; } } diff --git a/hivemq-edge/src/main/java/com/hivemq/api/resources/impl/ProtocolAdapterApiUtils.java b/hivemq-edge/src/main/java/com/hivemq/api/resources/impl/ProtocolAdapterApiUtils.java index 91a48d3ca3..aca6df8e52 100644 --- a/hivemq-edge/src/main/java/com/hivemq/api/resources/impl/ProtocolAdapterApiUtils.java +++ b/hivemq-edge/src/main/java/com/hivemq/api/resources/impl/ProtocolAdapterApiUtils.java @@ -39,6 +39,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -95,13 +96,13 @@ public class ProtocolAdapterApiUtils { true, info.getCapabilities() .stream() - .filter(cap -> cap != ProtocolAdapterCapability.WRITE || adapterManager.writingEnabled()) + .filter(cap -> cap != ProtocolAdapterCapability.WRITE || adapterManager.isWritingEnabled()) .map(ProtocolAdapter.Capability::from) .collect(Collectors.toSet()), convertApiCategory(info.getCategory()), - info.getTags() != null ? info.getTags().stream().map(Enum::toString).toList() : null, + info.getTags() != null ? info.getTags().stream().map(Enum::toString).toList() : List.of(), new ProtocolAdapterSchemaManager(objectMapper, - adapterManager.writingEnabled() ? + adapterManager.isWritingEnabled() ? info.configurationClassNorthAndSouthbound() : info.configurationClassNorthbound()).generateSchemaNode(), getUiSchemaForAdapter(objectMapper, info)); @@ -152,10 +153,10 @@ public class ProtocolAdapterApiUtils { getLogoUrl(module, configurationService), module.getProvisioningLink() != null ? module.getProvisioningLink().getUrl() : null, module.getAuthor(), - false, + Boolean.FALSE, Set.of(), null, - null, + List.of(), null, null); } @@ -163,18 +164,18 @@ public class ProtocolAdapterApiUtils { private static @Nullable String getLogoUrl( final @NotNull Module module, final @NotNull ConfigurationService configurationService) { - String logoUrl = null; if (module.getLogoUrl() != null) { - logoUrl = module.getLogoUrl().getUrl(); + final String logoUrl = module.getLogoUrl().getUrl(); if (logoUrl != null) { - logoUrl = logoUrl.startsWith("/") ? "/module" + logoUrl : logoUrl; - logoUrl = applyAbsoluteServerAddressInDeveloperMode(logoUrl, configurationService); + return applyAbsoluteServerAddressInDeveloperMode( + logoUrl.startsWith("/") ? "/module" + logoUrl : logoUrl, + configurationService); } } - return logoUrl; + return null; } - private static @NotNull String getLogoUrl( + private static @Nullable String getLogoUrl( final @NotNull ProtocolAdapterInformation info, final @NotNull ConfigurationService configurationService, final @Nullable String xOriginalURI) { @@ -196,12 +197,13 @@ public class ProtocolAdapterApiUtils { } } logoUrl = applyAbsoluteServerAddressInDeveloperMode(logoUrl, configurationService); + return logoUrl; } else { // although it is marked as not null it is input from outside (possible customer adapter), // so we should trust but validate and at least log. log.warn("Logo url for adapter '{}' was null. ", info.getDisplayName()); + return null; } - return logoUrl; } @VisibleForTesting @@ -226,14 +228,13 @@ public class ProtocolAdapterApiUtils { * * @param category the category enum to convert */ - @org.jetbrains.annotations.VisibleForTesting + @VisibleForTesting public static @Nullable ProtocolAdapterCategory convertApiCategory(final @Nullable com.hivemq.adapter.sdk.api.ProtocolAdapterCategory category) { - if (category == null) { - return null; - } - return new ProtocolAdapterCategory(category.name(), - category.getDisplayName(), - category.getDescription(), - category.getImage()); + return category != null ? + new ProtocolAdapterCategory(category.name(), + category.getDisplayName(), + category.getDescription(), + category.getImage()) : + null; } } diff --git a/hivemq-edge/src/main/java/com/hivemq/api/resources/impl/ProtocolAdaptersResourceImpl.java b/hivemq-edge/src/main/java/com/hivemq/api/resources/impl/ProtocolAdaptersResourceImpl.java index 5ad7affe6f..b4f7c924ab 100644 --- a/hivemq-edge/src/main/java/com/hivemq/api/resources/impl/ProtocolAdaptersResourceImpl.java +++ b/hivemq-edge/src/main/java/com/hivemq/api/resources/impl/ProtocolAdaptersResourceImpl.java @@ -70,6 +70,8 @@ import com.hivemq.edge.api.model.StatusTransitionResult; import com.hivemq.edge.api.model.TagSchema; import com.hivemq.edge.modules.adapters.impl.ProtocolAdapterDiscoveryOutputImpl; +import com.hivemq.persistence.mappings.NorthboundMapping; +import com.hivemq.persistence.mappings.SouthboundMapping; import com.hivemq.persistence.topicfilter.TopicFilterPersistence; import com.hivemq.persistence.topicfilter.TopicFilterPojo; import com.hivemq.protocols.InternalProtocolAdapterWritingService; @@ -325,21 +327,37 @@ public int getDepth() { @SuppressWarnings("unchecked") @Override public @NotNull Response updateAdapter(final @NotNull String adapterId, final @NotNull Adapter adapter) { - return systemInformation.isConfigWriteable() ? - configExtractor.getAdapterByAdapterId(adapterId).map(oldInstance -> { - final ProtocolAdapterEntity newConfig = new ProtocolAdapterEntity(oldInstance.getAdapterId(), - oldInstance.getProtocolId(), - oldInstance.getConfigVersion(), - (Map) adapter.getConfig(), - oldInstance.getNorthboundMappings(), - oldInstance.getSouthboundMappings(), - oldInstance.getTags()); - if (!configExtractor.updateAdapter(newConfig)) { - return adapterCannotBeUpdatedError(adapterId); - } - return Response.ok().build(); - }).orElseGet(adapterNotFoundError(adapterId)) : - errorResponse(new ConfigWritingDisabled()); + if (!systemInformation.isConfigWriteable()) { + return errorResponse(new ConfigWritingDisabled()); + } + + // Validate adapter configuration before updating + final ApiErrorMessages errorMessages = ApiErrorUtils.createErrorContainer(); + validateAdapterSchema(errorMessages, adapter); + if (hasRequestErrors(errorMessages)) { + return errorResponse(new AdapterFailedSchemaValidationError(errorMessages.toErrorList())); + } + + return configExtractor.getAdapterByAdapterId(adapterId).map(oldInstance -> { + try { + final ProtocolAdapterEntity newConfig = new ProtocolAdapterEntity(oldInstance.getAdapterId(), + oldInstance.getProtocolId(), + oldInstance.getConfigVersion(), + (Map) adapter.getConfig(), + oldInstance.getNorthboundMappings(), + oldInstance.getSouthboundMappings(), + oldInstance.getTags()); + if (!configExtractor.updateAdapter(newConfig)) { + return adapterCannotBeUpdatedError(adapterId); + } + return Response.ok().build(); + } catch (final @NotNull IllegalArgumentException e) { + if (e.getCause() instanceof final UnrecognizedPropertyException pe) { + addValidationError(errorMessages, pe.getPropertyName(), "Unknown field on adapter configuration"); + } + return errorResponse(new AdapterFailedSchemaValidationError(errorMessages.toErrorList())); + } + }).orElseGet(adapterNotFoundError(adapterId)); } @Override @@ -380,20 +398,20 @@ public int getDepth() { log.trace("Adapter '{}' was started successfully.", adapterId); } }); - case STOP -> protocolAdapterManager.stopAsync(adapterId, false).whenComplete((result, throwable) -> { + case STOP -> protocolAdapterManager.stopAsync(adapterId).whenComplete((result, throwable) -> { if (throwable != null) { log.error("Failed to stop adapter '{}'.", adapterId, throwable); } else { log.trace("Adapter '{}' was stopped successfully.", adapterId); } }); - case RESTART -> protocolAdapterManager.stopAsync(adapterId, false) - .thenRun(() -> protocolAdapterManager.startAsync(adapterId)) + case RESTART -> protocolAdapterManager.stopAsync(adapterId) + .thenCompose(ignored -> protocolAdapterManager.startAsync(adapterId)) .whenComplete((result, throwable) -> { if (throwable != null) { log.error("Failed to restart adapter '{}'.", adapterId, throwable); } else { - log.trace("Adapter '{}' was restarted successfully.", adapterId); + log.info("Adapter '{}' was restarted successfully.", adapterId); } }); } @@ -560,21 +578,11 @@ public int getDepth() { return adapterNotFoundError(adapterId).get(); }) : errorResponse(new ConfigWritingDisabled()); - - //TODO - -// case ALREADY_USED_BY_ANOTHER_ADAPTER: -// //noinspection DataFlowIssue cant be null here. -// final @NotNull String tagName = domainTagUpdateResult.getErrorMessage(); -// return ErrorResponseUtil.errorResponse(new AlreadyExistsError("The tag '" + -// tagName + -// "' cannot be created since another item already exists with the same id.")); } @Override public @NotNull Response getDomainTags() { final List domainTags = protocolAdapterManager.getDomainTags(); - // empty list is also 200 as discussed. return Response.ok(new DomainTagList().items(domainTags.stream() .map(com.hivemq.persistence.domain.DomainTag::toModel) .toList())).build(); @@ -787,22 +795,31 @@ public int getDepth() { missingTags)); } - return configExtractor.getAdapterByAdapterId(adapterId) - .map(cfg -> new ProtocolAdapterEntity(cfg.getAdapterId(), - cfg.getProtocolId(), - cfg.getConfigVersion(), - cfg.getConfig(), - converted, - cfg.getSouthboundMappings(), - cfg.getTags())) - .map(newCfg -> { - if (!configExtractor.updateAdapter(newCfg)) { - return adapterCannotBeUpdatedError(adapterId); - } - log.info("Successfully updated northbound mappings for adapter '{}'.", adapterId); - return Response.ok(northboundMappings).build(); - }) - .orElseGet(adapterNotUpdatedError(adapterId)); + final List convertedMappings = + converted.stream().map(NorthboundMappingEntity::toPersistence).toList(); + if (protocolAdapterManager.updateNorthboundMappingsHotReload(adapterId, convertedMappings)) { + // update config persistence + return configExtractor.getAdapterByAdapterId(adapterId) + .map(cfg -> new ProtocolAdapterEntity(cfg.getAdapterId(), + cfg.getProtocolId(), + cfg.getConfigVersion(), + cfg.getConfig(), + converted, + cfg.getSouthboundMappings(), + cfg.getTags())) + .map(newCfg -> { + if (!configExtractor.updateAdapter(newCfg, false)) { + return adapterCannotBeUpdatedError(adapterId); + } + log.info("Successfully updated northbound mappings for adapter '{}' via hot-reload.", + adapterId); + return Response.ok(northboundMappings).build(); + }) + .orElseGet(adapterNotUpdatedError(adapterId)); + } else { + log.error("Hot-reload failed for northbound mappings on adapter '{}'", adapterId); + return errorResponse(new InternalServerError("Failed to hot-reload northbound mappings")); + } }; } @@ -825,22 +842,31 @@ public int getDepth() { missingTags)); } - return configExtractor.getAdapterByAdapterId(adapterId) - .map(cfg -> new ProtocolAdapterEntity(cfg.getAdapterId(), - cfg.getProtocolId(), - cfg.getConfigVersion(), - cfg.getConfig(), - cfg.getNorthboundMappings(), - converted, - cfg.getTags())) - .map(newCfg -> { - if (!configExtractor.updateAdapter(newCfg)) { - return adapterCannotBeUpdatedError(adapterId); - } - log.info("Successfully updated fromMappings for adapter '{}'.", adapterId); - return Response.ok(southboundMappings).build(); - }) - .orElseGet(adapterNotUpdatedError(adapterId)); + final List convertedMappings = + converted.stream().map(entity -> entity.toPersistence(objectMapper)).toList(); + if (protocolAdapterManager.updateSouthboundMappingsHotReload(adapterId, convertedMappings)) { + // update config persistence + return configExtractor.getAdapterByAdapterId(adapterId) + .map(cfg -> new ProtocolAdapterEntity(cfg.getAdapterId(), + cfg.getProtocolId(), + cfg.getConfigVersion(), + cfg.getConfig(), + cfg.getNorthboundMappings(), + converted, + cfg.getTags())) + .map(newCfg -> { + if (!configExtractor.updateAdapter(newCfg, false)) { + return adapterCannotBeUpdatedError(adapterId); + } + log.info("Successfully updated southbound mappings for adapter '{}' via hot-reload.", + adapterId); + return Response.ok(southboundMappings).build(); + }) + .orElseGet(adapterNotUpdatedError(adapterId)); + } else { + log.error("Hot-reload failed for southbound mappings on adapter '{}'", adapterId); + return errorResponse(new InternalServerError("Failed to hot-reload southbound mappings")); + } }; } @@ -894,17 +920,14 @@ private void validateAdapterSchema( } private @NotNull Adapter toAdapter(final @NotNull ProtocolAdapterWrapper value) { - final Thread currentThread = Thread.currentThread(); - final ClassLoader ctxClassLoader = currentThread.getContextClassLoader(); - final Map config; - try { - currentThread.setContextClassLoader(value.getAdapterFactory().getClass().getClassLoader()); - config = value.getAdapterFactory().unconvertConfigObject(objectMapper, value.getConfigObject()); - config.put("id", value.getId()); - } finally { - currentThread.setContextClassLoader(ctxClassLoader); - } final String adapterId = value.getId(); + final Map config = + runWithContextLoader(value.getAdapterFactory().getClass().getClassLoader(), () -> { + final Map cfg = + value.getAdapterFactory().unconvertConfigObject(objectMapper, value.getConfigObject()); + cfg.put("id", value.getId()); + return cfg; + }); return new Adapter(adapterId).type(value.getAdapterInformation().getProtocolId()) .config(objectMapper.valueToTree(config)) .status(getAdapterStatusInternal(adapterId)); diff --git a/hivemq-edge/src/main/java/com/hivemq/bootstrap/LoggingBootstrap.java b/hivemq-edge/src/main/java/com/hivemq/bootstrap/LoggingBootstrap.java index b2c8b6b657..a718867932 100644 --- a/hivemq-edge/src/main/java/com/hivemq/bootstrap/LoggingBootstrap.java +++ b/hivemq-edge/src/main/java/com/hivemq/bootstrap/LoggingBootstrap.java @@ -214,6 +214,7 @@ public void onStart(final @NotNull LoggerContext context) { @Override public void onReset(final @NotNull LoggerContext context) { log.trace("logback.xml was changed"); + context.getTurboFilterList().remove(logLevelModifierTurboFilter); context.addTurboFilter(logLevelModifierTurboFilter); } diff --git a/hivemq-edge/src/main/java/com/hivemq/bootstrap/ioc/Injector.java b/hivemq-edge/src/main/java/com/hivemq/bootstrap/ioc/Injector.java index b4357d6497..c70481c268 100644 --- a/hivemq-edge/src/main/java/com/hivemq/bootstrap/ioc/Injector.java +++ b/hivemq-edge/src/main/java/com/hivemq/bootstrap/ioc/Injector.java @@ -57,9 +57,10 @@ import com.hivemq.uns.ioc.UnsServiceModule; import dagger.BindsInstance; import dagger.Component; - import jakarta.inject.Singleton; + import java.util.Set; +import java.util.concurrent.ExecutorService; @SuppressWarnings({"NullabilityAnnotations", "UnusedReturnValue"}) @Component(modules = { @@ -121,7 +122,7 @@ public interface Injector { // UnsServiceModule uns(); -// ExecutorsModule executors(); + ExecutorService executorService(); @Component.Builder interface Builder { @@ -176,5 +177,4 @@ interface Builder { Injector build(); } - } diff --git a/hivemq-edge/src/main/java/com/hivemq/bridge/BridgeService.java b/hivemq-edge/src/main/java/com/hivemq/bridge/BridgeService.java index 9cdb072ce2..32d143e1c6 100644 --- a/hivemq-edge/src/main/java/com/hivemq/bridge/BridgeService.java +++ b/hivemq-edge/src/main/java/com/hivemq/bridge/BridgeService.java @@ -97,7 +97,6 @@ public synchronized void updateBridges(final @NotNull List bridges) toUpdate.removeAll(toRemove); - final long start = System.currentTimeMillis(); if (log.isDebugEnabled()) { log.debug("Updating {} active bridges connections from {} configured connections", @@ -111,7 +110,7 @@ public synchronized void updateBridges(final @NotNull List bridges) toRemove.forEach(bridgeId -> { final var active = activeBridgeNamesToClient.remove(bridgeId); allKnownBridgeConfigs.remove(bridgeId); - if(active != null) { + if (active != null) { log.info("Removing bridge {}", bridgeId); internalStopBridge(active, true, List.of()); } else { @@ -122,15 +121,14 @@ public synchronized void updateBridges(final @NotNull List bridges) toUpdate.forEach(bridgeId -> { final var active = activeBridgeNamesToClient.get(bridgeId); final var newBridge = bridgeIdToConfig.get(bridgeId); - if(active != null) { - if(active.bridge().equals(newBridge)) { + if (active != null) { + if (active.bridge().equals(newBridge)) { log.debug("Not restarting bridge {} because config is unchanged", bridgeId); } else { log.info("Restarting bridge {} because config has changed", bridgeId); allKnownBridgeConfigs.put(bridgeId, newBridge); internalStopBridge(active, true, List.of()); - activeBridgeNamesToClient.put( - bridgeId, + activeBridgeNamesToClient.put(bridgeId, new MqttBridgeAndClient(newBridge, internalStartBridge(newBridge))); } } @@ -140,9 +138,7 @@ public synchronized void updateBridges(final @NotNull List bridges) final var newBridge = bridgeIdToConfig.get(bridgeId); log.info("Adding bridge {}", bridgeId); allKnownBridgeConfigs.put(bridgeId, newBridge); - activeBridgeNamesToClient.put( - bridgeId, - new MqttBridgeAndClient(newBridge, internalStartBridge(newBridge))); + activeBridgeNamesToClient.put(bridgeId, new MqttBridgeAndClient(newBridge, internalStartBridge(newBridge))); }); if (log.isTraceEnabled()) { @@ -157,7 +153,7 @@ public synchronized void updateBridges(final @NotNull List bridges) public synchronized boolean isConnected(final @NotNull String bridgeName) { final var mqttBridgeAndClient = activeBridgeNamesToClient.get(bridgeName); - if(mqttBridgeAndClient != null) { + if (mqttBridgeAndClient != null) { return mqttBridgeAndClient.mqttClient().isConnected(); } return false; @@ -185,19 +181,17 @@ public synchronized void stopBridge( } public synchronized boolean restartBridge( - final @NotNull String bridgeId, final @Nullable MqttBridge newBridgeConfig) { + final @NotNull String bridgeId, + final @Nullable MqttBridge newBridgeConfig) { final var bridgeToClient = activeBridgeNamesToClient.get(bridgeId); if (bridgeToClient != null) { log.info("Restarting bridge '{}'", bridgeId); final List unchangedForwarders = newForwarderIds(newBridgeConfig); stopBridge(bridgeId, true, unchangedForwarders); - final var mqttBridgeAndClient = new MqttBridgeAndClient( - newBridgeConfig, + final var mqttBridgeAndClient = new MqttBridgeAndClient(newBridgeConfig, internalStartBridge(newBridgeConfig != null ? newBridgeConfig : bridgeToClient.bridge())); - activeBridgeNamesToClient.put( - bridgeId, - mqttBridgeAndClient); - if(newBridgeConfig != null) { + activeBridgeNamesToClient.put(bridgeId, mqttBridgeAndClient); + if (newBridgeConfig != null) { allKnownBridgeConfigs.put(bridgeId, newBridgeConfig); } return true; @@ -211,12 +205,8 @@ public synchronized boolean startBridge(final @NotNull String bridgId) { final var bridge = allKnownBridgeConfigs.get(bridgId); if (bridge != null && !activeBridgeNamesToClient.containsKey(bridgId)) { log.info("Starting bridge '{}'", bridgId); - final var mqttBridgeAndClient = new MqttBridgeAndClient( - bridge, - internalStartBridge(bridge)); - activeBridgeNamesToClient.put( - bridgId, - mqttBridgeAndClient); + final var mqttBridgeAndClient = new MqttBridgeAndClient(bridge, internalStartBridge(bridge)); + activeBridgeNamesToClient.put(bridgId, mqttBridgeAndClient); return true; } else { log.debug("Not starting bridge '{}' since it was already started", bridgId); @@ -224,9 +214,10 @@ public synchronized boolean startBridge(final @NotNull String bridgId) { } } - private synchronized void internalStopBridge(final @NotNull MqttBridgeAndClient bridgeAndClient, - final boolean clearQueue, - final @NotNull List retainQueueForForwarders) { + private synchronized void internalStopBridge( + final @NotNull MqttBridgeAndClient bridgeAndClient, + final boolean clearQueue, + final @NotNull List retainQueueForForwarders) { final var start = System.currentTimeMillis(); final var bridgeId = bridgeAndClient.bridge().getId(); final var client = bridgeAndClient.mqttClient(); @@ -247,7 +238,9 @@ private synchronized void internalStopBridge(final @NotNull MqttBridgeAndClient } } - private BridgeMqttClient internalStartBridgeMqttClient(final @NotNull MqttBridge bridge, final @NotNull BridgeMqttClient bridgeMqttClient) { + private BridgeMqttClient internalStartBridgeMqttClient( + final @NotNull MqttBridge bridge, + final @NotNull BridgeMqttClient bridgeMqttClient) { final var start = System.currentTimeMillis(); final var bridgeId = bridge.getId(); final ListenableFuture future = bridgeMqttClient.start(); @@ -261,8 +254,7 @@ public void onSuccess(@Nullable final Void result) { bridgeNameToLastError.remove(bridge.getId()); final HiveMQEdgeRemoteEvent startedEvent = new HiveMQEdgeRemoteEvent(HiveMQEdgeRemoteEvent.EVENT_TYPE.BRIDGE_STARTED); - startedEvent.addUserData("cloudBridge", - String.valueOf(bridge.getHost().endsWith("hivemq.cloud"))); + startedEvent.addUserData("cloudBridge", String.valueOf(bridge.getHost().endsWith("hivemq.cloud"))); startedEvent.addUserData("name", bridgeId); remoteService.fireUsageEvent(startedEvent); Checkpoints.checkpoint("mqtt-bridge-connected"); @@ -274,8 +266,7 @@ public void onFailure(final @NotNull Throwable t) { bridgeNameToLastError.put(bridge.getId(), t); final HiveMQEdgeRemoteEvent errorEvent = new HiveMQEdgeRemoteEvent(HiveMQEdgeRemoteEvent.EVENT_TYPE.BRIDGE_ERROR); - errorEvent.addUserData("cloudBridge", - String.valueOf(bridge.getHost().endsWith("hivemq.cloud"))); + errorEvent.addUserData("cloudBridge", String.valueOf(bridge.getHost().endsWith("hivemq.cloud"))); errorEvent.addUserData("cause", t.getMessage()); errorEvent.addUserData("name", bridgeId); remoteService.fireUsageEvent(errorEvent); @@ -334,5 +325,6 @@ public void run() { } } - public record MqttBridgeAndClient(MqttBridge bridge, BridgeMqttClient mqttClient) {} + public record MqttBridgeAndClient(MqttBridge bridge, BridgeMqttClient mqttClient) { + } } diff --git a/hivemq-edge/src/main/java/com/hivemq/common/executors/ioc/ExecutorsModule.java b/hivemq-edge/src/main/java/com/hivemq/common/executors/ioc/ExecutorsModule.java index 462778c5b7..f53224b53a 100644 --- a/hivemq-edge/src/main/java/com/hivemq/common/executors/ioc/ExecutorsModule.java +++ b/hivemq-edge/src/main/java/com/hivemq/common/executors/ioc/ExecutorsModule.java @@ -17,53 +17,58 @@ import dagger.Module; import dagger.Provides; - import jakarta.inject.Singleton; +import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicInteger; -/** - * @author Simon L Johnson - */ @Module public abstract class ExecutorsModule { - static final String GROUP_NAME = "hivemq-edge-group"; - static final String SCHEDULED_WORKER_GROUP_NAME = "hivemq-edge-scheduled-group"; - static final String CACHED_WORKER_GROUP_NAME = "hivemq-edge-cached-group"; - private static final ThreadGroup coreGroup = new ThreadGroup(GROUP_NAME); + public static final @NotNull String SCHEDULED_WORKER_GROUP_NAME = "hivemq-edge-scheduled-group"; + public static final @NotNull String CACHED_WORKER_GROUP_NAME = "hivemq-edge-cached-group"; + + private static final @NotNull Logger log = LoggerFactory.getLogger(ExecutorsModule.class); + + private static final @NotNull String GROUP_NAME = "hivemq-edge-group"; + private static final int SCHEDULED_WORKER_GROUP_THREAD_COUNT = 4; + private static final @NotNull ThreadGroup coreGroup = new ThreadGroup(GROUP_NAME); @Provides @Singleton - static ScheduledExecutorService scheduledExecutor() { - return Executors.newScheduledThreadPool(4, + static @NotNull ScheduledExecutorService scheduledExecutor() { + return Executors.newScheduledThreadPool(SCHEDULED_WORKER_GROUP_THREAD_COUNT, new HiveMQEdgeThreadFactory(SCHEDULED_WORKER_GROUP_NAME)); } @Provides @Singleton - static ExecutorService executorService() { + static @NotNull ExecutorService executorService() { return Executors.newCachedThreadPool(new HiveMQEdgeThreadFactory(CACHED_WORKER_GROUP_NAME)); } - static class HiveMQEdgeThreadFactory implements ThreadFactory { - private final String factoryName; - private final ThreadGroup group; - private volatile int counter = 0; + private static class HiveMQEdgeThreadFactory implements ThreadFactory { + private final @NotNull String factoryName; + private final @NotNull ThreadGroup group; + private final @NotNull AtomicInteger counter = new AtomicInteger(0); - public HiveMQEdgeThreadFactory(final String factoryName) { + public HiveMQEdgeThreadFactory(final @NotNull String factoryName) { this.factoryName = factoryName; - this.group = new ThreadGroup(coreGroup, factoryName); + this.group = new ThreadGroup(coreGroup, factoryName); } @Override - public Thread newThread(final Runnable r) { - synchronized (group) { - Thread thread = new Thread(coreGroup, r, String.format(factoryName + "-%d", counter++)); - return thread; - } + public @NotNull Thread newThread(final @NotNull Runnable r) { + final Thread thread = new Thread(group, r, factoryName + "-" + counter.getAndIncrement()); + thread.setDaemon(true); + thread.setUncaughtExceptionHandler((t, e) -> log.error("Uncaught exception in thread {}", t.getName(), e)); + return thread; } } } diff --git a/hivemq-edge/src/main/java/com/hivemq/common/shutdown/ShutdownHooks.java b/hivemq-edge/src/main/java/com/hivemq/common/shutdown/ShutdownHooks.java index 8ea920b18b..9c3829e625 100644 --- a/hivemq-edge/src/main/java/com/hivemq/common/shutdown/ShutdownHooks.java +++ b/hivemq-edge/src/main/java/com/hivemq/common/shutdown/ShutdownHooks.java @@ -36,8 +36,6 @@ *

* If you add a shutdown hook, the shutdown hook is added to the registry. Please note that the * synchronous shutdown hook is not executed by itself when HiveMQ is shutting down. - * - * @author Dominik Obermaier */ public class ShutdownHooks { @@ -48,7 +46,6 @@ public class ShutdownHooks { public ShutdownHooks() { shuttingDown = new AtomicBoolean(false); - synchronousHooks = MultimapBuilder.SortedSetMultimapBuilder .treeKeys(Ordering.natural().reverse()) //High priorities first .arrayListValues() diff --git a/hivemq-edge/src/main/java/com/hivemq/configuration/reader/ProtocolAdapterExtractor.java b/hivemq-edge/src/main/java/com/hivemq/configuration/reader/ProtocolAdapterExtractor.java index d47ba6b56e..09918973f4 100644 --- a/hivemq-edge/src/main/java/com/hivemq/configuration/reader/ProtocolAdapterExtractor.java +++ b/hivemq-edge/src/main/java/com/hivemq/configuration/reader/ProtocolAdapterExtractor.java @@ -19,44 +19,44 @@ import com.hivemq.configuration.entity.HiveMQConfigEntity; import com.hivemq.configuration.entity.adapter.ProtocolAdapterEntity; import com.hivemq.configuration.entity.adapter.TagEntity; +import jakarta.xml.bind.ValidationEvent; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import jakarta.xml.bind.ValidationEvent; import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; -import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; -import java.util.stream.Collectors; - -public class ProtocolAdapterExtractor implements ReloadableExtractor, List<@NotNull ProtocolAdapterEntity>> { - private volatile @NotNull List allConfigs = List.of(); - - private final @NotNull Set tagNames = new CopyOnWriteArraySet<>(); - private volatile @Nullable Consumer> consumer = cfg -> log.debug("No consumer registered yet"); +public class ProtocolAdapterExtractor + implements ReloadableExtractor, List<@NotNull ProtocolAdapterEntity>> { + private final @NotNull Set tagNames; private final @NotNull ConfigFileReaderWriter configFileReaderWriter; + private volatile @NotNull List allConfigs; + private volatile @Nullable Consumer> consumer; public ProtocolAdapterExtractor(final @NotNull ConfigFileReaderWriter configFileReaderWriter) { this.configFileReaderWriter = configFileReaderWriter; + this.tagNames = new CopyOnWriteArraySet<>(); + this.allConfigs = List.of(); + this.consumer = cfg -> log.debug("No consumer registered yet"); } public @NotNull List getAllConfigs() { return allConfigs; } - public @NotNull Optional getAdapterByAdapterId(String adapterId) { + public @NotNull Optional getAdapterByAdapterId(final @NotNull String adapterId) { return allConfigs.stream().filter(adapter -> adapter.getAdapterId().equals(adapterId)).findFirst(); } @Override - public synchronized Configurator.@NotNull ConfigResult updateConfig(final HiveMQConfigEntity config) { + public synchronized Configurator.@NotNull ConfigResult updateConfig(final @NotNull HiveMQConfigEntity config) { final List validationEvents = new ArrayList<>(); final var newConfigs = List.copyOf(config.getProtocolAdapterConfig()); newConfigs.forEach(entity -> entity.validate(validationEvents)); @@ -76,17 +76,15 @@ public ProtocolAdapterExtractor(final @NotNull ConfigFileReaderWriter configFile }); } - public synchronized Configurator.ConfigResult updateAllAdapters(final @NotNull List adapterConfigs) { + public synchronized @NotNull Configurator.ConfigResult updateAllAdapters(final @NotNull List adapterConfigs) { final var newConfigs = List.copyOf(adapterConfigs); - return updateTagNames(newConfigs) - .map(duplicates -> Configurator.ConfigResult.ERROR) - .orElseGet(() -> { - replaceConfigsAndTriggerWrite(newConfigs); - return Configurator.ConfigResult.SUCCESS; - }); + return updateTagNames(newConfigs).map(duplicates -> Configurator.ConfigResult.ERROR).orElseGet(() -> { + replaceConfigsAndTriggerWrite(newConfigs); + return Configurator.ConfigResult.SUCCESS; + }); } - private void replaceConfigsAndTriggerWrite(List<@NotNull ProtocolAdapterEntity> newConfigs) { + private void replaceConfigsAndTriggerWrite(final @NotNull List<@NotNull ProtocolAdapterEntity> newConfigs) { allConfigs = newConfigs; notifyConsumer(); configFileReaderWriter.writeConfigWithSync(); @@ -99,68 +97,79 @@ public synchronized boolean addAdapter(final @NotNull ProtocolAdapterEntity prot protocolAdapterConfig.getProtocolId() + "'"); } - return addTagNamesIfNoDuplicates(protocolAdapterConfig.getTags()) - .map(dupes -> { - log.error("Found duplicated tag names: {}", dupes); - return false; - }) - .orElseGet(() -> { - final var newConfigs = new ImmutableList.Builder() - .addAll(allConfigsTemp) - .add(protocolAdapterConfig) - .build(); - replaceConfigsAndTriggerWrite(newConfigs); - return true; - }); + return addTagNamesIfNoDuplicates(protocolAdapterConfig.getTags()).map(dupes -> { + log.error("Found duplicated tag names: {}", dupes); + return false; + }).orElseGet(() -> { + final var newConfigs = new ImmutableList.Builder<@NotNull ProtocolAdapterEntity>().addAll(allConfigsTemp) + .add(protocolAdapterConfig) + .build(); + replaceConfigsAndTriggerWrite(newConfigs); + return true; + }); } public synchronized boolean updateAdapter( final @NotNull ProtocolAdapterEntity protocolAdapterConfig) { + return updateAdapter(protocolAdapterConfig, true); + } + + /** + * Update adapter config, optionally triggering refresh. + * When triggerRefresh=false, this is used for hot-reload operations where the adapter + * has already been updated in-place and we only need to persist the config change. + */ + public synchronized boolean updateAdapter( + final @NotNull ProtocolAdapterEntity protocolAdapterConfig, + final boolean triggerRefresh) { final var duplicateTags = new HashSet(); final var updated = new AtomicBoolean(false); - final var newConfigs = allConfigs - .stream() - .map(oldInstance -> { - if(oldInstance.getAdapterId().equals(protocolAdapterConfig.getAdapterId())) { - return replaceTagNamesIfNoDuplicates(oldInstance.getTags(), protocolAdapterConfig.getTags()) - .map(dupes -> { - duplicateTags.addAll(dupes); - return oldInstance; - }) - .orElseGet(() -> { - updated.set(true); - return protocolAdapterConfig; - }); - } else { - return oldInstance; - } - }).toList(); - if(updated.get()) { - if(!duplicateTags.isEmpty()) { + final var newConfigs = allConfigs.stream().map(oldInstance -> { + if (oldInstance.getAdapterId().equals(protocolAdapterConfig.getAdapterId())) { + return replaceTagNamesIfNoDuplicates(oldInstance.getTags(), + protocolAdapterConfig.getTags()).map(dupes -> { + duplicateTags.addAll(dupes); + return oldInstance; + }).orElseGet(() -> { + updated.set(true); + return protocolAdapterConfig; + }); + } else { + return oldInstance; + } + }).toList(); + if (updated.get()) { + if (!duplicateTags.isEmpty()) { log.error("Found duplicated tag names: {}", duplicateTags); return false; } else { - replaceConfigsAndTriggerWrite(newConfigs); + if (triggerRefresh) { + replaceConfigsAndTriggerWrite(newConfigs); + } else { + // Hot-reload: update config without triggering refresh (adapter already updated) + replaceConfigsWithoutNotify(newConfigs); + } return true; } } return false; } + private void replaceConfigsWithoutNotify(final @NotNull List<@NotNull ProtocolAdapterEntity> newConfigs) { + allConfigs = newConfigs; + configFileReaderWriter.writeConfigWithSync(); + } + public synchronized boolean deleteAdapter(final @NotNull String adapterId) { final var newConfigs = new ArrayList<>(allConfigs); - final var deleted = allConfigs - .stream() - .filter(config -> config.getAdapterId().equals(adapterId)) - .findFirst() - .map(found -> { + final var deleted = + allConfigs.stream().filter(config -> config.getAdapterId().equals(adapterId)).findFirst().map(found -> { newConfigs.remove(found); - tagNames.removeAll(found.getTags().stream().map(TagEntity::getName).toList()); + found.getTags().stream().map(TagEntity::getName).toList().forEach(tagNames::remove); return true; - }) - .orElse(false); + }).orElse(false); - if(deleted) { + if (deleted) { replaceConfigsAndTriggerWrite(List.copyOf(newConfigs)); return true; } @@ -169,25 +178,23 @@ public synchronized boolean deleteAdapter(final @NotNull String adapterId) { private void notifyConsumer() { final var consumer = this.consumer; - if(consumer != null) { + if (consumer != null) { consumer.accept(allConfigs); } } - private Optional> updateTagNames(List entities) { + private @NotNull Optional> updateTagNames(final @NotNull List entities) { final var newTagNames = new HashSet(); final var duplicates = new HashSet(); - entities.stream() - .flatMap(cfg -> - cfg.getTags().stream()).forEach(tag -> { - if (newTagNames.contains(tag.getName())) { - duplicates.add(tag.getName()); - } else { - newTagNames.add(tag.getName()); - } - }); + entities.stream().flatMap(cfg -> cfg.getTags().stream()).forEach(tag -> { + if (newTagNames.contains(tag.getName())) { + duplicates.add(tag.getName()); + } else { + newTagNames.add(tag.getName()); + } + }); - if(!duplicates.isEmpty()) { + if (!duplicates.isEmpty()) { log.error("Duplicate tags detected while updating: {}", duplicates); return Optional.of(duplicates); } @@ -196,7 +203,7 @@ private Optional> updateTagNames(List entitie return Optional.empty(); } - private Optional> addTagNamesIfNoDuplicates(List newTags) { + private @NotNull Optional> addTagNamesIfNoDuplicates(final @NotNull List newTags) { final var newTagNames = new HashSet(); final var duplicates = new HashSet(); newTags.forEach(tag -> { @@ -207,7 +214,7 @@ private Optional> addTagNamesIfNoDuplicates(List newTags) } }); - if(!duplicates.isEmpty()) { + if (!duplicates.isEmpty()) { log.error("Duplicate tags detected while adding: {}", duplicates); return Optional.of(duplicates); } @@ -215,13 +222,15 @@ private Optional> addTagNamesIfNoDuplicates(List newTags) return Optional.empty(); } - private Optional> replaceTagNamesIfNoDuplicates(List oldTags, List newTags) { + private @NotNull Optional> replaceTagNamesIfNoDuplicates( + final @NotNull List oldTags, + final @NotNull List newTags) { final var newTagNames = new HashSet(); final var duplicates = new HashSet(); final var currentTagNames = new HashSet<>(tagNames); - currentTagNames.removeAll(oldTags.stream().map(TagEntity::getName).toList()); + oldTags.stream().map(TagEntity::getName).toList().forEach(currentTagNames::remove); newTags.forEach(tag -> { if (currentTagNames.contains(tag.getName())) { @@ -230,7 +239,7 @@ private Optional> replaceTagNamesIfNoDuplicates(List oldT newTagNames.add(tag.getName()); } }); - if(!duplicates.isEmpty()) { + if (!duplicates.isEmpty()) { log.error("Duplicate tags detected while replacing: {}", duplicates); return Optional.of(duplicates); } @@ -240,13 +249,13 @@ private Optional> replaceTagNamesIfNoDuplicates(List oldT } @Override - public synchronized void registerConsumer(final Consumer> consumer) { + public synchronized void registerConsumer(final @Nullable Consumer> consumer) { this.consumer = consumer; notifyConsumer(); } @Override - public boolean needsRestartWithConfig(final HiveMQConfigEntity config) { + public boolean needsRestartWithConfig(final @NotNull HiveMQConfigEntity config) { return false; } diff --git a/hivemq-edge/src/main/java/com/hivemq/edge/modules/ModuleLoader.java b/hivemq-edge/src/main/java/com/hivemq/edge/modules/ModuleLoader.java index 19a63fc5d0..a20d2a7d20 100644 --- a/hivemq-edge/src/main/java/com/hivemq/edge/modules/ModuleLoader.java +++ b/hivemq-edge/src/main/java/com/hivemq/edge/modules/ModuleLoader.java @@ -20,72 +20,72 @@ import com.hivemq.edge.modules.adapters.impl.IsolatedModuleClassloader; import com.hivemq.extensions.loader.ClassServiceLoader; import com.hivemq.http.handlers.AlternativeClassloadingStaticFileHandler; +import jakarta.inject.Inject; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import jakarta.inject.Inject; import java.io.File; import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Comparator; -import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; public class ModuleLoader { - private static final Logger log = LoggerFactory.getLogger(ModuleLoader.class); - - private final @NotNull SystemInformation systemInformation; - protected final @NotNull Set modules = new HashSet<>(); - protected final @NotNull Comparator fileComparator = (o1, o2) -> { + protected static final @NotNull Comparator fileComparator = (o1, o2) -> { final long delta = o2.lastModified() - o1.lastModified(); - // we cna easily get an overflow within months, so we can not use the delta directly by casting it to integer! - if (delta == 0) { - return 0; - } else if (delta < 0) { - return -1; - } else { - return 1; - } + return delta == 0 ? 0 : delta < 0 ? -1 : 1; }; - - private final @NotNull ClassServiceLoader classServiceLoader = new ClassServiceLoader(); - private final AtomicBoolean loaded = new AtomicBoolean(); + private static final @NotNull Logger log = LoggerFactory.getLogger(ModuleLoader.class); + protected final @NotNull Set modules; + private final @NotNull SystemInformation systemInformation; + private final @NotNull ClassServiceLoader classServiceLoader; + private final @NotNull AtomicBoolean loaded; @Inject public ModuleLoader(final @NotNull SystemInformation systemInformation) { this.systemInformation = systemInformation; + this.classServiceLoader = new ClassServiceLoader(); + this.loaded = new AtomicBoolean(false); + this.modules = Collections.newSetFromMap(new ConcurrentHashMap<>()); + } + + private static void logException(final @NotNull File file, final @NotNull IOException ioException) { + log.warn("Exception with reason {} while reading module file {}", + ioException.getMessage(), + file.getAbsolutePath()); + log.debug("Original exception", ioException); } public void loadModules() { - if (loaded.get()) { - // avoid duplicate loads - return; - } - loaded.set(true); - final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); - if (Boolean.getBoolean(HiveMQEdgeConstants.DEVELOPMENT_MODE)) { - log.info(String.format("Welcome '%s' is starting...", "48 69 76 65 4D 51 45 64 67 65")); - log.warn( - "\n################################################################################################################\n" + - "# You are running HiveMQ Edge in Development Mode and Modules will be loaded from your workspace NOT your #\n" + - "# HIVEMQ_HOME/modules directory. To load runtime modules from your HOME directory please remove #\n" + - "# '-Dhivemq.edge.workspace.modules=true' from your startup script #\n" + - "################################################################################################################"); - loadFromWorkspace(contextClassLoader); - // load the commercial module loader from the workspace folder - // the loadFromWorkspace() will not find it. - log.info("Loading the commercial module loader from workspace."); - loadCommercialModuleLoaderFromWorkSpace(contextClassLoader); - } else { - // the commercial module loader will be found here in case of a "normal" running hivemq edge - loadFromModulesDirectory(contextClassLoader); + if (loaded.compareAndSet(false, true)) { + final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); + if (Boolean.getBoolean(HiveMQEdgeConstants.DEVELOPMENT_MODE)) { + log.info(String.format("Welcome '%s' is starting...", "48 69 76 65 4D 51 45 64 67 65")); + log.warn(""" + + ################################################################################################################ + # You are running HiveMQ Edge in Development Mode and Modules will be loaded from your workspace NOT your # + # HIVEMQ_HOME/modules directory. To load runtime modules from your HOME directory please remove # + # '-Dhivemq.edge.workspace.modules=true' from your startup script # + ################################################################################################################"""); + loadFromWorkspace(contextClassLoader); + // load the commercial module loader from the workspace folder + // the loadFromWorkspace() will not find it. + log.info("Loading the commercial module loader from workspace."); + loadCommercialModuleLoaderFromWorkSpace(contextClassLoader); + } else { + // the commercial module loader will be found here in case of a "normal" running hivemq edge + loadFromModulesDirectory(contextClassLoader); + } } } @@ -98,31 +98,24 @@ private void loadCommercialModuleLoaderFromWorkSpace(final @NotNull ClassLoader return; } - - final File commercialModuleLoaderLibFolder = - new File(commercialModulesRepoRootFolder, "hivemq-edge-commercial-modules-loader/build/libs"); - if (!commercialModuleLoaderLibFolder.exists()) { + final File libs = new File(commercialModulesRepoRootFolder, "hivemq-edge-commercial-modules-loader/build/libs"); + if (!libs.exists()) { log.error("Could not load commercial module loader as the assumed lib folder '{}' does not exist.", - commercialModuleLoaderLibFolder.getAbsolutePath()); + libs.getAbsolutePath()); return; } - - final File[] tmp = commercialModuleLoaderLibFolder.listFiles(file -> file.getName().endsWith("proguarded.jar")); - + final File[] tmp = libs.listFiles(file -> file.getName().endsWith("proguarded.jar")); if (tmp == null || tmp.length == 0) { - log.info("No commercial module loader jar was discovered in libs folder '{}'", - commercialModuleLoaderLibFolder); + log.info("No commercial module loader jar was discovered in libs folder '{}'", libs); return; } - final List potentialCommercialModuleJars = - new ArrayList<>(Arrays.stream(tmp).sorted(fileComparator).toList()); - - final String absolutePathJar = potentialCommercialModuleJars.get(0).getAbsolutePath(); - if (potentialCommercialModuleJars.size() > 1) { + final List jars = new ArrayList<>(Arrays.stream(tmp).sorted(fileComparator).toList()); + final String absolutePathJar = jars.get(0).getAbsolutePath(); + if (jars.size() > 1) { log.debug( "More than one commercial module loader jar was discovered in libs folder '{}'. Clean unwanted jars to avoid loading the wrong version. The first found jar '{}' will be loaded.", - commercialModuleLoaderLibFolder, + libs, absolutePathJar); } else { log.info("Commercial Module jar '{}' was discovered.", absolutePathJar); @@ -138,15 +131,14 @@ private void loadCommercialModulesLoaderJar(final File jarFile, final @NotNull C log.error("", e); } log.info("Loading commercial module loader from {}", jarFile.getAbsoluteFile()); - final IsolatedModuleClassloader isolatedClassloader = - new IsolatedModuleClassloader(urls.toArray(new URL[0]), parentClassloader); - modules.add(new ModuleLoader.EdgeModule(jarFile, isolatedClassloader, false)); + modules.add(new ModuleLoader.EdgeModule(jarFile, + new IsolatedModuleClassloader(urls.toArray(new URL[0]), parentClassloader), + false)); } protected void loadFromWorkspace(final @NotNull ClassLoader parentClassloader) { log.debug("Loading modules from development workspace."); - final File userDir = new File(System.getProperty("user.dir")); - loadFromWorkspace(parentClassloader, userDir); + loadFromWorkspace(parentClassloader, new File(System.getProperty("user.dir"))); } /** @@ -158,7 +150,7 @@ protected void loadFromWorkspace(final @NotNull ClassLoader parentClassloader) { * @param parentClassloader the parent classloader * @param currentDir the current dir */ - protected void loadFromWorkspace(final @NotNull ClassLoader parentClassloader, final @NotNull File currentDir) { + private void loadFromWorkspace(final @NotNull ClassLoader parentClassloader, final @NotNull File currentDir) { if (currentDir.exists() && currentDir.isDirectory()) { if (currentDir.getName().equals("hivemq-edge")) { discoverWorkspaceModule(new File(currentDir, "modules"), parentClassloader); @@ -173,7 +165,7 @@ protected void loadFromWorkspace(final @NotNull ClassLoader parentClassloader, f } } - protected void discoverWorkspaceModule(final File dir, final @NotNull ClassLoader parentClassloader) { + protected void discoverWorkspaceModule(final @NotNull File dir, final @NotNull ClassLoader parentClassloader) { if (dir.exists()) { final File[] files = dir.listFiles(pathname -> pathname.isDirectory() && pathname.canRead() && @@ -196,14 +188,11 @@ protected void discoverWorkspaceModule(final File dir, final @NotNull ClassLoade urls.add(jar.toURI().toURL()); } } - final IsolatedModuleClassloader isolatedClassloader = - new IsolatedModuleClassloader(urls.toArray(new URL[0]), parentClassloader); - modules.add(new EdgeModule(file, isolatedClassloader, true)); + modules.add(new EdgeModule(file, + new IsolatedModuleClassloader(urls.toArray(new URL[0]), parentClassloader), + true)); } catch (final IOException ioException) { - log.warn("Exception with reason {} while reading module file {}", - ioException.getMessage(), - file.getAbsolutePath()); - log.debug("Original exception", ioException); + logException(file, ioException); } } } @@ -225,10 +214,7 @@ protected void loadFromModulesDirectory(final @NotNull ClassLoader parentClasslo log.debug("Ignoring non jar file in module folder {}.", lib.getAbsolutePath()); } } catch (final IOException ioException) { - log.warn("Exception with reason {} while reading module file {}", - ioException.getMessage(), - lib.getAbsolutePath()); - log.debug("Original exception", ioException); + logException(lib, ioException); } } } @@ -256,7 +242,7 @@ protected void loadFromModulesDirectory(final @NotNull ClassLoader parentClasslo } public @NotNull Set getModules() { - return modules; + return Collections.unmodifiableSet(modules); } public void clear() { @@ -264,7 +250,6 @@ public void clear() { } public static class EdgeModule { - private final @NotNull File root; private final @NotNull ClassLoader classloader; diff --git a/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/ProtocolAdapterStateImpl.java b/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/ProtocolAdapterStateImpl.java index 7a87ca2fed..7b08c12d96 100644 --- a/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/ProtocolAdapterStateImpl.java +++ b/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/ProtocolAdapterStateImpl.java @@ -20,38 +20,42 @@ import com.hivemq.adapter.sdk.api.events.model.Payload; import com.hivemq.adapter.sdk.api.state.ProtocolAdapterState; import com.hivemq.edge.modules.api.events.model.EventImpl; +import org.apache.commons.lang3.exception.ExceptionUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.apache.commons.lang3.exception.ExceptionUtils; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; public class ProtocolAdapterStateImpl implements ProtocolAdapterState { + private final @NotNull AtomicReference runtimeStatus; + private final @NotNull AtomicReference connectionStatus; + private final @NotNull AtomicReference<@Nullable String> lastErrorMessage; private final @NotNull EventService eventService; private final @NotNull String adapterId; private final @NotNull String protocolId; - protected @NotNull AtomicReference runtimeStatus = new AtomicReference<>(RuntimeStatus.STOPPED); - protected @NotNull AtomicReference connectionStatus = - new AtomicReference<>(ConnectionStatus.DISCONNECTED); - protected @Nullable String lastErrorMessage; - private final AtomicReference> connectionStatusListener = new AtomicReference<>(); + private final @NotNull AtomicReference> connectionStatusListener; - public ProtocolAdapterStateImpl(final @NotNull EventService eventService, - final @NotNull String adapterId, - final @NotNull String protocolId) { + public ProtocolAdapterStateImpl( + final @NotNull EventService eventService, + final @NotNull String adapterId, + final @NotNull String protocolId) { this.eventService = eventService; this.adapterId = adapterId; this.protocolId = protocolId; + this.runtimeStatus = new AtomicReference<>(RuntimeStatus.STOPPED); + this.connectionStatus = new AtomicReference<>(ConnectionStatus.DISCONNECTED); + this.lastErrorMessage = new AtomicReference<>(null); + this.connectionStatusListener = new AtomicReference<>(); } @Override public boolean setConnectionStatus(final @NotNull ConnectionStatus connectionStatus) { Preconditions.checkNotNull(connectionStatus); final var changed = this.connectionStatus.getAndSet(connectionStatus) != connectionStatus; - if(changed) { + if (changed) { final var listener = connectionStatusListener.get(); - if(listener != null) { + if (listener != null) { listener.accept(connectionStatus); } } @@ -68,52 +72,50 @@ public boolean setConnectionStatus(final @NotNull ConnectionStatus connectionSta * and the errorMessage to that supplied. */ @Override - public void setErrorConnectionStatus( - final @Nullable Throwable t, - final @Nullable String errorMessage) { - final boolean changed = setConnectionStatus(ConnectionStatus.ERROR); - reportErrorMessage( t, errorMessage, changed); + public void setErrorConnectionStatus(final @Nullable Throwable t, final @Nullable String errorMessage) { + reportErrorMessage(t, errorMessage, setConnectionStatus(ConnectionStatus.ERROR)); } - /** - * Sets the last error message associated with the adapter runtime. This is can be sent through the API to - * give an indication of the status of an adapter runtime. - * - * @param errorMessage - */ @Override public void reportErrorMessage( final @Nullable Throwable throwable, final @Nullable String errorMessage, final boolean sendEvent) { - this.lastErrorMessage = errorMessage == null ? throwable == null ? null : throwable.getMessage() : errorMessage; + // Sets the last error message associated with the adapter runtime. + // This is can be sent through the API to give an indication of the + // status of an adapter runtime. + lastErrorMessage.set(errorMessage == null ? throwable == null ? null : throwable.getMessage() : errorMessage); if (sendEvent) { - eventService.createAdapterEvent(adapterId, protocolId) + final var eventBuilder = eventService.createAdapterEvent(adapterId, protocolId) .withSeverity(EventImpl.SEVERITY.ERROR) - .withMessage(String.format("Adapter '%s' encountered an error.", adapterId)) - .withPayload(Payload.ContentType.PLAIN_TEXT, ExceptionUtils.getStackTrace(throwable)) - .fire(); + .withMessage(String.format("Adapter '%s' encountered an error.", adapterId)); + if (throwable != null) { + eventBuilder.withPayload(Payload.ContentType.PLAIN_TEXT, ExceptionUtils.getStackTrace(throwable)); + } else if (errorMessage != null) { + eventBuilder.withPayload(Payload.ContentType.PLAIN_TEXT, errorMessage); + } + eventBuilder.fire(); } } @Override - public void setRuntimeStatus(final @NotNull RuntimeStatus runtimeStatus) { - this.runtimeStatus.set(runtimeStatus); + public @NotNull RuntimeStatus getRuntimeStatus() { + return runtimeStatus.get(); } @Override - public @NotNull RuntimeStatus getRuntimeStatus() { - return this.runtimeStatus.get(); + public void setRuntimeStatus(final @NotNull RuntimeStatus status) { + runtimeStatus.set(status); } @Override public @Nullable String getLastErrorMessage() { - return lastErrorMessage; + return lastErrorMessage.get(); } - public void setConnectionStatusListener(final @NotNull Consumer connectionStatusListener) { - this.connectionStatusListener.set(connectionStatusListener); - connectionStatusListener.accept(connectionStatus.get()); + public void setConnectionStatusListener(final @NotNull Consumer listener) { + final ConnectionStatus currentStatus = connectionStatus.get(); + connectionStatusListener.set(listener); + listener.accept(currentStatus); } - } diff --git a/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/polling/PollingTask.java b/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/polling/PollingTask.java index bfe9f4e653..85ab22daaa 100644 --- a/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/polling/PollingTask.java +++ b/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/polling/PollingTask.java @@ -19,9 +19,9 @@ import com.hivemq.adapter.sdk.api.events.model.Event; import com.hivemq.configuration.service.InternalConfigurations; import com.hivemq.edge.modules.api.adapters.ProtocolAdapterPollingSampler; -import org.jetbrains.annotations.NotNull; import com.hivemq.util.ExceptionUtils; import com.hivemq.util.NanoTimeProvider; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -29,10 +29,12 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; public class PollingTask implements Runnable { @@ -42,12 +44,11 @@ public class PollingTask implements Runnable { private final @NotNull ScheduledExecutorService scheduledExecutorService; private final @NotNull EventService eventService; private final @NotNull NanoTimeProvider nanoTimeProvider; - private final @NotNull AtomicInteger watchdogErrorCount = new AtomicInteger(); - private final @NotNull AtomicInteger applicationErrorCount = new AtomicInteger(); - + private final @NotNull AtomicInteger watchdogErrorCount; + private final @NotNull AtomicInteger applicationErrorCount; + private final @NotNull AtomicBoolean continueScheduling; + private final @NotNull AtomicReference> currentScheduledFuture; private volatile long nanosOfLastPolling; - private final @NotNull AtomicBoolean continueScheduling = new AtomicBoolean(true); - public PollingTask( final @NotNull ProtocolAdapterPollingSampler sampler, @@ -58,6 +59,19 @@ public PollingTask( this.scheduledExecutorService = scheduledExecutorService; this.eventService = eventService; this.nanoTimeProvider = nanoTimeProvider; + this.watchdogErrorCount = new AtomicInteger(); + this.applicationErrorCount = new AtomicInteger(); + this.continueScheduling = new AtomicBoolean(true); + this.currentScheduledFuture = new AtomicReference<>(); + } + + private static long getBackoff(final int errorCount) { + //-- This will backoff up to a max of about a day (unless the max provided is less) + final long max = InternalConfigurations.ADAPTER_RUNTIME_MAX_APPLICATION_ERROR_BACKOFF.get(); + long f = (long) (Math.pow(2, Math.min(errorCount, 20)) * 100); + f += ThreadLocalRandom.current().nextInt(0, errorCount * 100); + f = Math.min(f, max); + return f; } @Override @@ -90,6 +104,10 @@ public void run() { public void stopScheduling() { continueScheduling.set(false); + final ScheduledFuture future = currentScheduledFuture.getAndSet(null); + if (future != null) { + future.cancel(true); + } } private void handleInterruptionException(final @NotNull Throwable throwable) { @@ -99,7 +117,8 @@ private void handleInterruptionException(final @NotNull Throwable throwable) { final var errorCountTotal = watchdogErrorCount.incrementAndGet(); final var stopBecauseOfTooManyErrors = errorCountTotal > InternalConfigurations.ADAPTER_RUNTIME_WATCHDOG_TIMEOUT_ERRORS_BEFORE_INTERRUPT.get(); - final var milliSecondsSinceLastPoll = TimeUnit.NANOSECONDS.toMillis(nanoTimeProvider.nanoTime() - nanosOfLastPolling); + final var milliSecondsSinceLastPoll = + TimeUnit.NANOSECONDS.toMillis(nanoTimeProvider.nanoTime() - nanosOfLastPolling); if (stopBecauseOfTooManyErrors) { log.warn( "Detected bad system process {} in sampler {} - terminating process to maintain health ({}ms runtime)", @@ -121,7 +140,6 @@ private void handleInterruptionException(final @NotNull Throwable throwable) { } } - private void handleExceptionDuringPolling(final @NotNull Throwable throwable) { final int errorCountTotal = applicationErrorCount.incrementAndGet(); final int maxErrorsBeforeRemoval = sampler.getMaxErrorsBeforeRemoval(); @@ -145,11 +163,12 @@ private void handleExceptionDuringPolling(final @NotNull Throwable throwable) { notifyOnError(sampler, throwable, false); // no rescheduling } - } private void notifyOnError( - final @NotNull ProtocolAdapterPollingSampler sampler, final @NotNull Throwable t, final boolean continuing) { + final @NotNull ProtocolAdapterPollingSampler sampler, + final @NotNull Throwable t, + final boolean continuing) { try { sampler.error(t, continuing); } catch (final Throwable samplerError) { @@ -178,12 +197,10 @@ private void reschedule(final int errorCountTotal) { } final long nonNegativeDelay = Math.max(0, delayInMillis); - if (errorCountTotal == 0) { schedule(nonNegativeDelay); } else { - final long backoff = getBackoff(errorCountTotal, - InternalConfigurations.ADAPTER_RUNTIME_MAX_APPLICATION_ERROR_BACKOFF.get()); + final long backoff = getBackoff(errorCountTotal); final long effectiveDelay = Math.max(nonNegativeDelay, backoff); schedule(effectiveDelay); } @@ -193,8 +210,10 @@ private void reschedule(final int errorCountTotal) { void schedule(final long nonNegativeDelay) { if (continueScheduling.get()) { try { - scheduledExecutorService.schedule(this, nonNegativeDelay, TimeUnit.MILLISECONDS); - } catch (final RejectedExecutionException rejectedExecutionException) { + currentScheduledFuture.set(scheduledExecutorService.schedule(this, + nonNegativeDelay, + TimeUnit.MILLISECONDS)); + } catch (final RejectedExecutionException ignored) { // ignore. This is fine during shutdown. } } @@ -204,12 +223,4 @@ private void resetErrorStats() { applicationErrorCount.set(0); watchdogErrorCount.set(0); } - - private static long getBackoff(final int errorCount, final long max) { - //-- This will backoff up to a max of about a day (unless the max provided is less) - long f = (long) (Math.pow(2, Math.min(errorCount, 20)) * 100); - f += ThreadLocalRandom.current().nextInt(0, errorCount * 100); - f = Math.min(f, max); - return f; - } } diff --git a/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/polling/ProtocolAdapterPollingServiceImpl.java b/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/polling/ProtocolAdapterPollingServiceImpl.java index 68d6365c03..a6904f3578 100644 --- a/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/polling/ProtocolAdapterPollingServiceImpl.java +++ b/hivemq-edge/src/main/java/com/hivemq/edge/modules/adapters/impl/polling/ProtocolAdapterPollingServiceImpl.java @@ -21,21 +21,18 @@ import com.hivemq.common.shutdown.ShutdownHooks; import com.hivemq.edge.modules.api.adapters.ProtocolAdapterPollingSampler; import com.hivemq.edge.modules.api.adapters.ProtocolAdapterPollingService; -import org.jetbrains.annotations.NotNull; import com.hivemq.util.NanoTimeProvider; +import jakarta.inject.Inject; +import jakarta.inject.Singleton; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import jakarta.inject.Inject; -import jakarta.inject.Singleton; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -/** - * @author Daniel Krüger - */ @Singleton public class ProtocolAdapterPollingServiceImpl implements ProtocolAdapterPollingService { @@ -60,7 +57,8 @@ public ProtocolAdapterPollingServiceImpl( @Override public void schedulePolling(final @NotNull ProtocolAdapterPollingSampler sampler) { - final PollingTask pollingTask = new PollingTask(sampler, scheduledExecutorService, eventService, nanoTimeProvider); + final PollingTask pollingTask = + new PollingTask(sampler, scheduledExecutorService, eventService, nanoTimeProvider); scheduledExecutorService.schedule(pollingTask, sampler.getInitialDelay(), sampler.getUnit()); samplerToTask.put(sampler, pollingTask); } @@ -82,7 +80,6 @@ public void stopAllPolling() { samplerToTask.keySet().forEach(this::stopPolling); } - private class Shutdown implements HiveMQShutdownHook { @Override public @NotNull String name() { @@ -104,5 +101,4 @@ public void run() { } } } - } diff --git a/hivemq-edge/src/main/java/com/hivemq/embedded/internal/EmbeddedHiveMQImpl.java b/hivemq-edge/src/main/java/com/hivemq/embedded/internal/EmbeddedHiveMQImpl.java index 09cda3fe67..e5e55f11d6 100644 --- a/hivemq-edge/src/main/java/com/hivemq/embedded/internal/EmbeddedHiveMQImpl.java +++ b/hivemq-edge/src/main/java/com/hivemq/embedded/internal/EmbeddedHiveMQImpl.java @@ -27,9 +27,9 @@ import com.hivemq.edge.modules.ModuleLoader; import com.hivemq.embedded.EmbeddedExtension; import com.hivemq.embedded.EmbeddedHiveMQ; +import com.hivemq.util.ThreadFactoryUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import com.hivemq.util.ThreadFactoryUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,22 +43,17 @@ import java.util.concurrent.Future; import java.util.function.Function; -/** - * @author Georg Held - */ class EmbeddedHiveMQImpl implements EmbeddedHiveMQ { private static final @NotNull Logger log = LoggerFactory.getLogger(EmbeddedHiveMQImpl.class); - - private final @NotNull SystemInformationImpl systemInformation; - private final @NotNull MetricRegistry metricRegistry; @VisibleForTesting final @NotNull ExecutorService stateChangeExecutor; + private final @NotNull SystemInformationImpl systemInformation; + private final @NotNull MetricRegistry metricRegistry; private final @Nullable EmbeddedExtension embeddedExtension; + private final @NotNull Function moduleLoaderFactory; private @Nullable ConfigurationService configurationService; private @Nullable HiveMQEdgeMain hiveMQServer; - private final @NotNull Function moduleLoaderFactory; - private @NotNull State currentState = State.STOPPED; private @NotNull State desiredState = State.STOPPED; private @Nullable Exception failedException; @@ -120,13 +115,6 @@ public void close() throws ExecutionException, InterruptedException { shutDownFuture.get(); } - private enum State { - RUNNING, - STOPPED, - FAILED, - CLOSED - } - private void stateChange() { final List> localStartFutures; final List> localStopFutures; @@ -169,9 +157,8 @@ private void stateChange() { final ModuleLoader moduleLoader = moduleLoaderFactory.apply(systemInformation); moduleLoader.loadModules(); - hiveMQServer = new HiveMQEdgeMain(systemInformation, - metricRegistry, - configurationService, moduleLoader); + hiveMQServer = + new HiveMQEdgeMain(systemInformation, metricRegistry, configurationService, moduleLoader); hiveMQServer.bootstrap(); hiveMQServer.start(embeddedExtension); @@ -210,7 +197,6 @@ private void performStop( try { final long startTime = System.currentTimeMillis(); - try { hiveMQServer.stop(); } catch (final Exception ex) { @@ -243,7 +229,8 @@ private void failFutureLists( } private void failFutureList( - final @NotNull Exception exception, final @NotNull List> futures) { + final @NotNull Exception exception, + final @NotNull List> futures) { for (final CompletableFuture future : futures) { future.completeExceptionally(exception); } @@ -293,6 +280,13 @@ private void succeedFutureList(final @NotNull List> futu return hiveMQServer != null ? hiveMQServer.getInjector() : null; } + private enum State { + RUNNING, + STOPPED, + FAILED, + CLOSED + } + private static class AbortedStateChangeException extends Exception { public AbortedStateChangeException(final @NotNull String message) { diff --git a/hivemq-edge/src/main/java/com/hivemq/fsm/ProtocolAdapterFSM.java b/hivemq-edge/src/main/java/com/hivemq/fsm/ProtocolAdapterFSM.java index e54768642f..b75a015794 100644 --- a/hivemq-edge/src/main/java/com/hivemq/fsm/ProtocolAdapterFSM.java +++ b/hivemq-edge/src/main/java/com/hivemq/fsm/ProtocolAdapterFSM.java @@ -25,61 +25,75 @@ import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.LockSupport; +import java.util.function.BiFunction; +import java.util.function.BiPredicate; import java.util.function.Consumer; public abstract class ProtocolAdapterFSM implements Consumer { - private static final @NotNull Logger log = LoggerFactory.getLogger(ProtocolAdapterFSM.class); - - public enum StateEnum { - DISCONNECTED, - CONNECTING, - CONNECTED, - DISCONNECTING, - ERROR_CLOSING, - CLOSING, - ERROR, - CLOSED, - NOT_SUPPORTED - } - - public static final @NotNull Map> possibleTransitions = Map.of( - StateEnum.DISCONNECTED, Set.of(StateEnum.CONNECTING, StateEnum.CONNECTED, StateEnum.CLOSED), //for compatibility, we allow to go from CONNECTING to CONNECTED directly, and allow testing transition to CLOSED - StateEnum.CONNECTING, Set.of(StateEnum.CONNECTED, StateEnum.ERROR, StateEnum.DISCONNECTED), // can go back to DISCONNECTED - StateEnum.CONNECTED, Set.of(StateEnum.DISCONNECTING, StateEnum.CONNECTING, StateEnum.CLOSING, StateEnum.ERROR_CLOSING, StateEnum.DISCONNECTED), // transition to CONNECTING in case of recovery, DISCONNECTED for direct transition - StateEnum.DISCONNECTING, Set.of(StateEnum.DISCONNECTED, StateEnum.CLOSING), // can go to DISCONNECTED or CLOSING - StateEnum.CLOSING, Set.of(StateEnum.CLOSED), - StateEnum.ERROR_CLOSING, Set.of(StateEnum.ERROR), - StateEnum.ERROR, Set.of(StateEnum.CONNECTING, StateEnum.DISCONNECTED), // can recover from error - StateEnum.CLOSED, Set.of(StateEnum.DISCONNECTED, StateEnum.CLOSING) // can restart from closed or go to closing + public static final @NotNull Map> possibleTransitions = Map.of(StateEnum.DISCONNECTED, + Set.of(StateEnum.DISCONNECTED, + StateEnum.CONNECTING, + StateEnum.CONNECTED, + StateEnum.CLOSED, + StateEnum.NOT_SUPPORTED), + //allow idempotent DISCONNECTED->DISCONNECTED transitions; for compatibility, we allow to go from CONNECTING to CONNECTED directly, and allow testing transition to CLOSED; NOT_SUPPORTED for adapters without southbound + StateEnum.CONNECTING, + Set.of(StateEnum.CONNECTING, StateEnum.CONNECTED, StateEnum.ERROR, StateEnum.DISCONNECTED), + // allow idempotent CONNECTING->CONNECTING; can go back to DISCONNECTED + StateEnum.CONNECTED, + Set.of(StateEnum.CONNECTED, + StateEnum.DISCONNECTING, + StateEnum.CONNECTING, + StateEnum.CLOSING, + StateEnum.ERROR_CLOSING, + StateEnum.DISCONNECTED), + // allow idempotent CONNECTED->CONNECTED; transition to CONNECTING in case of recovery, DISCONNECTED for direct transition + StateEnum.DISCONNECTING, + Set.of(StateEnum.DISCONNECTED, StateEnum.CLOSING), + // can go to DISCONNECTED or CLOSING + StateEnum.CLOSING, + Set.of(StateEnum.CLOSED), + StateEnum.ERROR_CLOSING, + Set.of(StateEnum.ERROR), + StateEnum.ERROR, + Set.of(StateEnum.ERROR, StateEnum.CONNECTING, StateEnum.DISCONNECTED), + // allow idempotent ERROR->ERROR; can recover from error + StateEnum.CLOSED, + Set.of(StateEnum.DISCONNECTED, StateEnum.CLOSING), + // can restart from closed or go to closing + StateEnum.NOT_SUPPORTED, + Set.of(StateEnum.NOT_SUPPORTED) + // Terminal state for adapters without southbound support; allow idempotent transitions ); - - public enum AdapterStateEnum { - STARTING, - STARTED, - STOPPING, - STOPPED - } - public static final Map> possibleAdapterStateTransitions = Map.of( - AdapterStateEnum.STOPPED, Set.of(AdapterStateEnum.STARTING), - AdapterStateEnum.STARTING, Set.of(AdapterStateEnum.STARTED, AdapterStateEnum.STOPPED), - AdapterStateEnum.STARTED, Set.of(AdapterStateEnum.STOPPING), - AdapterStateEnum.STOPPING, Set.of(AdapterStateEnum.STOPPED) - ); - - private final AtomicReference northboundState = new AtomicReference<>(StateEnum.DISCONNECTED); - private final AtomicReference southboundState = new AtomicReference<>(StateEnum.DISCONNECTED); - private final AtomicReference adapterState = new AtomicReference<>(AdapterStateEnum.STOPPED); - - private final List> stateTransitionListeners = new CopyOnWriteArrayList<>(); - - public record State(AdapterStateEnum state, StateEnum northbound, StateEnum southbound) { } - - private final String adapterId; + AdapterStateEnum.STOPPED, + Set.of(AdapterStateEnum.STARTING), + AdapterStateEnum.STARTING, + Set.of(AdapterStateEnum.STARTED, AdapterStateEnum.STOPPED), + AdapterStateEnum.STARTED, + Set.of(AdapterStateEnum.STOPPING), + AdapterStateEnum.STOPPING, + Set.of(AdapterStateEnum.STOPPED)); + private static final @NotNull Logger log = LoggerFactory.getLogger(ProtocolAdapterFSM.class); + protected final @NotNull AtomicReference adapterState; + private final @NotNull AtomicReference northboundState; + private final @NotNull AtomicReference southboundState; + private final @NotNull List> stateTransitionListeners; + private final @NotNull String adapterId; public ProtocolAdapterFSM(final @NotNull String adapterId) { this.adapterId = adapterId; + this.northboundState = new AtomicReference<>(StateEnum.DISCONNECTED); + this.southboundState = new AtomicReference<>(StateEnum.DISCONNECTED); + this.adapterState = new AtomicReference<>(AdapterStateEnum.STOPPED); + this.stateTransitionListeners = new CopyOnWriteArrayList<>(); + } + + private static boolean canTransition(final @NotNull StateEnum currentState, final @NotNull StateEnum newState) { + final var allowedTransitions = possibleTransitions.get(currentState); + return allowedTransitions != null && allowedTransitions.contains(newState); } public abstract boolean onStarting(); @@ -90,10 +104,10 @@ public ProtocolAdapterFSM(final @NotNull String adapterId) { // ADAPTER signals public void startAdapter() { - if(transitionAdapterState(AdapterStateEnum.STARTING)) { + if (transitionAdapterState(AdapterStateEnum.STARTING)) { log.debug("Protocol adapter {} starting", adapterId); - if(onStarting()) { - if(!transitionAdapterState(AdapterStateEnum.STARTED)) { + if (onStarting()) { + if (!transitionAdapterState(AdapterStateEnum.STARTED)) { log.warn("Protocol adapter {} already started", adapterId); } } else { @@ -105,9 +119,9 @@ public void startAdapter() { } public void stopAdapter() { - if(transitionAdapterState(AdapterStateEnum.STOPPING)) { + if (transitionAdapterState(AdapterStateEnum.STOPPING)) { onStopping(); - if(!transitionAdapterState(AdapterStateEnum.STOPPED)) { + if (!transitionAdapterState(AdapterStateEnum.STOPPED)) { log.warn("Protocol adapter {} already stopped", adapterId); } } else { @@ -116,66 +130,95 @@ public void stopAdapter() { } public boolean transitionAdapterState(final @NotNull AdapterStateEnum newState) { - final var currentState = adapterState.get(); - if(canTransition(currentState, newState)) { - if(adapterState.compareAndSet(currentState, newState)) { - log.debug("Adapter state transition from {} to {} for adapter {}", currentState, newState, adapterId); - notifyListenersAboutStateTransition(currentState()); - return true; - } - } else { - throw new IllegalStateException("Cannot transition adapter state to " + newState); - } - return false; + return performStateTransition(adapterState, + newState, + this::canTransition, + "Adapter", + (current, next) -> new State(next, northboundState.get(), southboundState.get())); } public boolean transitionNorthboundState(final @NotNull StateEnum newState) { - final var currentState = northboundState.get(); - if(canTransition(currentState, newState)) { - if(northboundState.compareAndSet(currentState, newState)) { - log.debug("Northbound state transition from {} to {} for adapter {}", currentState, newState, adapterId); - notifyListenersAboutStateTransition(currentState()); - return true; - } - } else { - throw new IllegalStateException("Cannot transition northbound state to " + newState); - } - return false; + return performStateTransition(northboundState, + newState, + ProtocolAdapterFSM::canTransition, + "Northbound", + (current, next) -> new State(adapterState.get(), next, southboundState.get())); } public boolean transitionSouthboundState(final @NotNull StateEnum newState) { - final var currentState = southboundState.get(); - if(canTransition(currentState, newState)) { - if(southboundState.compareAndSet(currentState, newState)) { - log.debug("Southbound state transition from {} to {} for adapter {}", currentState, newState, adapterId); - notifyListenersAboutStateTransition(currentState()); - return true; + return performStateTransition(southboundState, + newState, + ProtocolAdapterFSM::canTransition, + "Southbound", + (current, next) -> new State(adapterState.get(), northboundState.get(), next)); + } + + /** + * Generic state transition implementation with retry logic and exponential backoff. + * Eliminates code duplication across adapter, northbound, and southbound transitions. + * + * @param stateRef the atomic reference to the state being transitioned + * @param newState the target state + * @param canTransitionFn function to check if transition is valid + * @param stateName name of the state type for logging + * @param stateSnapshotFn function to create state snapshot after successful transition + * @param the state type (StateEnum or AdapterStateEnum) + * @return true if transition succeeded + * @throws IllegalStateException if transition is not allowed from current state + */ + private boolean performStateTransition( + final @NotNull AtomicReference stateRef, + final @NotNull T newState, + final @NotNull BiPredicate canTransitionFn, + final @NotNull String stateName, + final @NotNull BiFunction stateSnapshotFn) { + int retryCount = 0; + while (true) { + final T currentState = stateRef.get(); + if (canTransitionFn.test(currentState, newState)) { + if (stateRef.compareAndSet(currentState, newState)) { + final State snapshotState = stateSnapshotFn.apply(currentState, newState); + log.debug("{} state transition from {} to {} for adapter {}", + stateName, + currentState, + newState, + adapterId); + notifyListenersAboutStateTransition(snapshotState); + return true; + } + retryCount++; + if (retryCount > 3) { + // progressive backoff: 1μs, 2μs, 4μs, 8μs, ..., capped at 100μs + // reduces CPU consumption and cache line contention under high load + final long backoffNanos = Math.min(1_000L * (1L << (retryCount - 4)), 100_000L); + LockSupport.parkNanos(backoffNanos); + } + // Fast retry for attempts 1-3 (optimizes for low contention case) + } else { + // Transition not allowed from current state + throw new IllegalStateException("Cannot transition " + + stateName.toLowerCase() + + " state to " + + newState); } - } else { - throw new IllegalStateException("Cannot transition southbound state to " + newState); } - return false; } @Override public void accept(final ProtocolAdapterState.ConnectionStatus connectionStatus) { final var transitionResult = switch (connectionStatus) { - case CONNECTED -> - transitionNorthboundState(StateEnum.CONNECTED) && startSouthbound(); - + case CONNECTED -> transitionNorthboundState(StateEnum.CONNECTED) && startSouthbound(); case CONNECTING -> transitionNorthboundState(StateEnum.CONNECTING); case DISCONNECTED -> transitionNorthboundState(StateEnum.DISCONNECTED); case ERROR -> transitionNorthboundState(StateEnum.ERROR); case UNKNOWN -> transitionNorthboundState(StateEnum.DISCONNECTED); case STATELESS -> transitionNorthboundState(StateEnum.NOT_SUPPORTED); }; - if(!transitionResult) { + if (!transitionResult) { log.warn("Failed to transition connection state to {} for adapter {}", connectionStatus, adapterId); } } - // Additional methods to support full state machine functionality - public boolean startDisconnecting() { return transitionNorthboundState(StateEnum.DISCONNECTING); } @@ -184,6 +227,8 @@ public boolean startClosing() { return transitionNorthboundState(StateEnum.CLOSING); } + // Additional methods to support full state machine functionality + public boolean startErrorClosing() { return transitionNorthboundState(StateEnum.ERROR_CLOSING); } @@ -233,7 +278,7 @@ public void unregisterStateTransitionListener(final @NotNull Consumer sta stateTransitionListeners.remove(stateTransitionListener); } - public State currentState() { + public @NotNull State currentState() { return new State(adapterState.get(), northboundState.get(), southboundState.get()); } @@ -241,14 +286,33 @@ private void notifyListenersAboutStateTransition(final @NotNull State newState) stateTransitionListeners.forEach(listener -> listener.accept(newState)); } - private static boolean canTransition(final @NotNull StateEnum currentState, final @NotNull StateEnum newState) { - final var allowedTransitions = possibleTransitions.get(currentState); + private boolean canTransition( + final @NotNull AdapterStateEnum currentState, + final @NotNull AdapterStateEnum newState) { + final var allowedTransitions = possibleAdapterStateTransitions.get(currentState); return allowedTransitions != null && allowedTransitions.contains(newState); } - private static boolean canTransition(final @NotNull AdapterStateEnum currentState, final @NotNull AdapterStateEnum newState) { - final var allowedTransitions = possibleAdapterStateTransitions.get(currentState); - return allowedTransitions != null && allowedTransitions.contains(newState); + public enum StateEnum { + DISCONNECTED, + CONNECTING, + CONNECTED, + DISCONNECTING, + ERROR_CLOSING, + CLOSING, + ERROR, + CLOSED, + NOT_SUPPORTED + } + + public enum AdapterStateEnum { + STARTING, + STARTED, + STOPPING, + STOPPED + } + + public record State(AdapterStateEnum adapter, StateEnum northbound, StateEnum southbound) { } } diff --git a/hivemq-edge/src/main/java/com/hivemq/fsm/state-machine.md b/hivemq-edge/src/main/java/com/hivemq/fsm/state-machine.md index 1596dff640..f72e8f6ee9 100644 --- a/hivemq-edge/src/main/java/com/hivemq/fsm/state-machine.md +++ b/hivemq-edge/src/main/java/com/hivemq/fsm/state-machine.md @@ -26,20 +26,28 @@ Four states control adapter lifecycle. 2. **STARTING** - Initialization in progress - Calls `onStarting()` hook - - Transitions: → STARTED (success), → STOPPED (failure) + - Transitions: → STARTED (success), → STOPPED (failure), → ERROR (non recoverable error) 3. **STARTED** - Adapter operational - - Transitions: → STOPPING + - Transitions: → STOPPING, → ERROR (non recoverable error) 4. **STOPPING** - Cleanup in progress - Calls `onStopping()` hook - Transitions: → STOPPED +5. **ERROR** + - Non-recoverable error, adapter is dead + - This is a terminal state + - Transitions: → STARTING + ### Transition Rules ``` + + ERROR + STOPPED → STARTING → STARTED → STOPPING → STOPPED ↑ ↓ └────────────────────────────────┘ diff --git a/hivemq-edge/src/main/java/com/hivemq/fsm/transitions.md b/hivemq-edge/src/main/java/com/hivemq/fsm/transitions.md index a25f02090b..bb9ac9ac39 100644 --- a/hivemq-edge/src/main/java/com/hivemq/fsm/transitions.md +++ b/hivemq-edge/src/main/java/com/hivemq/fsm/transitions.md @@ -22,8 +22,9 @@ graph TB subgraph Adapter Lifecycle STOPPED -->|startAdapter| STARTING STARTING -->|success| STARTED - STARTING -->|failure| STOPPED + STARTING -->|non recoverable error| ERROR STARTED -->|stopAdapter| STOPPING + STARTED -->|non recoverable error| ERROR STOPPING --> STOPPED end ``` diff --git a/hivemq-edge/src/main/java/com/hivemq/persistence/ScheduledCleanUpService.java b/hivemq-edge/src/main/java/com/hivemq/persistence/ScheduledCleanUpService.java index def255b775..bb2d3ea445 100644 --- a/hivemq-edge/src/main/java/com/hivemq/persistence/ScheduledCleanUpService.java +++ b/hivemq-edge/src/main/java/com/hivemq/persistence/ScheduledCleanUpService.java @@ -23,19 +23,19 @@ import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.hivemq.configuration.service.InternalConfigurationService; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import com.hivemq.persistence.clientqueue.ClientQueuePersistence; import com.hivemq.persistence.clientsession.ClientSessionPersistence; import com.hivemq.persistence.clientsession.ClientSessionSubscriptionPersistence; import com.hivemq.persistence.ioc.annotation.Persistence; import com.hivemq.persistence.retained.RetainedMessagePersistence; import com.hivemq.persistence.util.FutureUtils; +import jakarta.inject.Inject; +import jakarta.inject.Singleton; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import jakarta.inject.Inject; -import jakarta.inject.Singleton; import java.util.concurrent.Callable; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; @@ -50,35 +50,27 @@ * This service is used to remove full remove tombstones that are older than a certain amount of time * It is also used to check if the time to live of publishes, retained messages or client session is expired and mark * those that are expired as tombstones - * - * @author Lukas Brandl */ @Singleton public class ScheduledCleanUpService { - static final int NUMBER_OF_PERSISTENCES = 4; - /** * The counter index that is associated with the client session persistence in the clean up job scheduling logic */ public static final int CLIENT_SESSION_PERSISTENCE_INDEX = 0; - /** * The counter index that is associated with the subscription persistence in the clean up job scheduling logic */ public static final int SUBSCRIPTION_PERSISTENCE_INDEX = 1; - /** * The counter index that is associated with the retained messages persistence in the clean up job scheduling logic */ public static final int RETAINED_MESSAGES_PERSISTENCE_INDEX = 2; - /** * The counter index that is associated with the client queue persistence in the clean up job scheduling logic */ public static final int CLIENT_QUEUE_PERSISTENCE_INDEX = 3; - - + static final int NUMBER_OF_PERSISTENCES = 4; private static final Logger log = LoggerFactory.getLogger(ScheduledCleanUpService.class); private final @NotNull ListeningScheduledExecutorService scheduledExecutorService; @@ -86,20 +78,20 @@ public class ScheduledCleanUpService { private final @NotNull ClientSessionSubscriptionPersistence subscriptionPersistence; private final @NotNull RetainedMessagePersistence retainedMessagePersistence; private final @NotNull ClientQueuePersistence clientQueuePersistence; - - private int bucketIndex = 0; - private int persistenceIndex = 0; private final int persistenceBucketCount; private final int cleanUpJobSchedule; private final int cleanUpTaskTimeoutSec; + private int bucketIndex = 0; + private int persistenceIndex = 0; @Inject - public ScheduledCleanUpService(final @NotNull @Persistence ListeningScheduledExecutorService scheduledExecutorService, - final @NotNull ClientSessionPersistence clientSessionPersistence, - final @NotNull ClientSessionSubscriptionPersistence subscriptionPersistence, - final @NotNull RetainedMessagePersistence retainedMessagePersistence, - final @NotNull ClientQueuePersistence clientQueuePersistence, - final @NotNull InternalConfigurationService internalConfigurationService) { + public ScheduledCleanUpService( + final @NotNull @Persistence ListeningScheduledExecutorService scheduledExecutorService, + final @NotNull ClientSessionPersistence clientSessionPersistence, + final @NotNull ClientSessionSubscriptionPersistence subscriptionPersistence, + final @NotNull RetainedMessagePersistence retainedMessagePersistence, + final @NotNull ClientQueuePersistence clientQueuePersistence, + final @NotNull InternalConfigurationService internalConfigurationService) { this.scheduledExecutorService = scheduledExecutorService; this.clientSessionPersistence = clientSessionPersistence; @@ -123,15 +115,11 @@ synchronized void scheduleCleanUpTask() { if (scheduledExecutorService.isShutdown()) { return; } - final ListenableScheduledFuture schedule = scheduledExecutorService.schedule( - new CleanUpTask( - this, - scheduledExecutorService, - cleanUpTaskTimeoutSec, - bucketIndex, - persistenceIndex), - cleanUpJobSchedule, - TimeUnit.SECONDS); + final ListenableScheduledFuture schedule = scheduledExecutorService.schedule(new CleanUpTask(this, + scheduledExecutorService, + cleanUpTaskTimeoutSec, + bucketIndex, + persistenceIndex), cleanUpJobSchedule, TimeUnit.SECONDS); persistenceIndex = (persistenceIndex + 1) % NUMBER_OF_PERSISTENCES; if (persistenceIndex == 0) { bucketIndex = (bucketIndex + 1) % persistenceBucketCount; @@ -166,11 +154,12 @@ static final class CleanUpTask implements Callable { private final int persistenceIndex; @VisibleForTesting - CleanUpTask(final @NotNull ScheduledCleanUpService scheduledCleanUpService, - final @NotNull ListeningScheduledExecutorService scheduledExecutorService, - final int cleanUpTaskTimeoutSec, - final int bucketIndex, - final int persistenceIndex) { + CleanUpTask( + final @NotNull ScheduledCleanUpService scheduledCleanUpService, + final @NotNull ListeningScheduledExecutorService scheduledExecutorService, + final int cleanUpTaskTimeoutSec, + final int bucketIndex, + final int persistenceIndex) { checkNotNull(scheduledCleanUpService, "Clean up service must not be null"); checkNotNull(scheduledExecutorService, "Executor service must not be null"); this.scheduledCleanUpService = scheduledCleanUpService; @@ -188,11 +177,11 @@ public Void call() { @Override public void onSuccess(final @Nullable Void aVoid) { - scheduledCleanUpService.scheduleCleanUpTask(); + scheduledCleanUpService.scheduleCleanUpTask(); } @Override - public void onFailure(final Throwable throwable) { + public void onFailure(final @NotNull Throwable throwable) { // We expect CancellationExceptions only for timeouts. We don't want to spam the log with // messages that suggest to a customer that something is wrong because the task is actually // still running, but we're going to schedule the next one to ensure progress. @@ -210,7 +199,10 @@ public void onFailure(final Throwable throwable) { // Note that the "cancelled" CleanUpTask is expected to continue running because the implementation // currently doesn't react to a set thread interrupt flag. But we expect this to be a rare case and want // to ensure the progress of other cleanup procedures despite the potential additional load. - Futures.withTimeout(future, cleanUpTaskTimeoutSec, TimeUnit.SECONDS, scheduledExecutorService); + // Only schedule timeout task if executor is not shutting down to avoid RejectedExecutionException + if (!scheduledExecutorService.isShutdown()) { + Futures.withTimeout(future, cleanUpTaskTimeoutSec, TimeUnit.SECONDS, scheduledExecutorService); + } } catch (final Throwable throwable) { log.error("Exception in clean up job ", throwable); scheduledCleanUpService.scheduleCleanUpTask(); diff --git a/hivemq-edge/src/main/java/com/hivemq/persistence/domain/DomainTagAddResult.java b/hivemq-edge/src/main/java/com/hivemq/persistence/domain/DomainTagAddResult.java index 8a0b10e334..d088bf877b 100644 --- a/hivemq-edge/src/main/java/com/hivemq/persistence/domain/DomainTagAddResult.java +++ b/hivemq-edge/src/main/java/com/hivemq/persistence/domain/DomainTagAddResult.java @@ -65,8 +65,7 @@ public DomainTagAddResult( public enum DomainTagPutStatus { SUCCESS(), ALREADY_EXISTS(), - ADAPTER_MISSING() + ADAPTER_MISSING(), + ADAPTER_FAILED_TO_START(), } - - } diff --git a/hivemq-edge/src/main/java/com/hivemq/protocols/AbstractSubscriptionSampler.java b/hivemq-edge/src/main/java/com/hivemq/protocols/AbstractSubscriptionSampler.java index c8d5414f5e..0716427fe1 100644 --- a/hivemq-edge/src/main/java/com/hivemq/protocols/AbstractSubscriptionSampler.java +++ b/hivemq-edge/src/main/java/com/hivemq/protocols/AbstractSubscriptionSampler.java @@ -33,7 +33,6 @@ public abstract class AbstractSubscriptionSampler implements ProtocolAdapterPollingSampler { - private final long initialDelay; private final long period; private final int maxErrorsBeforeRemoval; @@ -49,12 +48,11 @@ public abstract class AbstractSubscriptionSampler implements ProtocolAdapterPoll protected final @NotNull ProtocolAdapterWrapper protocolAdapter; protected final @NotNull EventService eventService; - public AbstractSubscriptionSampler( final @NotNull ProtocolAdapterWrapper protocolAdapter, final @NotNull EventService eventService) { this.protocolAdapter = protocolAdapter; + this.eventService = eventService; this.adapterId = protocolAdapter.getId(); - if (protocolAdapter.getAdapter() instanceof final PollingProtocolAdapter adapter) { this.initialDelay = Math.max(adapter.getPollingIntervalMillis(), 100); this.period = Math.max(adapter.getPollingIntervalMillis(), 10); @@ -66,7 +64,6 @@ public AbstractSubscriptionSampler( } else { throw new IllegalArgumentException("Adapter must be a polling or batch polling protocol adapter"); } - this.eventService = eventService; this.uuid = UUID.randomUUID(); this.created = new Date(); } @@ -88,7 +85,7 @@ protected void onSamplerError( final @NotNull Throwable exception, final boolean continuing) { protocolAdapter.setErrorConnectionStatus(exception, null); if (!continuing) { - protocolAdapter.stopAsync(false); + protocolAdapter.stopAsync(); } } @@ -173,6 +170,4 @@ public void setScheduledFuture(final @NotNull ScheduledFuture future) { public int hashCode() { return Objects.hash(uuid); } - - } diff --git a/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterConfig.java b/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterConfig.java index 07375c919d..ee71662df8 100644 --- a/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterConfig.java +++ b/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterConfig.java @@ -30,12 +30,12 @@ public class ProtocolAdapterConfig { private final @NotNull ProtocolSpecificAdapterConfig adapterConfig; - private final @NotNull List tags; private final @NotNull String adapterId; private final @NotNull String protocolId; private final int configVersion; - private final @NotNull List southboundMappings; - private final @NotNull List northboundMappings; + private @NotNull List tags; + private @NotNull List southboundMappings; + private @NotNull List northboundMappings; public ProtocolAdapterConfig( final @NotNull String adapterId, @@ -87,21 +87,35 @@ public ProtocolAdapterConfig( return tags; } + public void setTags(final @NotNull List tags) { + this.tags = tags; + } + public @NotNull List getNorthboundMappings() { return northboundMappings; } + public void setNorthboundMappings(final @NotNull List northboundMappings) { + this.northboundMappings = northboundMappings; + } + public @NotNull List getSouthboundMappings() { return southboundMappings; } + public void setSouthboundMappings(final @NotNull List southboundMappings) { + this.southboundMappings = southboundMappings; + } + public int getConfigVersion() { return configVersion; } @Override public boolean equals(final Object o) { - if (o == null || getClass() != o.getClass()) return false; + if (o == null || getClass() != o.getClass()) { + return false; + } final ProtocolAdapterConfig that = (ProtocolAdapterConfig) o; return getConfigVersion() == that.getConfigVersion() && Objects.equals(getAdapterConfig(), that.getAdapterConfig()) && diff --git a/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterManager.java b/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterManager.java index 7fc4c32645..9b40b2621a 100644 --- a/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterManager.java +++ b/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterManager.java @@ -25,7 +25,8 @@ import com.hivemq.adapter.sdk.api.factories.ProtocolAdapterFactory; import com.hivemq.adapter.sdk.api.services.ProtocolAdapterMetricsService; import com.hivemq.adapter.sdk.api.state.ProtocolAdapterState; -import com.hivemq.adapter.sdk.api.tag.Tag; +import com.hivemq.common.shutdown.HiveMQShutdownHook; +import com.hivemq.common.shutdown.ShutdownHooks; import com.hivemq.configuration.entity.adapter.ProtocolAdapterEntity; import com.hivemq.configuration.reader.ProtocolAdapterExtractor; import com.hivemq.edge.HiveMQEdgeRemoteService; @@ -39,6 +40,8 @@ import com.hivemq.edge.modules.api.adapters.ProtocolAdapterPollingService; import com.hivemq.persistence.domain.DomainTag; import com.hivemq.persistence.domain.DomainTagAddResult; +import com.hivemq.persistence.mappings.NorthboundMapping; +import com.hivemq.persistence.mappings.SouthboundMapping; import com.hivemq.protocols.northbound.NorthboundConsumerFactory; import jakarta.inject.Inject; import jakarta.inject.Singleton; @@ -53,21 +56,25 @@ import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; +import static com.hivemq.persistence.domain.DomainTagAddResult.DomainTagPutStatus.ADAPTER_FAILED_TO_START; import static com.hivemq.persistence.domain.DomainTagAddResult.DomainTagPutStatus.ADAPTER_MISSING; import static com.hivemq.persistence.domain.DomainTagAddResult.DomainTagPutStatus.ALREADY_EXISTS; +import static java.util.Objects.requireNonNull; +import static java.util.concurrent.CompletableFuture.failedFuture; @SuppressWarnings("unchecked") @Singleton public class ProtocolAdapterManager { - private static final Logger log = LoggerFactory.getLogger(ProtocolAdapterManager.class); + private static final @NotNull Logger log = LoggerFactory.getLogger(ProtocolAdapterManager.class); private final @NotNull Map protocolAdapters; private final @NotNull MetricRegistry metricRegistry; @@ -76,14 +83,16 @@ public class ProtocolAdapterManager { private final @NotNull EventService eventService; private final @NotNull ProtocolAdapterConfigConverter configConverter; private final @NotNull VersionProvider versionProvider; - private final @NotNull ProtocolAdapterPollingService protocolAdapterPollingService; + private final @NotNull ProtocolAdapterPollingService pollingService; private final @NotNull ProtocolAdapterMetrics protocolAdapterMetrics; - private final @NotNull InternalProtocolAdapterWritingService protocolAdapterWritingService; - private final @NotNull ProtocolAdapterFactoryManager protocolAdapterFactoryManager; + private final @NotNull InternalProtocolAdapterWritingService writingService; + private final @NotNull ProtocolAdapterFactoryManager factoryManager; private final @NotNull NorthboundConsumerFactory northboundConsumerFactory; private final @NotNull TagManager tagManager; - private final @NotNull ProtocolAdapterExtractor protocolAdapterConfig; - private final @NotNull ExecutorService executorService; + private final @NotNull ProtocolAdapterExtractor config; + private final @NotNull ExecutorService refreshExecutor; + private final @NotNull ExecutorService sharedAdapterExecutor; + private final @NotNull AtomicBoolean shutdownInitiated; @Inject public ProtocolAdapterManager( @@ -93,55 +102,74 @@ public ProtocolAdapterManager( final @NotNull EventService eventService, final @NotNull ProtocolAdapterConfigConverter configConverter, final @NotNull VersionProvider versionProvider, - final @NotNull ProtocolAdapterPollingService protocolAdapterPollingService, + final @NotNull ProtocolAdapterPollingService pollingService, final @NotNull ProtocolAdapterMetrics protocolAdapterMetrics, - final @NotNull InternalProtocolAdapterWritingService protocolAdapterWritingService, - final @NotNull ProtocolAdapterFactoryManager protocolAdapterFactoryManager, + final @NotNull InternalProtocolAdapterWritingService writingService, + final @NotNull ProtocolAdapterFactoryManager factoryManager, final @NotNull NorthboundConsumerFactory northboundConsumerFactory, final @NotNull TagManager tagManager, - final @NotNull ProtocolAdapterExtractor protocolAdapterConfig) { + final @NotNull ProtocolAdapterExtractor config, + final @NotNull ExecutorService sharedAdapterExecutor, + final @NotNull ShutdownHooks shutdownHooks) { this.metricRegistry = metricRegistry; this.moduleServices = moduleServices; this.remoteService = remoteService; this.eventService = eventService; this.configConverter = configConverter; this.versionProvider = versionProvider; - this.protocolAdapterPollingService = protocolAdapterPollingService; + this.pollingService = pollingService; this.protocolAdapterMetrics = protocolAdapterMetrics; - this.protocolAdapterWritingService = protocolAdapterWritingService; - this.protocolAdapterFactoryManager = protocolAdapterFactoryManager; + this.writingService = writingService; + this.factoryManager = factoryManager; this.northboundConsumerFactory = northboundConsumerFactory; this.tagManager = tagManager; - this.protocolAdapterConfig = protocolAdapterConfig; + this.config = config; + this.sharedAdapterExecutor = sharedAdapterExecutor; this.protocolAdapters = new ConcurrentHashMap<>(); - this.executorService = Executors.newSingleThreadExecutor(); - Runtime.getRuntime().addShutdownHook(new Thread(executorService::shutdown)); - protocolAdapterWritingService.addWritingChangedCallback(() -> protocolAdapterFactoryManager.writingEnabledChanged( - protocolAdapterWritingService.writingEnabled())); - } + this.refreshExecutor = Executors.newSingleThreadExecutor(); + this.shutdownInitiated = new AtomicBoolean(false); + shutdownHooks.add(new HiveMQShutdownHook() { + @Override + public @NotNull String name() { + return "protocol-adapter-manager-shutdown"; + } - // API, must be threadsafe + @Override + public void run() { + shutdown(); + } + }); + this.writingService.addWritingChangedCallback(() -> factoryManager.writingEnabledChanged(writingService.writingEnabled())); + } - private static void syncFuture(final @NotNull Future future) { + public static @NotNull T runWithContextLoader( + final @NotNull ClassLoader ctxLoader, + final @NotNull Supplier snippet) { + final Thread t = Thread.currentThread(); + final ClassLoader currentCtx = t.getContextClassLoader(); try { - future.get(); - } catch (final InterruptedException e) { - Thread.currentThread().interrupt(); - log.error("Interrupted while async execution: ", e.getCause()); - } catch (final ExecutionException e) { - log.error("Exception happened while async execution: ", e.getCause()); + t.setContextClassLoader(ctxLoader); + return snippet.get(); + } finally { + t.setContextClassLoader(currentCtx); } } - public static @NotNull T runWithContextLoader( - final @NotNull ClassLoader contextLoader, - final @NotNull Supplier wrapperSupplier) { - final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); + private static @NotNull Boolean updateMappingsHotReload( + final @NotNull ProtocolAdapterWrapper wrapper, + final @NotNull String mappingType, + final @NotNull Runnable updateOperation) { try { - Thread.currentThread().setContextClassLoader(contextLoader); - return wrapperSupplier.get(); - } finally { - Thread.currentThread().setContextClassLoader(contextClassLoader); + log.debug("Updating {} mappings for adapter '{}' via hot-reload", mappingType, wrapper.getId()); + updateOperation.run(); + log.info("Successfully updated {} mappings for adapter '{}' via hot-reload", mappingType, wrapper.getId()); + return Boolean.TRUE; + } catch (final IllegalStateException e) { + log.error("Cannot hot-reload {} mappings, adapter not in correct state: {}", mappingType, e.getMessage()); + return Boolean.FALSE; + } catch (final Throwable e) { + log.error("Exception happened while updating {} mappings via hot-reload: ", mappingType, e); + return Boolean.FALSE; } } @@ -149,23 +177,28 @@ public void start() { if (log.isDebugEnabled()) { log.debug("Starting adapters"); } - protocolAdapterConfig.registerConsumer(this::refresh); + config.registerConsumer(this::refresh); } - public void refresh(final @NotNull List configs) { - executorService.submit(() -> { + private void refresh(final @NotNull List configs) { + if (shutdownInitiated.get() || refreshExecutor.isShutdown()) { + log.debug("Skipping refresh because manager is shutting down"); + return; + } + + refreshExecutor.submit(() -> { log.info("Refreshing adapters"); final Map protocolAdapterConfigs = configs.stream() .map(configConverter::fromEntity) .collect(Collectors.toMap(ProtocolAdapterConfig::getAdapterId, Function.identity())); - final List loadListOfAdapterNames = new ArrayList<>(protocolAdapterConfigs.keySet()); + final List loadedAdapterIds = new ArrayList<>(protocolAdapterConfigs.keySet()); final List adaptersToBeDeleted = new ArrayList<>(protocolAdapters.keySet()); - adaptersToBeDeleted.removeAll(loadListOfAdapterNames); + adaptersToBeDeleted.removeAll(loadedAdapterIds); - final List adaptersToBeCreated = new ArrayList<>(loadListOfAdapterNames); + final List adaptersToBeCreated = new ArrayList<>(loadedAdapterIds); adaptersToBeCreated.removeAll(protocolAdapters.keySet()); final List adaptersToBeUpdated = new ArrayList<>(protocolAdapters.keySet()); @@ -179,17 +212,19 @@ public void refresh(final @NotNull List configs) { if (log.isDebugEnabled()) { log.debug("Deleting adapter '{}'", name); } - stopAsync(name, true).whenComplete((ignored, t) -> deleteAdapterInternal(name)).get(); + stopAsync(name).handle((result, throwable) -> { + deleteAdapterInternal(name); + return null; + }).get(); } catch (final InterruptedException e) { Thread.currentThread().interrupt(); failedAdapters.add(name); log.error("Interrupted while deleting adapter {}", name, e); - } catch (final ExecutionException e) { + } catch (final Throwable e) { failedAdapters.add(name); log.error("Failed deleting adapter {}", name, e); } }); - adaptersToBeCreated.forEach(name -> { try { if (log.isDebugEnabled()) { @@ -201,12 +236,11 @@ public void refresh(final @NotNull List configs) { Thread.currentThread().interrupt(); failedAdapters.add(name); log.error("Interrupted while adding adapter {}", name, e); - } catch (final ExecutionException e) { + } catch (final Throwable e) { failedAdapters.add(name); log.error("Failed adding adapter {}", name, e); } }); - adaptersToBeUpdated.forEach(name -> { try { final var wrapper = protocolAdapters.get(name); @@ -214,28 +248,35 @@ public void refresh(final @NotNull List configs) { log.error( "Existing adapters were modified while a refresh was ongoing, adapter with name '{}' was deleted and could not be updated", name); + return; } - if (wrapper != null && !protocolAdapterConfigs.get(name).equals(wrapper.getConfig())) { - if (log.isDebugEnabled()) { - log.debug("Updating adapter '{}'", name); - } - stopAsync(name, true).thenApply(v -> { - deleteAdapterInternal(name); - return null; - }) - .thenCompose(ignored -> startAsync(createAdapterInternal(protocolAdapterConfigs.get(name), - versionProvider.getVersion()))) - .get(); - } else { - if (log.isDebugEnabled()) { - log.debug("Not-updating adapter '{}' since the config is unchanged", name); + + if (!protocolAdapterConfigs.get(name).equals(wrapper.getConfig())) { + if (wrapper.getRuntimeStatus() != ProtocolAdapterState.RuntimeStatus.STARTED) { + if (log.isDebugEnabled()) { + log.debug("Updating config for stopped adapter '{}' without starting", name); + } + deleteAdapterInternal(name); + createAdapterInternal(protocolAdapterConfigs.get(name), versionProvider.getVersion()); + } else { + if (log.isDebugEnabled()) { + log.debug("Updating adapter '{}'", name); + } + // TODO is this correct + stopAsync(name).thenApply(v -> { + deleteAdapterInternal(name); + return null; + }) + .thenCompose(ignored -> startAsync(createAdapterInternal(protocolAdapterConfigs.get( + name), versionProvider.getVersion()))) + .get(); } } } catch (final InterruptedException e) { Thread.currentThread().interrupt(); failedAdapters.add(name); log.error("Interrupted while updating adapter {}", name, e); - } catch (final ExecutionException e) { + } catch (final Throwable e) { failedAdapters.add(name); log.error("Failed updating adapter {}", name, e); } @@ -255,34 +296,194 @@ public void refresh(final @NotNull List configs) { }); } - //INTERNAL - public boolean protocolAdapterFactoryExists(final @NotNull String protocolAdapterType) { Preconditions.checkNotNull(protocolAdapterType); - return protocolAdapterFactoryManager.get(protocolAdapterType).isPresent(); + return factoryManager.get(protocolAdapterType).isPresent(); + } + + public @NotNull CompletableFuture startAsync(final @NotNull String adapterId) { + Preconditions.checkNotNull(adapterId); + return getProtocolAdapterWrapperByAdapterId(adapterId).map(this::startAsync) + .orElseGet(() -> failedFuture(new ProtocolAdapterException("Adapter '" + adapterId + "'not found."))); + } + + public @NotNull CompletableFuture stopAsync(final @NotNull String adapterId) { + Preconditions.checkNotNull(adapterId); + return getProtocolAdapterWrapperByAdapterId(adapterId).map(this::stopAsync) + .orElseGet(() -> failedFuture(new ProtocolAdapterException("Adapter '" + adapterId + "'not found."))); + } + + public @NotNull Optional getProtocolAdapterWrapperByAdapterId(final @NotNull String adapterId) { + Preconditions.checkNotNull(adapterId); + return Optional.ofNullable(protocolAdapters.get(adapterId)); + } + + public @NotNull Optional getAdapterTypeById(final @NotNull String typeId) { + Preconditions.checkNotNull(typeId); + return Optional.ofNullable(getAllAvailableAdapterTypes().get(typeId)); + } + + public @NotNull Map getAllAvailableAdapterTypes() { + return factoryManager.getAllAvailableAdapterTypes(); + } + + public @NotNull Map getProtocolAdapters() { + return Map.copyOf(protocolAdapters); + } + + public boolean isWritingEnabled() { + return writingService.writingEnabled(); + } + + public @NotNull DomainTagAddResult addDomainTag( + final @NotNull String adapterId, + final @NotNull DomainTag domainTag) { + return getProtocolAdapterWrapperByAdapterId(adapterId).map(wrapper -> { + final var tags = new ArrayList<>(wrapper.getTags()); + final boolean alreadyExists = tags.stream().anyMatch(t -> t.getName().equals(domainTag.getTagName())); + if (alreadyExists) { + return DomainTagAddResult.failed(ALREADY_EXISTS, adapterId); + } + + try { + final var convertedTag = + configConverter.domainTagToTag(wrapper.getProtocolAdapterInformation().getProtocolId(), + domainTag); + + // Use hot-reload to add tag without restarting the adapter + log.debug("Adding tag '{}' to adapter '{}' via hot-reload", domainTag.getTagName(), adapterId); + wrapper.addTagHotReload(convertedTag, eventService); + + log.info("Successfully added tag '{}' to adapter '{}' via hot-reload", + domainTag.getTagName(), + adapterId); + return DomainTagAddResult.success(); + } catch (final IllegalStateException e) { + log.error("Cannot hot-reload tag, adapter not in correct state: {}", e.getMessage()); + return DomainTagAddResult.failed(ADAPTER_FAILED_TO_START, adapterId); + } catch (final Throwable e) { + log.error("Exception happened while adding tag via hot-reload: ", e); + return DomainTagAddResult.failed(ADAPTER_FAILED_TO_START, adapterId); + } + }).orElse(DomainTagAddResult.failed(ADAPTER_MISSING, adapterId)); + } + + public boolean updateNorthboundMappingsHotReload( + final @NotNull String adapterId, + final @NotNull List northboundMappings) { + return getProtocolAdapterWrapperByAdapterId(adapterId).map(wrapper -> updateMappingsHotReload(wrapper, + "northbound", + () -> wrapper.updateMappingsHotReload(northboundMappings, null, eventService))).orElse(Boolean.FALSE); + } + + public boolean updateSouthboundMappingsHotReload( + final @NotNull String adapterId, + final @NotNull List southboundMappings) { + return getProtocolAdapterWrapperByAdapterId(adapterId).map(wrapper -> updateMappingsHotReload(wrapper, + "southbound", + () -> wrapper.updateMappingsHotReload(null, southboundMappings, eventService))).orElse(Boolean.FALSE); + } + + public @NotNull List getDomainTags() { + return protocolAdapters.values() + .stream() + .flatMap(wrapper -> wrapper.getTags() + .stream() + .map(tag -> new DomainTag(tag.getName(), + wrapper.getId(), + tag.getDescription(), + configConverter.convertTagDefinitionToJsonNode(tag.getDefinition())))) + .toList(); + } + + public @NotNull Optional getDomainTagByName(final @NotNull String tagName) { + return protocolAdapters.values() + .stream() + .flatMap(wrapper -> wrapper.getTags() + .stream() + .filter(t -> t.getName().equals(tagName)) + .map(tag -> new DomainTag(tag.getName(), + wrapper.getId(), + tag.getDescription(), + configConverter.convertTagDefinitionToJsonNode(tag.getDefinition())))) + .findFirst(); + } + + public @NotNull Optional> getTagsForAdapter(final @NotNull String adapterId) { + return getProtocolAdapterWrapperByAdapterId(adapterId).map(adapterToConfig -> adapterToConfig.getTags() + .stream() + .map(tag -> new DomainTag(tag.getName(), + adapterToConfig.getId(), + tag.getDescription(), + configConverter.convertTagDefinitionToJsonNode(tag.getDefinition()))) + .toList()); } - public @NotNull CompletableFuture startAsync(final @NotNull String protocolAdapterId) { - Preconditions.checkNotNull(protocolAdapterId); - return getProtocolAdapterWrapperByAdapterId(protocolAdapterId).map(this::startAsync) - .orElseGet(() -> CompletableFuture.failedFuture(new ProtocolAdapterException("Adapter '" + - protocolAdapterId + - "'not found."))); + private void shutdown() { + if (shutdownInitiated.compareAndSet(false, true)) { + shutdownRefreshExecutor(); + + log.info("Initiating shutdown of Protocol Adapter Manager"); + final List adaptersToStop = new ArrayList<>(protocolAdapters.values()); + if (adaptersToStop.isEmpty()) { + log.debug("No adapters to stop during shutdown"); + return; + } + + // initiate stop for all adapters + final List> stopFutures = new ArrayList<>(); + for (final ProtocolAdapterWrapper wrapper : adaptersToStop) { + try { + log.debug("Initiating stop for adapter '{}'", wrapper.getId()); + stopFutures.add(wrapper.stopAsync()); + } catch (final Exception e) { + log.error("Error initiating stop for adapter '{}' during shutdown", wrapper.getId(), e); + } + } + // wait for all adapters to stop, with timeout + try { + CompletableFuture.allOf(stopFutures.toArray(new CompletableFuture[0])).get(15, TimeUnit.SECONDS); + log.info("All adapters stopped successfully during shutdown"); + } catch (final TimeoutException e) { + log.warn("Timeout waiting for adapters to stop during shutdown"); + for (int i = 0; i < stopFutures.size(); i++) { + if (!stopFutures.get(i).isDone()) { + log.warn("Adapter '{}' did not complete stop operation", adaptersToStop.get(i).getId()); + } + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + log.error("Interrupted while waiting for adapters to stop during shutdown", e); + } catch (final Throwable e) { + log.error("Error occurred while stopping adapters during shutdown", e.getCause()); + } + log.info("Protocol Adapter Manager shutdown completed"); + } } - public @NotNull CompletableFuture stopAsync(final @NotNull String protocolAdapterId, final boolean destroy) { - Preconditions.checkNotNull(protocolAdapterId); - return getProtocolAdapterWrapperByAdapterId(protocolAdapterId).map(wrapper -> stopAsync(wrapper, destroy)) - .orElseGet(() -> CompletableFuture.failedFuture(new ProtocolAdapterException("Adapter '" + - protocolAdapterId + - "'not found."))); + private void shutdownRefreshExecutor() { + final String name = "protocol-adapter-manager-refresh"; + final int timeoutSeconds = 5; + log.debug("Shutting {} executor service", name); + refreshExecutor.shutdown(); + try { + if (!refreshExecutor.awaitTermination(timeoutSeconds, TimeUnit.SECONDS)) { + refreshExecutor.shutdownNow(); + if (!refreshExecutor.awaitTermination(2, TimeUnit.SECONDS)) { + log.error("Executor service {} still has running tasks after forced shutdown", name); + } + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + log.warn("Interrupted while waiting for executor service {} to terminate", name); + refreshExecutor.shutdownNow(); + } } private @NotNull ProtocolAdapterWrapper createAdapterInternal( final @NotNull ProtocolAdapterConfig config, final @NotNull String version) { return protocolAdapters.computeIfAbsent(config.getAdapterId(), ignored -> { - final String configProtocolId = config.getProtocolId(); // legacy handling, hardcoded here, to not add legacy stuff into the adapter-sdk final String adapterType = switch (configProtocolId) { @@ -292,12 +493,11 @@ public boolean protocolAdapterFactoryExists(final @NotNull String protocolAdapte default -> configProtocolId; }; - final Optional> maybeFactory = protocolAdapterFactoryManager.get(adapterType); + final Optional> maybeFactory = factoryManager.get(adapterType); if (maybeFactory.isEmpty()) { throw new IllegalArgumentException("Protocol adapter for config " + adapterType + " not found."); } final ProtocolAdapterFactory factory = maybeFactory.get(); - log.info("Found configuration for adapter {} / {}", config.getAdapterId(), adapterType); config.missingTags().ifPresent(missing -> { throw new IllegalArgumentException("Tags used in mappings but not configured in adapter " + @@ -307,19 +507,16 @@ public boolean protocolAdapterFactoryExists(final @NotNull String protocolAdapte }); return runWithContextLoader(factory.getClass().getClassLoader(), () -> { - final ProtocolAdapterMetricsService metricsService = new ProtocolAdapterMetricsServiceImpl(configProtocolId, config.getAdapterId(), metricRegistry); - - final ProtocolAdapterStateImpl state = - new ProtocolAdapterStateImpl(moduleServices.eventService(), config.getAdapterId(), configProtocolId); - + final ProtocolAdapterStateImpl state = new ProtocolAdapterStateImpl(moduleServices.eventService(), + config.getAdapterId(), + configProtocolId); final ModuleServicesPerModuleImpl perModule = new ModuleServicesPerModuleImpl(moduleServices.adapterPublishService(), eventService, - protocolAdapterWritingService, + writingService, tagManager); - final ProtocolAdapter protocolAdapter = factory.createAdapter(factory.getInformation(), new ProtocolAdapterInputImpl(configProtocolId, config.getAdapterId(), @@ -332,18 +529,18 @@ public boolean protocolAdapterFactoryExists(final @NotNull String protocolAdapte metricsService)); // hen-egg problem. Rather solve this here as have not final fields in the adapter. perModule.setAdapter(protocolAdapter); - protocolAdapterMetrics.increaseProtocolAdapterMetric(configProtocolId); return new ProtocolAdapterWrapper(metricsService, - protocolAdapterWritingService, - protocolAdapterPollingService, + writingService, + pollingService, config, protocolAdapter, factory, factory.getInformation(), state, northboundConsumerFactory, - tagManager); + tagManager, + sharedAdapterExecutor); }); }); } @@ -366,16 +563,16 @@ private void deleteAdapterInternal(final @NotNull String adapterId) { Preconditions.checkNotNull(wrapper); final String wid = wrapper.getId(); log.info("Starting protocol-adapter '{}'.", wid); - return wrapper.startAsync(writingEnabled(), moduleServices).whenComplete((result, throwable) -> { + return requireNonNull(wrapper.startAsync(moduleServices)).whenComplete((result, throwable) -> { if (throwable == null) { log.info("Protocol-adapter '{}' started successfully.", wid); - fireEvent(wrapper, + fireStartEvent(wrapper, HiveMQEdgeRemoteEvent.EVENT_TYPE.ADAPTER_STARTED, Event.SEVERITY.INFO, "Adapter '" + wid + "' started OK."); } else { log.warn("Protocol-adapter '{}' could not be started, reason: {}", wid, "unknown"); - fireEvent(wrapper, + fireStartEvent(wrapper, HiveMQEdgeRemoteEvent.EVENT_TYPE.ADAPTER_ERROR, Event.SEVERITY.CRITICAL, "Error starting adapter '" + wid + "'."); @@ -383,7 +580,7 @@ private void deleteAdapterInternal(final @NotNull String adapterId) { }); } - private void fireEvent( + private void fireStartEvent( final @NotNull ProtocolAdapterWrapper wrapper, final @NotNull HiveMQEdgeRemoteEvent.EVENT_TYPE eventType, final @NotNull Event.SEVERITY severity, @@ -396,11 +593,10 @@ private void fireEvent( } @VisibleForTesting - @NotNull CompletableFuture stopAsync(final @NotNull ProtocolAdapterWrapper wrapper, final boolean destroy) { + @NotNull CompletableFuture stopAsync(final @NotNull ProtocolAdapterWrapper wrapper) { Preconditions.checkNotNull(wrapper); log.info("Stopping protocol-adapter '{}'.", wrapper.getId()); - - return wrapper.stopAsync(destroy).whenComplete((result, throwable) -> { + return requireNonNull(wrapper.stopAsync()).whenComplete((result, throwable) -> { final Event.SEVERITY severity; final String message; final String wid = wrapper.getId(); @@ -418,101 +614,4 @@ private void fireEvent( eventService.createAdapterEvent(wid, protocolId).withSeverity(severity).withMessage(message).fire(); }); } - - private void updateAdapter(final ProtocolAdapterConfig protocolAdapterConfig) { - deleteAdapterInternal(protocolAdapterConfig.getAdapterId()); - syncFuture(startAsync(createAdapterInternal(protocolAdapterConfig, versionProvider.getVersion()))); - } - - private boolean updateAdapterTags(final @NotNull String adapterId, final @NotNull List tags) { - Preconditions.checkNotNull(adapterId); - return getProtocolAdapterWrapperByAdapterId(adapterId).map(wrapper -> { - final var protocolId = wrapper.getAdapterInformation().getProtocolId(); - final var protocolAdapterConfig = new ProtocolAdapterConfig(wrapper.getId(), - protocolId, - wrapper.getAdapterInformation().getCurrentConfigVersion(), - wrapper.getConfigObject(), - wrapper.getSouthboundMappings(), - wrapper.getNorthboundMappings(), - tags); - updateAdapter(protocolAdapterConfig); - return true; - }).orElse(false); - } - - public @NotNull Optional getProtocolAdapterWrapperByAdapterId(final @NotNull String id) { - Preconditions.checkNotNull(id); - return Optional.ofNullable(protocolAdapters.get(id)); - } - - public @NotNull Optional getAdapterTypeById(final @NotNull String typeId) { - Preconditions.checkNotNull(typeId); - final ProtocolAdapterInformation information = getAllAvailableAdapterTypes().get(typeId); - return Optional.ofNullable(information); - } - - public @NotNull Map getAllAvailableAdapterTypes() { - return protocolAdapterFactoryManager.getAllAvailableAdapterTypes(); - } - - public @NotNull Map getProtocolAdapters() { - return Map.copyOf(protocolAdapters); - } - - public boolean writingEnabled() { - return protocolAdapterWritingService.writingEnabled(); - } - - public @NotNull DomainTagAddResult addDomainTag( - final @NotNull String adapterId, - final @NotNull DomainTag domainTag) { - return getProtocolAdapterWrapperByAdapterId(adapterId).map(wrapper -> { - final var tags = new ArrayList<>(wrapper.getTags()); - final boolean alreadyExists = tags.stream().anyMatch(t -> t.getName().equals(domainTag.getTagName())); - if (!alreadyExists) { - tags.add(configConverter.domainTagToTag(wrapper.getProtocolAdapterInformation().getProtocolId(), - domainTag)); - - updateAdapterTags(adapterId, tags); - return DomainTagAddResult.success(); - } else { - return DomainTagAddResult.failed(ALREADY_EXISTS, adapterId); - } - }).orElse(DomainTagAddResult.failed(ADAPTER_MISSING, adapterId)); - } - - public @NotNull List getDomainTags() { - return protocolAdapters.values() - .stream() - .flatMap(wrapper -> wrapper.getTags() - .stream() - .map(tag -> new DomainTag(tag.getName(), - wrapper.getId(), - tag.getDescription(), - configConverter.convertTagDefinitionToJsonNode(tag.getDefinition())))) - .toList(); - } - - public @NotNull Optional getDomainTagByName(final @NotNull String tagName) { - return protocolAdapters.values() - .stream() - .flatMap(wrapper -> wrapper.getTags() - .stream() - .filter(t -> t.getName().equals(tagName)) - .map(tag -> new DomainTag(tag.getName(), - wrapper.getId(), - tag.getDescription(), - configConverter.convertTagDefinitionToJsonNode(tag.getDefinition())))) - .findFirst(); - } - - public @NotNull Optional> getTagsForAdapter(final @NotNull String adapterId) { - return getProtocolAdapterWrapperByAdapterId(adapterId).map(adapterToConfig -> adapterToConfig.getTags() - .stream() - .map(tag -> new DomainTag(tag.getName(), - adapterToConfig.getId(), - tag.getDescription(), - configConverter.convertTagDefinitionToJsonNode(tag.getDefinition()))) - .toList()); - } } diff --git a/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterStartOutputImpl.java b/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterStartOutputImpl.java index 1494f54f2a..4fb098ccfa 100644 --- a/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterStartOutputImpl.java +++ b/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterStartOutputImpl.java @@ -20,23 +20,29 @@ import org.jetbrains.annotations.Nullable; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicBoolean; public class ProtocolAdapterStartOutputImpl implements ProtocolAdapterStartOutput { - private @Nullable volatile String message = null; - private @Nullable volatile Throwable throwable = null; + private @Nullable volatile String message; + private @Nullable volatile Throwable throwable; private final @NotNull CompletableFuture startFuture = new CompletableFuture<>(); + private final @NotNull AtomicBoolean completed = new AtomicBoolean(false); @Override public void startedSuccessfully() { - this.startFuture.complete(null); + if (completed.compareAndSet(false, true)) { + startFuture.complete(null); + } } @Override public void failStart(final @NotNull Throwable t, final @Nullable String errorMessage) { - this.throwable = t; - this.message = errorMessage; - this.startFuture.completeExceptionally(t); + if (completed.compareAndSet(false, true)) { + throwable = t; + message = errorMessage; + startFuture.completeExceptionally(t); + } } public @Nullable Throwable getThrowable() { diff --git a/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterWrapper.java b/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterWrapper.java index 8df0b009bb..5f84f08d8a 100644 --- a/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterWrapper.java +++ b/hivemq-edge/src/main/java/com/hivemq/protocols/ProtocolAdapterWrapper.java @@ -33,6 +33,7 @@ import com.hivemq.edge.modules.adapters.data.TagManager; import com.hivemq.edge.modules.adapters.impl.ProtocolAdapterStateImpl; import com.hivemq.edge.modules.api.adapters.ProtocolAdapterPollingService; +import com.hivemq.fsm.ProtocolAdapterFSM; import com.hivemq.persistence.mappings.NorthboundMapping; import com.hivemq.persistence.mappings.SouthboundMapping; import com.hivemq.protocols.northbound.NorthboundConsumerFactory; @@ -48,73 +49,62 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Supplier; import java.util.stream.Collectors; -public class ProtocolAdapterWrapper { +public class ProtocolAdapterWrapper extends ProtocolAdapterFSM { private static final Logger log = LoggerFactory.getLogger(ProtocolAdapterWrapper.class); - - /** - * Represents the current operation state of the adapter to handle concurrent start/stop operations. - */ - private enum OperationState { - IDLE, - STARTING, - STOPPING - } + private static final long STOP_TIMEOUT_SECONDS = 30; + private static final @NotNull Consumer CONNECTION_STATUS_NOOP_CONSUMER = + status -> { + // Noop - adapter is stopping/stopped + }; private final @NotNull ProtocolAdapterMetricsService protocolAdapterMetricsService; private final @NotNull ProtocolAdapter adapter; private final @NotNull ProtocolAdapterFactory adapterFactory; private final @NotNull ProtocolAdapterInformation adapterInformation; private final @NotNull ProtocolAdapterStateImpl protocolAdapterState; - private final @NotNull InternalProtocolAdapterWritingService protocolAdapterWritingService; - private final @NotNull ProtocolAdapterPollingService protocolAdapterPollingService; + private final @NotNull InternalProtocolAdapterWritingService writingService; + private final @NotNull ProtocolAdapterPollingService pollingService; private final @NotNull ProtocolAdapterConfig config; private final @NotNull NorthboundConsumerFactory northboundConsumerFactory; private final @NotNull TagManager tagManager; - protected @Nullable Long lastStartAttemptTime; - private final List consumers = new ArrayList<>(); - - private final AtomicReference> startFutureRef = new AtomicReference<>(null); - private final AtomicReference> stopFutureRef = new AtomicReference<>(null); - - private final AtomicReference operationState = new AtomicReference<>(OperationState.IDLE); - - /** - * Exception thrown when attempting to start an adapter while a stop operation is in progress. - */ - public static class AdapterStartConflictException extends RuntimeException { - public AdapterStartConflictException(final String adapterId) { - super("Cannot start adapter '" + adapterId + "' while stop operation is in progress"); - } - } - - /** - * Exception thrown when attempting to stop an adapter while a start operation is in progress. - */ - public static class AdapterStopConflictException extends RuntimeException { - public AdapterStopConflictException(final String adapterId) { - super("Cannot stop adapter '" + adapterId + "' while start operation is in progress"); - } - } + private final @NotNull List consumers; + private final @NotNull ReentrantLock operationLock; + private final @NotNull Object adapterLock; // protects underlying adapter start/stop calls + private final @NotNull ExecutorService sharedAdapterExecutor; + protected volatile @Nullable Long lastStartAttemptTime; + private @Nullable CompletableFuture currentStartFuture; + private @Nullable CompletableFuture currentStopFuture; + private @Nullable Consumer connectionStatusListener; + private volatile @Nullable ModuleServices lastModuleServices; // Stored for hot-reload operations + private volatile boolean startOperationInProgress; + private volatile boolean stopOperationInProgress; public ProtocolAdapterWrapper( final @NotNull ProtocolAdapterMetricsService protocolAdapterMetricsService, - final @NotNull InternalProtocolAdapterWritingService protocolAdapterWritingService, - final @NotNull ProtocolAdapterPollingService protocolAdapterPollingService, + final @NotNull InternalProtocolAdapterWritingService writingService, + final @NotNull ProtocolAdapterPollingService pollingService, final @NotNull ProtocolAdapterConfig config, final @NotNull ProtocolAdapter adapter, final @NotNull ProtocolAdapterFactory adapterFactory, final @NotNull ProtocolAdapterInformation adapterInformation, final @NotNull ProtocolAdapterStateImpl protocolAdapterState, final @NotNull NorthboundConsumerFactory northboundConsumerFactory, - final @NotNull TagManager tagManager) { - this.protocolAdapterWritingService = protocolAdapterWritingService; - this.protocolAdapterPollingService = protocolAdapterPollingService; + final @NotNull TagManager tagManager, + final @NotNull ExecutorService sharedAdapterExecutor) { + super(config.getAdapterId()); + this.writingService = writingService; + this.pollingService = pollingService; this.protocolAdapterMetricsService = protocolAdapterMetricsService; this.adapter = adapter; this.adapterFactory = adapterFactory; @@ -123,195 +113,197 @@ public ProtocolAdapterWrapper( this.config = config; this.northboundConsumerFactory = northboundConsumerFactory; this.tagManager = tagManager; + this.consumers = new CopyOnWriteArrayList<>(); + this.operationLock = new ReentrantLock(); + this.sharedAdapterExecutor = sharedAdapterExecutor; + this.adapterLock = new Object(); + + if (log.isDebugEnabled()) { + registerStateTransitionListener(state -> log.debug( + "Adapter {} FSM adapter transition: adapter={}, northbound={}, southbound={}", + adapter.getId(), + state.adapter(), + state.northbound(), + state.southbound())); + } } - public @NotNull CompletableFuture startAsync( - final boolean writingEnabled, - final @NotNull ModuleServices moduleServices) { - final var existingStartFuture = getOngoingOperationIfPresent(operationState.get(), OperationState.STARTING); - if (existingStartFuture != null) return existingStartFuture; - // Try to atomically transition from IDLE to STARTING - if (!operationState.compareAndSet(OperationState.IDLE, OperationState.STARTING)) { - // State changed between check and set, retry - return startAsync(writingEnabled, moduleServices); + @Override + public boolean onStarting() { + try { + protocolAdapterState.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STARTED); + return true; + } catch (final Exception e) { + log.error("Adapter starting failed for adapter {}", adapter.getId(), e); + return false; } - initStartAttempt(); - final var output = new ProtocolAdapterStartOutputImpl(); - final var input = new ProtocolAdapterStartInputImpl(moduleServices); - final var startFuture = CompletableFuture - .supplyAsync(() -> { - try { - adapter.start(input, output); - } catch (final Throwable throwable) { - output.getStartFuture().completeExceptionally(throwable); - } - return output.getStartFuture(); - }) - .thenCompose(Function.identity()) - .handle((ignored, error) -> { - if(error != null) { - log.error("Error starting adapter", error); - stopAfterFailedStart(); - protocolAdapterState.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STOPPED); - //we still return the initial error since that's the most significant information - return CompletableFuture.failedFuture(error); - } else { - return attemptStartingConsumers(writingEnabled, moduleServices.eventService()) - .map(startException -> { - log.error("Failed to start adapter with id {}", adapter.getId(), startException); - stopAfterFailedStart(); - protocolAdapterState.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STOPPED); - //we still return the initial error since that's the most significant information - return CompletableFuture.failedFuture(startException); - }) - .orElseGet(() -> { - protocolAdapterState.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STARTED); - return CompletableFuture.completedFuture(null); - }); - } - }) - .thenApply(ignored -> (Void)null) - .whenComplete((result, throwable) -> { - //always clean up state - startFutureRef.set(null); - operationState.set(OperationState.IDLE); - }); + } - startFutureRef.set(startFuture); - return startFuture; + @Override + public void onStopping() { + protocolAdapterState.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STOPPED); } - private void stopAfterFailedStart() { - log.warn("Stopping adapter with id {} after a failed start", adapter.getId()); - final var stopInput = new ProtocolAdapterStopInputImpl(); - final var stopOutput = new ProtocolAdapterStopOutputImpl(); - stopPolling(protocolAdapterPollingService); - stopWriting(protocolAdapterWritingService); - try { - adapter.stop(stopInput, stopOutput); - } catch (final Throwable throwable) { - log.error("Stopping adapter after a start error failed", throwable); + @Override + public boolean startSouthbound() { + if (!isWriting()) { + transitionSouthboundState(StateEnum.NOT_SUPPORTED); + return true; } - //force waiting for the stop future to complete, we are in a separate thread so no harm caused - try { - stopOutput.getOutputFuture().get(); - } catch (final Throwable throwable) { - log.error("Stopping adapter after a start error failed", throwable); + log.debug("Start writing for protocol adapter with id '{}'", getId()); + final boolean started = writingService.startWriting((WritingProtocolAdapter) adapter, + protocolAdapterMetricsService, + config.getSouthboundMappings() + .stream() + .map(InternalWritingContextImpl::new) + .collect(Collectors.toList())); + if (started) { + // Allow time for writing capabilities to initialize after hot-reload + // This prevents data loss by ensuring ReactiveWriters complete MQTT subscription + // and are ready to process messages before hot-reload completes. + // TODO: Replace with proper event-based initialization by: + // 1. Making ReactiveWriter.start() return CompletableFuture + // 2. Having ProtocolAdapterWritingServiceImpl.startWriting() wait on all futures + // 3. Removing this sleep in favor of the future-based approach + try { + Thread.sleep(1000); + log.info("Southbound started for adapter {}", adapter.getId()); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + log.warn("Interrupted while waiting for southbound initialization after hot-reload for adapter '{}'", + getId()); + } + transitionSouthboundState(StateEnum.CONNECTED); + } else { + log.error("Southbound start failed for adapter {}", adapter.getId()); + transitionSouthboundState(StateEnum.ERROR); } + return started; } - private Optional attemptStartingConsumers(final boolean writingEnabled, final @NotNull EventService eventService) { + public @Nullable CompletableFuture startAsync(final @NotNull ModuleServices moduleServices) { + operationLock.lock(); try { - //Adapter started successfully, now start the consumers - createAndSubscribeTagConsumer(); - startPolling(protocolAdapterPollingService, eventService); - - if(writingEnabled && isWriting()) { - final var started = new AtomicBoolean(false); - protocolAdapterState.setConnectionStatusListener(status -> { - if(status == ProtocolAdapterState.ConnectionStatus.CONNECTED) { - if(started.compareAndSet(false, true)) { - if(startWriting(protocolAdapterWritingService)) { - log.info("Successfully started adapter with id {}", adapter.getId()); - } else { - log.error("Protocol adapter start failed as data hub is not available."); - } + if (startOperationInProgress) { + log.info("Start operation already in progress for adapter '{}'", getId()); + return currentStartFuture; + } + if (adapterState.get() == AdapterStateEnum.STARTED) { + log.info("Adapter '{}' is already started, returning success", getId()); + return CompletableFuture.completedFuture(null); + } + if (stopOperationInProgress) { + log.warn("Stop operation in progress for adapter '{}', waiting for it to complete before starting", + getId()); + final CompletableFuture stopFuture = currentStopFuture; + if (stopFuture != null) { + // Wait for stop to complete, then retry start + return stopFuture.handle((result, throwable) -> { + if (throwable != null) { + log.warn("Stop operation failed for adapter '{}', but proceeding with start", + getId(), + throwable); } - } - }); + return null; + }).thenCompose(v -> startAsync(moduleServices)); + } + log.error("Stop operation in progress but currentStopFuture is null for adapter '{}'", getId()); + return CompletableFuture.failedFuture(new IllegalStateException("Cannot start adapter '" + + adapter.getId() + + "' while stop operation is in progress")); } - } catch (final Throwable e) { - log.error("Protocol adapter start failed"); - return Optional.of(e); - } - return Optional.empty(); - } - public @NotNull CompletableFuture stopAsync(final boolean destroy) { - final var existingStopFuture = getOngoingOperationIfPresent(operationState.get(), OperationState.STOPPING); - if (existingStopFuture != null) return existingStopFuture; - - // Try to atomically transition from IDLE to STOPPING - if (!operationState.compareAndSet(OperationState.IDLE, OperationState.STOPPING)) { - // State changed between check and set, retry - return stopAsync(destroy); - } - // Double-check that no stop future exists - final var existingFuture = stopFutureRef.get(); - if (existingFuture != null) { - log.info("Stop operation already in progress for adapter with id '{}', returning existing future", getId()); - return existingFuture; + startOperationInProgress = true; + lastStartAttemptTime = System.currentTimeMillis(); + lastModuleServices = moduleServices; // Store for hot-reload operations + currentStartFuture = + CompletableFuture.supplyAsync(startProtocolAdapter(moduleServices), sharedAdapterExecutor) + .thenCompose(Function.identity()) + .thenRun(() -> startConsumers(moduleServices.eventService()).ifPresent(startException -> { + log.error("Failed to start adapter with id {}", adapter.getId(), startException); + stopProtocolAdapterOnFailedStart(); + throw new RuntimeException("Failed to start consumers", startException); + })) + .whenComplete((result, throwable) -> { + if (throwable != null) { + log.error("Error starting adapter", throwable); + stopProtocolAdapterOnFailedStart(); + } + operationLock.lock(); + try { + startOperationInProgress = false; + currentStartFuture = null; + } finally { + operationLock.unlock(); + } + }); + return currentStartFuture; + } finally { + operationLock.unlock(); } + } - consumers.forEach(tagManager::removeConsumer); - final var input = new ProtocolAdapterStopInputImpl(); - final var output = new ProtocolAdapterStopOutputImpl(); - - final var stopFuture = CompletableFuture - .supplyAsync(() -> { - stopPolling(protocolAdapterPollingService); - stopWriting(protocolAdapterWritingService); - try { - adapter.stop(input, output); - } catch (final Throwable throwable) { - output.getOutputFuture().completeExceptionally(throwable); - } - return output.getOutputFuture(); - }) - .thenCompose(Function.identity()) - .whenComplete((result, throwable) -> { - if (destroy) { - log.info("Destroying adapter with id '{}'", getId()); - adapter.destroy(); - } - if (throwable == null) { - log.info("Stopped adapter with id {}", adapter.getId()); - } else { - log.error("Error stopping adapter with id {}", adapter.getId(), throwable); - } - protocolAdapterState.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STOPPED); - stopFutureRef.set(null); - operationState.set(OperationState.IDLE); - }); - - stopFutureRef.set(stopFuture); - - return stopFuture; - } - - private @Nullable CompletableFuture getOngoingOperationIfPresent(final @NotNull OperationState currentState, final @NotNull OperationState targetState) { - switch (currentState) { - case STARTING: - if(targetState == OperationState.STARTING) { - // If already starting, return existing future - final var existingStartFuture = startFutureRef.get(); - if (existingStartFuture != null) { - log.info("Start operation already in progress for adapter with id '{}', returning existing future", getId()); - return existingStartFuture; - } - } else { - log.warn("Cannot stop adapter with id '{}' while start operation is in progress", getId()); - return CompletableFuture.failedFuture(new AdapterStopConflictException(getId())); - } - break; - case STOPPING: - if(targetState == OperationState.STOPPING) { - // If already stopping, return existing future - final var existingStopFuture = stopFutureRef.get(); - if (existingStopFuture != null) { - log.info("Stop operation already in progress for adapter with id '{}', returning existing future", getId()); - return existingStopFuture; - } - break; + public @Nullable CompletableFuture stopAsync() { + operationLock.lock(); + try { + if (stopOperationInProgress) { + log.info("Stop operation already in progress for adapter '{}'", getId()); + return currentStopFuture; + } + final var currentState = currentState(); + if (currentState.adapter() == AdapterStateEnum.STOPPED) { + log.info("Adapter '{}' is already stopped, returning success", getId()); + return CompletableFuture.completedFuture(null); + } + if (startOperationInProgress) { + log.warn("Start operation in progress for adapter '{}', waiting for it to complete before stopping", + getId()); + final CompletableFuture startFuture = currentStartFuture; + if (startFuture != null) { + // Wait for start to complete, then retry stop + return startFuture.handle((result, throwable) -> { + if (throwable != null) { + log.warn("Start operation failed for adapter '{}', but proceeding with stop", + getId(), + throwable); + } + return null; + }).thenCompose(v -> stopAsync()); } - // If stopping, return failed future immediately - log.warn("Cannot start adapter with id '{}' while stop operation is in progress", getId()); - return CompletableFuture.failedFuture(new AdapterStartConflictException(getId())); - case IDLE: - // Proceed with start operation - break; + log.error("Start operation in progress but currentStartFuture is null for adapter '{}'", getId()); + return CompletableFuture.failedFuture(new IllegalStateException("Cannot stop adapter '" + + adapter.getId() + + "' while start operation is in progress")); + } + + stopOperationInProgress = true; + log.debug("Adapter '{}': Creating stop operation future", getId()); + currentStopFuture = CompletableFuture.supplyAsync(this::stopProtocolAdapter, sharedAdapterExecutor) + .thenCompose(Function.identity()) + .whenComplete((result, throwable) -> { + log.debug("Adapter '{}': Stop operation completed, starting cleanup", getId()); + try { + adapter.destroy(); + } catch (final Exception destroyException) { + log.error("Error destroying adapter with id {}", adapter.getId(), destroyException); + } + if (throwable == null) { + log.info("Successfully stopped adapter with id '{}' successfully", adapter.getId()); + } else { + log.error("Error stopping adapter with id {}", adapter.getId(), throwable); + } + operationLock.lock(); + try { + stopOperationInProgress = false; + currentStopFuture = null; + } finally { + operationLock.unlock(); + } + }); + return currentStopFuture; + } finally { + operationLock.unlock(); } - return null; } public @NotNull ProtocolAdapterInformation getProtocolAdapterInformation() { @@ -319,7 +311,8 @@ private Optional attemptStartingConsumers(final boolean writingEnable } public void discoverValues( - final @NotNull ProtocolAdapterDiscoveryInput input, final @NotNull ProtocolAdapterDiscoveryOutput output) { + final @NotNull ProtocolAdapterDiscoveryInput input, + final @NotNull ProtocolAdapterDiscoveryOutput output) { adapter.discoverValues(input, output); } @@ -331,12 +324,12 @@ public void discoverValues( return protocolAdapterState.getRuntimeStatus(); } - public @Nullable String getErrorMessage() { - return protocolAdapterState.getLastErrorMessage(); + public void setRuntimeStatus(final @NotNull ProtocolAdapterState.RuntimeStatus runtimeStatus) { + protocolAdapterState.setRuntimeStatus(runtimeStatus); } - protected void initStartAttempt() { - lastStartAttemptTime = System.currentTimeMillis(); + public @Nullable String getErrorMessage() { + return protocolAdapterState.getLastErrorMessage(); } public @NotNull ProtocolAdapterFactory getAdapterFactory() { @@ -387,10 +380,6 @@ public void setErrorConnectionStatus(final @NotNull Throwable exception, final @ protocolAdapterState.setErrorConnectionStatus(exception, errorMessage); } - public void setRuntimeStatus(final @NotNull ProtocolAdapterState.RuntimeStatus runtimeStatus) { - protocolAdapterState.setRuntimeStatus(runtimeStatus); - } - public boolean isWriting() { return adapter instanceof WritingProtocolAdapter; } @@ -403,79 +392,371 @@ public boolean isBatchPolling() { return adapter instanceof BatchPollingProtocolAdapter; } - private void startPolling( - final @NotNull ProtocolAdapterPollingService protocolAdapterPollingService, + public void addTagHotReload(final @NotNull Tag tag, final @NotNull EventService eventService) { + // Wait for any in-progress operations before proceeding + waitForOperationsToComplete(); + + operationLock.lock(); + try { + if (startOperationInProgress || stopOperationInProgress) { + throw new IllegalStateException("Cannot hot-reload tag for adapter '" + + getId() + + "': operation started during wait"); + } + + // Update config with new tag regardless of adapter state + final List updatedTags = new ArrayList<>(config.getTags()); + updatedTags.add(tag); + config.setTags(updatedTags); + if (adapterState.get() != AdapterStateEnum.STARTED) { + log.debug("Adapter '{}' not started yet, only updating config for tag '{}'", getId(), tag.getName()); + return; + } + if (isPolling()) { + log.debug("Starting polling for new tag '{}' on adapter '{}'", tag.getName(), getId()); + schedulePollingForTag(tag, eventService); + } + log.info("Successfully added tag '{}' to adapter '{}' via hot-reload", tag.getName(), getId()); + } finally { + operationLock.unlock(); + } + } + + public void updateMappingsHotReload( + final @Nullable List northboundMappings, + final @Nullable List southboundMappings, final @NotNull EventService eventService) { + waitForOperationsToComplete(); - if (isBatchPolling()) { - log.debug("Schedule batch polling for protocol adapter with id '{}'", getId()); - final PerAdapterSampler sampler = - new PerAdapterSampler(this, eventService, tagManager); - protocolAdapterPollingService.schedulePolling(sampler); + operationLock.lock(); + try { + if (startOperationInProgress || stopOperationInProgress) { + throw new IllegalStateException("Cannot hot-reload mappings for adapter '" + + getId() + + "': operation started during wait"); + } + + if (northboundMappings != null) { + config.setNorthboundMappings(northboundMappings); + } + if (southboundMappings != null) { + config.setSouthboundMappings(southboundMappings); + } + if (adapterState.get() != AdapterStateEnum.STARTED) { + log.debug("Adapter '{}' not started yet, only updating config for mappings", getId()); + return; + } + + // Stop existing consumers and polling + if (northboundMappings != null) { + log.debug("Stopping existing consumers and polling for adapter '{}'", getId()); + consumers.forEach(tagManager::removeConsumer); + consumers.clear(); + + // Stop polling to restart with new mappings + stopPolling(); + + log.debug("Updating northbound mappings for adapter '{}'", getId()); + northboundMappings.stream() + .map(mapping -> northboundConsumerFactory.build(this, mapping, protocolAdapterMetricsService)) + .forEach(consumer -> { + tagManager.addConsumer(consumer); + consumers.add(consumer); + }); + + // Restart data flow with new consumers + // For polling adapters: schedule polling + // For subscription-based adapters: reconnect to restart subscriptions + final StateEnum currentNorthboundState = currentState().northbound(); + final boolean wasConnected = (currentNorthboundState == StateEnum.CONNECTED); + if (isPolling() || isBatchPolling()) { + log.debug("Restarting polling for adapter '{}'", getId()); + schedulePollingForAllTags(eventService); + } else if (wasConnected) { + log.debug("Reconnecting subscription-based adapter '{}' after hot-reload", getId()); + stopAdapterConnection(); + startAdapterConnection(); + } + } + + if (southboundMappings != null && isWriting()) { + log.debug("Updating southbound mappings for adapter '{}'", getId()); + final StateEnum currentSouthboundState = currentState().southbound(); + final boolean wasConnected = (currentSouthboundState == StateEnum.CONNECTED); + if (wasConnected) { + log.debug("Stopping southbound for adapter '{}' before hot-reload", getId()); + stopWriting(); + log.debug("Restarting southbound for adapter '{}' after hot-reload", getId()); + startSouthbound(); + } + } + log.info("Successfully updated mappings for adapter '{}' via hot-reload", getId()); + } finally { + operationLock.unlock(); + } + } + + private void waitForOperationsToComplete() { + CompletableFuture futureToWait = null; + operationLock.lock(); + try { + if (startOperationInProgress) { + log.debug("Adapter '{}': Waiting for start operation to complete before hot-reload", getId()); + futureToWait = currentStartFuture; + } else if (stopOperationInProgress) { + log.debug("Adapter '{}': Waiting for stop operation to complete before hot-reload", getId()); + futureToWait = currentStopFuture; + } + } finally { + operationLock.unlock(); } - if (isPolling()) { - config.getTags().forEach(tag -> { - final PerContextSampler sampler = - new PerContextSampler( - this, - new PollingContextWrapper( - "unused", - tag.getName(), - MessageHandlingOptions.MQTTMessagePerTag, - false, - false, - List.of(), - 1, - -1), - eventService, - tagManager); - protocolAdapterPollingService.schedulePolling(sampler); - }); + if (futureToWait != null) { + try { + // Wait with a timeout to prevent indefinite blocking + futureToWait.get(30, TimeUnit.SECONDS); + log.debug("Adapter '{}': Operation completed, proceeding with hot-reload", getId()); + } catch (final TimeoutException e) { + log.warn("Adapter '{}': Operation did not complete within 30 seconds, proceeding with hot-reload anyway", + getId()); + } catch (final Exception e) { + log.warn("Adapter '{}': Operation completed with error, but proceeding with hot-reload", getId(), e); + } + } + } + + private void cleanupConnectionStatusListener() { + final Consumer listenerToClean = connectionStatusListener; + if (listenerToClean != null) { + connectionStatusListener = null; + protocolAdapterState.setConnectionStatusListener(CONNECTION_STATUS_NOOP_CONSUMER); + } + } + + private @NotNull Optional startConsumers(final @NotNull EventService eventService) { + try { + // create/subscribe tag consumer + config.getNorthboundMappings() + .stream() + .map(mapping -> northboundConsumerFactory.build(this, mapping, protocolAdapterMetricsService)) + .forEach(northboundTagConsumer -> { + tagManager.addConsumer(northboundTagConsumer); + consumers.add(northboundTagConsumer); + }); + + // start polling + schedulePollingForAllTags(eventService); + + // FSM's accept() method handles: + // 1. Transitioning northbound adapter + // 2. Triggering startSouthbound() when CONNECTED (only for writing adapters) + // For non-writing adapters that are only polling, southbound is not applicable + // but we still need to track northbound connection status + connectionStatusListener = this; + protocolAdapterState.setConnectionStatusListener(connectionStatusListener); + return Optional.empty(); + } catch (final Throwable e) { + log.error("Protocol adapter start failed", e); + return Optional.of(e); } } - private void stopPolling( - final @NotNull ProtocolAdapterPollingService protocolAdapterPollingService) { + private void stopPolling() { if (isPolling() || isBatchPolling()) { log.debug("Stopping polling for protocol adapter with id '{}'", getId()); - protocolAdapterPollingService.stopPollingForAdapterInstance(getAdapter()); + try { + pollingService.stopPollingForAdapterInstance(adapter); + log.debug("Polling stopped successfully for adapter '{}'", getId()); + } catch (final Exception e) { + log.error("Error stopping polling for adapter '{}'", getId(), e); + } } } - private @NotNull boolean startWriting(final @NotNull InternalProtocolAdapterWritingService protocolAdapterWritingService) { - log.debug("Start writing for protocol adapter with id '{}'", getId()); + private void stopWriting() { + if (isWriting()) { + log.debug("Stopping writing for protocol adapter with id '{}'", getId()); + try { + // Transition southbound state to indicate shutdown in progress + final StateEnum currentSouthboundState = currentState().southbound(); + if (currentSouthboundState == StateEnum.CONNECTED) { + transitionSouthboundState(StateEnum.DISCONNECTING); + } + writingService.stopWriting((WritingProtocolAdapter) adapter, + config.getSouthboundMappings() + .stream() + .map(mapping -> (InternalWritingContext) new InternalWritingContextImpl(mapping)) + .toList()); + if (currentSouthboundState == StateEnum.CONNECTED || + currentSouthboundState == StateEnum.DISCONNECTING) { + transitionSouthboundState(StateEnum.DISCONNECTED); + } + log.debug("Writing stopped successfully for adapter '{}'", getId()); + } catch (final IllegalStateException stateException) { + // State transition failed, log but continue cleanup + log.warn("State transition failed while stopping writing for adapter '{}': {}", + getId(), + stateException.getMessage()); + } catch (final Exception e) { + log.error("Error stopping writing for adapter '{}'", getId(), e); + } + } + } - final var southboundMappings = getSouthboundMappings(); - final var writingContexts = southboundMappings.stream() - .map(InternalWritingContextImpl::new) - .collect(Collectors.toList()); + private void stopAdapterConnection() { + log.debug("Stopping adapter connection for adapter '{}' during hot-reload", getId()); + try { + // Transition northbound state to indicate shutdown in progress + final StateEnum currentNorthboundState = currentState().northbound(); + if (currentNorthboundState == StateEnum.CONNECTED) { + transitionNorthboundState(StateEnum.DISCONNECTING); + } - return protocolAdapterWritingService - .startWriting( - (WritingProtocolAdapter) getAdapter(), - getProtocolAdapterMetricsService(), - writingContexts); + synchronized (adapterLock) { + adapter.stop(new ProtocolAdapterStopInputImpl(), new ProtocolAdapterStopOutputImpl()); + } + + if (currentNorthboundState == StateEnum.CONNECTED || currentNorthboundState == StateEnum.DISCONNECTING) { + transitionNorthboundState(StateEnum.DISCONNECTED); + } + log.debug("Adapter connection stopped successfully for adapter '{}'", getId()); + } catch (final IllegalStateException stateException) { + // State transition failed, log but continue + log.warn("State transition failed while stopping adapter connection for '{}': {}", + getId(), + stateException.getMessage()); + } catch (final Exception e) { + log.error("Error stopping adapter connection for '{}'", getId(), e); + } } - private void stopWriting(final @NotNull InternalProtocolAdapterWritingService protocolAdapterWritingService) { - //no check for 'writing is enabled', as we have to stop it anyway since the license could have been removed in the meantime. - if (isWriting()) { - log.debug("Stopping writing for protocol adapter with id '{}'", getId()); - final var writingContexts = - getSouthboundMappings().stream() - .map(mapping -> (InternalWritingContext)new InternalWritingContextImpl(mapping)) - .toList(); - protocolAdapterWritingService.stopWriting((WritingProtocolAdapter) getAdapter(), writingContexts); + private void startAdapterConnection() { + log.debug("Starting adapter connection for adapter '{}' during hot-reload", getId()); + try { + transitionNorthboundState(StateEnum.CONNECTING); + + final ProtocolAdapterStartOutputImpl output = new ProtocolAdapterStartOutputImpl(); + synchronized (adapterLock) { + adapter.start(new ProtocolAdapterStartInputImpl(lastModuleServices), output); + } + + // Wait for the start to complete (with timeout) + output.getStartFuture().get(30, TimeUnit.SECONDS); + + // Connection status will transition to CONNECTED via the connection status listener + log.info("Adapter connection restarted successfully for adapter '{}'", getId()); + } catch (final TimeoutException e) { + log.error("Timeout while restarting adapter connection for '{}'", getId(), e); + transitionNorthboundState(StateEnum.ERROR); + } catch (final Exception e) { + log.error("Error restarting adapter connection for '{}'", getId(), e); + transitionNorthboundState(StateEnum.ERROR); + } + } + + private @NotNull Supplier<@NotNull CompletableFuture> startProtocolAdapter( + final @NotNull ModuleServices moduleServices) { + return () -> { + startAdapter(); // start FSM + final ProtocolAdapterStartOutputImpl output = new ProtocolAdapterStartOutputImpl(); + synchronized (adapterLock) { + log.debug("Adapter '{}': Calling adapter.start() in thread '{}'", + getId(), + Thread.currentThread().getName()); + try { + adapter.start(new ProtocolAdapterStartInputImpl(moduleServices), output); + } catch (final Throwable t) { + output.getStartFuture().completeExceptionally(t); + } + } + return output.getStartFuture(); + }; + } + + private @NotNull CompletableFuture stopProtocolAdapter() { + log.debug("Adapter '{}': Stop operation executing in thread '{}'", + adapter.getId(), + Thread.currentThread().getName()); + return performStopOperations(); + } + + private void stopProtocolAdapterOnFailedStart() { + log.warn("Stopping adapter with id {} after a failed start", adapter.getId()); + final CompletableFuture stopFuture = performStopOperations(); + + // Wait synchronously for stop to complete with timeout + try { + stopFuture.get(STOP_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } catch (final TimeoutException e) { + log.error("Timeout waiting for adapter {} to stop after failed start", adapter.getId()); + } catch (final Throwable throwable) { + log.error("Stopping adapter after a start error failed", throwable); + } + + // Always destroy adapter after failed start + try { + adapter.destroy(); + } catch (final Exception destroyException) { + log.error("Error destroying adapter with id {} after failed start", adapter.getId(), destroyException); } } - private void createAndSubscribeTagConsumer() { - getNorthboundMappings().stream() - .map(northboundMapping -> northboundConsumerFactory.build(this, northboundMapping, protocolAdapterMetricsService)) - .forEach(northboundTagConsumer -> { - tagManager.addConsumer(northboundTagConsumer); - consumers.add(northboundTagConsumer); - }); + private @NotNull CompletableFuture performStopOperations() { + stopAdapter(); + cleanupConnectionStatusListener(); + // Clean up consumers + try { + consumers.forEach(tagManager::removeConsumer); + consumers.clear(); + log.debug("Adapter '{}': Consumers cleaned up successfully", getId()); + } catch (final Exception e) { + log.error("Adapter '{}': Error cleaning up consumers", getId(), e); + } + + stopPolling(); + stopWriting(); + + // Initiate adapter stop + final var output = new ProtocolAdapterStopOutputImpl(); + synchronized (adapterLock) { + log.debug("Adapter '{}': Calling adapter.stop() in thread '{}'", getId(), Thread.currentThread().getName()); + try { + adapter.stop(new ProtocolAdapterStopInputImpl(), output); + } catch (final Throwable throwable) { + log.error("Adapter '{}': Exception during adapter.stop()", adapter.getId(), throwable); + output.getOutputFuture().completeExceptionally(throwable); + } + } + + log.debug("Adapter '{}': Waiting for stop output future", adapter.getId()); + return output.getOutputFuture(); + } + + private @NotNull PollingContextWrapper createPollingContextForTag(final @NotNull Tag tag) { + return new PollingContextWrapper("unused", + tag.getName(), + MessageHandlingOptions.MQTTMessagePerTag, + false, + false, + List.of(), + 1, + -1); + } + + private void schedulePollingForTag(final @NotNull Tag tag, final @NotNull EventService eventService) { + pollingService.schedulePolling(new PerContextSampler(this, + createPollingContextForTag(tag), + eventService, + tagManager)); + } + + private void schedulePollingForAllTags(final @NotNull EventService eventService) { + if (isBatchPolling()) { + log.debug("Schedule batch polling for protocol adapter with id '{}'", getId()); + pollingService.schedulePolling(new PerAdapterSampler(this, eventService, tagManager)); + } + if (isPolling()) { + config.getTags().forEach(tag -> schedulePollingForTag(tag, eventService)); + } } } diff --git a/hivemq-edge/src/test/java/com/hivemq/fsm/ProtocolAdapterFSMTest.java b/hivemq-edge/src/test/java/com/hivemq/fsm/ProtocolAdapterFSMTest.java index 2f20a4b97a..ad99c0767b 100644 --- a/hivemq-edge/src/test/java/com/hivemq/fsm/ProtocolAdapterFSMTest.java +++ b/hivemq-edge/src/test/java/com/hivemq/fsm/ProtocolAdapterFSMTest.java @@ -437,7 +437,7 @@ void stateListener_notifiedOnTransition() { fsm.startAdapter(); assertThat(capturedState.get()).isNotNull(); - assertThat(capturedState.get().state()).isEqualTo(AdapterStateEnum.STARTED); + assertThat(capturedState.get().adapter()).isEqualTo(AdapterStateEnum.STARTED); } @Test diff --git a/hivemq-edge/src/test/java/com/hivemq/protocols/ProtocolAdapterManagerTest.java b/hivemq-edge/src/test/java/com/hivemq/protocols/ProtocolAdapterManagerTest.java index bf723dd685..8b905e4f42 100644 --- a/hivemq-edge/src/test/java/com/hivemq/protocols/ProtocolAdapterManagerTest.java +++ b/hivemq-edge/src/test/java/com/hivemq/protocols/ProtocolAdapterManagerTest.java @@ -32,6 +32,7 @@ import com.hivemq.adapter.sdk.api.writing.WritingOutput; import com.hivemq.adapter.sdk.api.writing.WritingPayload; import com.hivemq.adapter.sdk.api.writing.WritingProtocolAdapter; +import com.hivemq.common.shutdown.ShutdownHooks; import com.hivemq.configuration.reader.ProtocolAdapterExtractor; import com.hivemq.edge.HiveMQEdgeRemoteService; import com.hivemq.edge.VersionProvider; @@ -43,10 +44,16 @@ import com.hivemq.protocols.northbound.NorthboundConsumerFactory; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -72,15 +79,19 @@ class ProtocolAdapterManagerTest { private final @NotNull NorthboundConsumerFactory northboundConsumerFactory = mock(); private final @NotNull TagManager tagManager = mock(); private final @NotNull ProtocolAdapterExtractor protocolAdapterExtractor = mock(); + private final @NotNull ShutdownHooks shutdownHooks = mock(); private final @NotNull ProtocolAdapterConfigConverter protocolAdapterConfigConverter = mock(); private @NotNull ProtocolAdapterManager protocolAdapterManager; + private @NotNull ExecutorService testExecutor; + private @NotNull ScheduledExecutorService testScheduledExecutor; @BeforeEach void setUp() { - protocolAdapterManager = new ProtocolAdapterManager( - metricRegistry, + testExecutor = Executors.newCachedThreadPool(); + testScheduledExecutor = Executors.newScheduledThreadPool(2); + protocolAdapterManager = new ProtocolAdapterManager(metricRegistry, moduleServices, remoteService, eventService, @@ -92,7 +103,21 @@ void setUp() { protocolAdapterFactoryManager, northboundConsumerFactory, tagManager, - protocolAdapterExtractor); + protocolAdapterExtractor, + testExecutor, + shutdownHooks); + } + + @AfterEach + void tearDown() throws InterruptedException { + if (testExecutor != null && !testExecutor.isShutdown()) { + testExecutor.shutdown(); + testExecutor.awaitTermination(5, TimeUnit.SECONDS); + } + if (testScheduledExecutor != null && !testScheduledExecutor.isShutdown()) { + testScheduledExecutor.shutdown(); + testScheduledExecutor.awaitTermination(5, TimeUnit.SECONDS); + } } @Test @@ -100,9 +125,7 @@ void test_startWritingAdapterSucceeded_eventsFired() throws Exception { final EventBuilder eventBuilder = new EventBuilderImpl(mock()); when(protocolAdapterWritingService.writingEnabled()).thenReturn(true); - when(protocolAdapterWritingService.startWriting(any(), - any(), - any())).thenReturn(true); + when(protocolAdapterWritingService.startWriting(any(), any(), any())).thenReturn(true); when(eventService.createAdapterEvent(anyString(), anyString())).thenReturn(eventBuilder); final var adapterState = new ProtocolAdapterStateImpl(eventService, "test-adapter", "test-protocol"); final ProtocolAdapterWrapper adapterWrapper = new ProtocolAdapterWrapper(mock(), @@ -114,7 +137,8 @@ void test_startWritingAdapterSucceeded_eventsFired() throws Exception { mock(), adapterState, northboundConsumerFactory, - tagManager); + tagManager, + testExecutor); protocolAdapterManager.startAsync(adapterWrapper).get(); @@ -139,7 +163,8 @@ void test_startWritingNotEnabled_writingNotStarted() throws Exception { mock(), adapterState, northboundConsumerFactory, - tagManager); + tagManager, + testExecutor); protocolAdapterManager.startAsync(adapterWrapper).get(); @@ -149,13 +174,11 @@ void test_startWritingNotEnabled_writingNotStarted() throws Exception { } @Test - void test_startWriting_adapterFailedStart_resourcesCleanedUp() throws Exception{ + void test_startWriting_adapterFailedStart_resourcesCleanedUp() throws Exception { final EventBuilder eventBuilder = new EventBuilderImpl(mock()); when(protocolAdapterWritingService.writingEnabled()).thenReturn(true); - when(protocolAdapterWritingService - .startWriting(any(), any(), any())) - .thenReturn(true); + when(protocolAdapterWritingService.startWriting(any(), any(), any())).thenReturn(true); when(eventService.createAdapterEvent(anyString(), anyString())).thenReturn(eventBuilder); final var adapterState = new ProtocolAdapterStateImpl(eventService, "test-adapter", "test-protocol"); @@ -169,10 +192,14 @@ void test_startWriting_adapterFailedStart_resourcesCleanedUp() throws Exception{ mock(), adapterState, northboundConsumerFactory, - tagManager); + tagManager, + testExecutor); - protocolAdapterManager.startAsync(adapterWrapper).get(); + // Start will fail, but we expect cleanup to happen + assertThatThrownBy(() -> protocolAdapterManager.startAsync(adapterWrapper).get()).isInstanceOf( + ExecutionException.class).hasCauseInstanceOf(RuntimeException.class).hasMessageContaining("failed"); + // Even though start failed, cleanup should have occurred assertThat(adapterWrapper.getRuntimeStatus()).isEqualTo(ProtocolAdapterState.RuntimeStatus.STOPPED); verify(remoteService).fireUsageEvent(any()); verify(protocolAdapterWritingService).stopWriting(eq((WritingProtocolAdapter) adapterWrapper.getAdapter()), @@ -186,9 +213,7 @@ void test_startWriting_eventServiceFailedStart_resourcesCleanedUp() throws Excep when(eventService.createAdapterEvent(anyString(), anyString())).thenReturn(eventBuilder); when(protocolAdapterWritingService.writingEnabled()).thenReturn(true); - when(protocolAdapterWritingService.startWriting(any(), - any(), - any())).thenReturn(true); + when(protocolAdapterWritingService.startWriting(any(), any(), any())).thenReturn(true); final var adapterState = new ProtocolAdapterStateImpl(eventService, "test-adapter", "test-protocol"); final ProtocolAdapterWrapper adapterWrapper = new ProtocolAdapterWrapper(mock(), @@ -200,10 +225,14 @@ void test_startWriting_eventServiceFailedStart_resourcesCleanedUp() throws Excep mock(), adapterState, northboundConsumerFactory, - tagManager); + tagManager, + testExecutor); - protocolAdapterManager.startAsync(adapterWrapper).get(); + // Start will fail, but we expect cleanup to happen + assertThatThrownBy(() -> protocolAdapterManager.startAsync(adapterWrapper).get()).isInstanceOf( + ExecutionException.class).hasCauseInstanceOf(RuntimeException.class).hasMessageContaining("failed"); + // Even though start failed, cleanup should have occurred assertThat(adapterWrapper.getRuntimeStatus()).isEqualTo(ProtocolAdapterState.RuntimeStatus.STOPPED); verify(protocolAdapterWritingService).stopWriting(eq((WritingProtocolAdapter) adapterWrapper.getAdapter()), any()); @@ -214,6 +243,7 @@ void test_stopWritingAdapterSucceeded_eventsFired() throws Exception { final EventBuilder eventBuilder = new EventBuilderImpl(mock()); when(protocolAdapterWritingService.writingEnabled()).thenReturn(true); + when(protocolAdapterWritingService.startWriting(any(), any(), any())).thenReturn(true); when(eventService.createAdapterEvent(anyString(), anyString())).thenReturn(eventBuilder); final var adapterState = new ProtocolAdapterStateImpl(eventService, "test-adapter", "test-protocol"); @@ -226,11 +256,13 @@ void test_stopWritingAdapterSucceeded_eventsFired() throws Exception { mock(), adapterState, northboundConsumerFactory, - tagManager); + tagManager, + testExecutor); - adapterWrapper.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STARTED); + // Start the adapter first to transition FSM state to STARTED + protocolAdapterManager.startAsync(adapterWrapper).get(); - protocolAdapterManager.stopAsync(adapterWrapper, false).get(); + protocolAdapterManager.stopAsync(adapterWrapper).get(); assertThat(adapterWrapper.getRuntimeStatus()).isEqualTo(ProtocolAdapterState.RuntimeStatus.STOPPED); } @@ -240,24 +272,27 @@ void test_stopWritingAdapterFailed_eventsFired() throws Exception { final EventBuilder eventBuilder = new EventBuilderImpl(mock()); when(protocolAdapterWritingService.writingEnabled()).thenReturn(true); + when(protocolAdapterWritingService.startWriting(any(), any(), any())).thenReturn(true); when(eventService.createAdapterEvent(anyString(), anyString())).thenReturn(eventBuilder); final var adapterState = new ProtocolAdapterStateImpl(eventService, "test-adapter", "test-protocol"); + // Use an adapter that succeeds on start but fails on stop final ProtocolAdapterWrapper adapterWrapper = new ProtocolAdapterWrapper(mock(), protocolAdapterWritingService, protocolAdapterPollingService, mock(), - new TestWritingAdapter(false, adapterState), + new TestWritingAdapterFailOnStop(adapterState), mock(), mock(), adapterState, northboundConsumerFactory, - tagManager); + tagManager, + testExecutor); - adapterWrapper.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STARTED); + // Start the adapter first to transition FSM state to STARTED + protocolAdapterManager.startAsync(adapterWrapper).get(); - assertThatThrownBy(() -> protocolAdapterManager.stopAsync(adapterWrapper, false).get()) - .rootCause() + assertThatThrownBy(() -> protocolAdapterManager.stopAsync(adapterWrapper).get()).rootCause() .isInstanceOf(RuntimeException.class) .hasMessage("failed"); @@ -348,8 +383,7 @@ static class TestWritingAdapter implements WritingProtocolAdapter { } @Override - public void write( - final @NotNull WritingInput writingInput, final @NotNull WritingOutput writingOutput) { + public void write(final @NotNull WritingInput writingInput, final @NotNull WritingOutput writingOutput) { } @@ -365,7 +399,8 @@ public void write( @Override public void start( - final @NotNull ProtocolAdapterStartInput input, final @NotNull ProtocolAdapterStartOutput output) { + final @NotNull ProtocolAdapterStartInput input, + final @NotNull ProtocolAdapterStartOutput output) { if (success) { adapterState.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STARTED); output.startedSuccessfully(); @@ -376,7 +411,8 @@ public void start( @Override public void stop( - final @NotNull ProtocolAdapterStopInput input, final @NotNull ProtocolAdapterStopOutput output) { + final @NotNull ProtocolAdapterStopInput input, + final @NotNull ProtocolAdapterStopOutput output) { if (success) { adapterState.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STOPPED); output.stoppedSuccessfully(); @@ -390,4 +426,48 @@ public void stop( return new TestWritingProtocolAdapterInformation(); } } + + static class TestWritingAdapterFailOnStop implements WritingProtocolAdapter { + + final ProtocolAdapterState adapterState; + + TestWritingAdapterFailOnStop(final @NotNull ProtocolAdapterState adapterState) { + this.adapterState = adapterState; + } + + @Override + public void write(final @NotNull WritingInput writingInput, final @NotNull WritingOutput writingOutput) { + + } + + @Override + public @NotNull Class getMqttPayloadClass() { + return null; + } + + @Override + public @NotNull String getId() { + return "test-writing"; + } + + @Override + public void start( + final @NotNull ProtocolAdapterStartInput input, + final @NotNull ProtocolAdapterStartOutput output) { + adapterState.setRuntimeStatus(ProtocolAdapterState.RuntimeStatus.STARTED); + output.startedSuccessfully(); + } + + @Override + public void stop( + final @NotNull ProtocolAdapterStopInput input, + final @NotNull ProtocolAdapterStopOutput output) { + output.failStop(new RuntimeException("failed"), "could not stop"); + } + + @Override + public @NotNull ProtocolAdapterInformation getProtocolAdapterInformation() { + return new TestWritingProtocolAdapterInformation(); + } + } } diff --git a/hivemq-edge/src/test/java/com/hivemq/protocols/ProtocolAdapterWrapperConcurrencyTest.java b/hivemq-edge/src/test/java/com/hivemq/protocols/ProtocolAdapterWrapperConcurrencyTest.java new file mode 100644 index 0000000000..f0a2b24c19 --- /dev/null +++ b/hivemq-edge/src/test/java/com/hivemq/protocols/ProtocolAdapterWrapperConcurrencyTest.java @@ -0,0 +1,499 @@ +/* + * Copyright 2019-present HiveMQ GmbH + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.hivemq.protocols; + +import com.hivemq.adapter.sdk.api.ProtocolAdapter; +import com.hivemq.adapter.sdk.api.ProtocolAdapterInformation; +import com.hivemq.adapter.sdk.api.factories.ProtocolAdapterFactory; +import com.hivemq.adapter.sdk.api.services.ModuleServices; +import com.hivemq.adapter.sdk.api.services.ProtocolAdapterMetricsService; +import com.hivemq.edge.modules.adapters.data.TagManager; +import com.hivemq.edge.modules.adapters.impl.ProtocolAdapterStateImpl; +import com.hivemq.edge.modules.api.adapters.ProtocolAdapterPollingService; +import com.hivemq.fsm.ProtocolAdapterFSM; +import com.hivemq.protocols.northbound.NorthboundConsumerFactory; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import static java.util.Objects.requireNonNull; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * These tests verify: + * - FSM state transitions are atomic + * - Concurrent operations don't cause race conditions + * - State reads are always consistent + * - North/South bound states are properly managed + */ +class ProtocolAdapterWrapperConcurrencyTest { + + private static final int SMALL_THREAD_COUNT = 10; + private static final int MEDIUM_THREAD_COUNT = 20; + private static final int LARGE_THREAD_COUNT = 50; + private static final int OPERATIONS_PER_THREAD = 100; + private static final int TRANSITIONS_PER_THREAD = 20; + private @Nullable ModuleServices mockModuleServices; + private @Nullable ProtocolAdapterWrapper wrapper; + private @Nullable ExecutorService executor; + + private static void verifyStateConsistency(final ProtocolAdapterFSM.State state) { + final ProtocolAdapterFSM.AdapterStateEnum adapterState = state.adapter(); + final ProtocolAdapterFSM.StateEnum northbound = state.northbound(); + final ProtocolAdapterFSM.StateEnum southbound = state.southbound(); + + // Verify all states are non-null + assertNotNull(adapterState, "Adapter state should not be null"); + assertNotNull(northbound, "Northbound state should not be null"); + assertNotNull(southbound, "Southbound state should not be null"); + + switch (adapterState) { + case STOPPED: + // STOPPED is a stable state - all connections must be fully disconnected + assertEquals(ProtocolAdapterFSM.StateEnum.DISCONNECTED, + northbound, + "Northbound should be DISCONNECTED when adapter is STOPPED"); + assertEquals(ProtocolAdapterFSM.StateEnum.DISCONNECTED, + southbound, + "Southbound should be DISCONNECTED when adapter is STOPPED"); + break; + + case STARTING: + case STARTED: + case STOPPING: + // Key invariant: When adapter is STOPPED, both connections MUST be DISCONNECTED. + // Other states allow various connection state combinations during transitions. + break; + + default: + fail("Unknown adapter state: " + adapterState); + } + } + + private void runConcurrentOperations(final int threadCount, final @NotNull Runnable operation) + throws InterruptedException { + executor = Executors.newFixedThreadPool(threadCount); + final CyclicBarrier barrier = new CyclicBarrier(threadCount); + final AtomicInteger attempts = new AtomicInteger(0); + + for (int i = 0; i < threadCount; i++) { + requireNonNull(executor).submit(() -> { + try { + barrier.await(); + attempts.incrementAndGet(); + operation.run(); + } catch (final Exception ignored) { + // Expected - concurrent operations may fail due to state conflicts + } + }); + } + + requireNonNull(executor).shutdown(); + assertTrue(requireNonNull(executor).awaitTermination(10, TimeUnit.SECONDS), "All threads should complete"); + assertEquals(threadCount, attempts.get(), "All attempts should be made"); + } + + private void runReaderWriterPattern( + final int readerThreads, + final int writerThreads, + final @NotNull Runnable readerOperation, + final @NotNull Runnable writerOperation, + final @NotNull AtomicInteger readerCounter) throws InterruptedException { + executor = Executors.newFixedThreadPool(readerThreads + writerThreads); + final CountDownLatch stopLatch = new CountDownLatch(1); + + // Readers + for (int i = 0; i < readerThreads; i++) { + requireNonNull(executor).submit(() -> { + try { + while (!stopLatch.await(0, TimeUnit.MILLISECONDS)) { + readerOperation.run(); + Thread.sleep(1); + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + } + + // Writers + for (int i = 0; i < writerThreads; i++) { + requireNonNull(executor).submit(() -> { + try { + for (int j = 0; j < 15; j++) { + writerOperation.run(); + Thread.sleep(10); + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + } + + Thread.sleep(250); + stopLatch.countDown(); + + requireNonNull(executor).shutdown(); + assertTrue(requireNonNull(executor).awaitTermination(5, TimeUnit.SECONDS), "All threads should complete"); + assertTrue(readerCounter.get() > 0, "Should have performed operations"); + } + + @BeforeEach + void setUp() { + executor = Executors.newCachedThreadPool(); + + final ProtocolAdapter mockAdapter = mock(ProtocolAdapter.class); + when(mockAdapter.getId()).thenReturn("test-adapter"); + when(mockAdapter.getProtocolAdapterInformation()).thenReturn(mock(ProtocolAdapterInformation.class)); + + // Mock adapter.start() to complete the output future immediately + doAnswer(invocation -> { + final com.hivemq.adapter.sdk.api.model.ProtocolAdapterStartOutput output = invocation.getArgument(1); + output.startedSuccessfully(); + return null; + }).when(mockAdapter).start(any(), any()); + + // Mock adapter.stop() to complete the output future immediately + doAnswer(invocation -> { + final com.hivemq.adapter.sdk.api.model.ProtocolAdapterStopOutput output = invocation.getArgument(1); + output.stoppedSuccessfully(); + return null; + }).when(mockAdapter).stop(any(), any()); + + // Mock adapter.destroy() to do nothing + doNothing().when(mockAdapter).destroy(); + + final ProtocolAdapterMetricsService metricsService = mock(ProtocolAdapterMetricsService.class); + final InternalProtocolAdapterWritingService writingService = mock(InternalProtocolAdapterWritingService.class); + final ProtocolAdapterPollingService pollingService = mock(ProtocolAdapterPollingService.class); + final ProtocolAdapterFactory adapterFactory = mock(ProtocolAdapterFactory.class); + final ProtocolAdapterInformation adapterInformation = mock(ProtocolAdapterInformation.class); + final ProtocolAdapterStateImpl adapterState = mock(ProtocolAdapterStateImpl.class); + final NorthboundConsumerFactory consumerFactory = mock(NorthboundConsumerFactory.class); + final TagManager tagManager = mock(TagManager.class); + mockModuleServices = mock(ModuleServices.class); + + final ProtocolAdapterConfig config = mock(ProtocolAdapterConfig.class); + when(config.getAdapterId()).thenReturn("test-adapter"); + when(config.getTags()).thenReturn(java.util.List.of()); + when(config.getNorthboundMappings()).thenReturn(java.util.List.of()); + when(config.getSouthboundMappings()).thenReturn(java.util.List.of()); + + wrapper = new ProtocolAdapterWrapper(metricsService, + writingService, + pollingService, + config, + mockAdapter, + adapterFactory, + adapterInformation, + adapterState, + consumerFactory, + tagManager, + executor); + } + + @AfterEach + void tearDown() throws InterruptedException { + if (executor != null && !executor.isShutdown()) { + executor.shutdown(); + assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + } + } + + @Test + @Timeout(10) + void test_fsmStateTransitions_areAtomic() throws Exception { + executor = Executors.newFixedThreadPool(SMALL_THREAD_COUNT); + final CountDownLatch startLatch = new CountDownLatch(1); + final AtomicInteger invalidTransitions = new AtomicInteger(0); + final AtomicInteger totalAttempts = new AtomicInteger(0); + + for (int i = 0; i < SMALL_THREAD_COUNT; i++) { + final boolean shouldStart = i % 2 == 0; + requireNonNull(executor).submit(() -> { + try { + startLatch.await(); + for (int j = 0; j < TRANSITIONS_PER_THREAD; j++) { + try { + totalAttempts.incrementAndGet(); + if (shouldStart) { + requireNonNull(wrapper).startAdapter(); + } else { + requireNonNull(wrapper).stopAdapter(); + } + + // Verify complete state is valid + final var state = requireNonNull(wrapper).currentState(); + assertNotNull(state, "State should never be null"); + assertNotNull(state.adapter(), "Adapter state should never be null"); + assertNotNull(state.northbound(), "Northbound state should never be null"); + assertNotNull(state.southbound(), "Southbound state should never be null"); + + // Verify state consistency + verifyStateConsistency(state); + + } catch (final IllegalStateException e) { + // Expected when transition is not allowed + } catch (final Exception e) { + invalidTransitions.incrementAndGet(); + } + + Thread.sleep(1); + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + } + + startLatch.countDown(); + requireNonNull(executor).shutdown(); + assertTrue(requireNonNull(executor).awaitTermination(10, TimeUnit.SECONDS), "All threads should complete"); + + // Verify no invalid transitions occurred + assertEquals(0, + invalidTransitions.get(), + "No invalid state transitions should occur (total attempts: " + totalAttempts.get() + ")"); + + // Verify final state is complete and valid + final var finalState = requireNonNull(wrapper).currentState(); + assertNotNull(finalState, "Final state should not be null"); + assertNotNull(finalState.adapter(), "Final adapter state should not be null"); + assertNotNull(finalState.northbound(), "Final northbound state should not be null"); + assertNotNull(finalState.southbound(), "Final southbound state should not be null"); + + // Final state should be STOPPED or STARTED + assertTrue(finalState.adapter() == ProtocolAdapterFSM.AdapterStateEnum.STOPPED || + finalState.adapter() == ProtocolAdapterFSM.AdapterStateEnum.STARTED, + "Final state should be stable: " + finalState.adapter()); + } + + @Test + @Timeout(10) + void test_stateReads_alwaysConsistent() throws Exception { + final int READER_THREADS = 5; + final int WRITER_THREADS = 2; + executor = Executors.newFixedThreadPool(READER_THREADS + WRITER_THREADS); + final CountDownLatch stopLatch = new CountDownLatch(1); + final AtomicInteger inconsistentReads = new AtomicInteger(0); + final AtomicInteger totalReads = new AtomicInteger(0); + + // Reader threads - verify complete state consistency + for (int i = 0; i < READER_THREADS; i++) { + requireNonNull(executor).submit(() -> { + try { + while (!stopLatch.await(0, TimeUnit.MILLISECONDS)) { + final var state = requireNonNull(wrapper).currentState(); + assertNotNull(state, "State should never be null"); + assertNotNull(state.adapter(), "Adapter state should never be null"); + assertNotNull(state.northbound(), "Northbound state should never be null"); + assertNotNull(state.southbound(), "Southbound state should never be null"); + + // Verify state consistency + verifyStateConsistency(state); + + totalReads.incrementAndGet(); + Thread.sleep(1); + } + } catch (final AssertionError e) { + inconsistentReads.incrementAndGet(); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + } + + // Writer threads + for (int i = 0; i < WRITER_THREADS; i++) { + requireNonNull(executor).submit(() -> { + try { + for (int j = 0; j < 10; j++) { + try { + requireNonNull(wrapper).startAdapter(); + Thread.sleep(10); + requireNonNull(wrapper).stopAdapter(); + Thread.sleep(10); + } catch (final IllegalStateException e) { + // Expected + } + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + } + + // Let it run + Thread.sleep(200); + stopLatch.countDown(); + + requireNonNull(executor).shutdown(); + assertTrue(requireNonNull(executor).awaitTermination(5, TimeUnit.SECONDS), "All threads should complete"); + + // Verify no inconsistent reads + assertEquals(0, inconsistentReads.get(), "No inconsistent state reads should occur"); + assertTrue(totalReads.get() > 0, "Should have performed reads: " + totalReads.get()); + } + + @Test + @Timeout(5) + void test_concurrentStateChecks_noExceptions() throws Exception { + executor = Executors.newFixedThreadPool(MEDIUM_THREAD_COUNT); + final CountDownLatch startLatch = new CountDownLatch(1); + final AtomicInteger successfulChecks = new AtomicInteger(0); + final AtomicInteger failures = new AtomicInteger(0); + + for (int i = 0; i < MEDIUM_THREAD_COUNT; i++) { + requireNonNull(executor).submit(() -> { + try { + startLatch.await(); + + for (int j = 0; j < OPERATIONS_PER_THREAD; j++) { + try { + // Various state check operations + requireNonNull(wrapper).currentState(); + requireNonNull(wrapper).getRuntimeStatus(); + requireNonNull(wrapper).getConnectionStatus(); + requireNonNull(wrapper).getId(); + requireNonNull(wrapper).getAdapterInformation(); + successfulChecks.incrementAndGet(); + } catch (final Exception e) { + failures.incrementAndGet(); + } + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + } + + startLatch.countDown(); + requireNonNull(executor).shutdown(); + assertTrue(requireNonNull(executor).awaitTermination(5, TimeUnit.SECONDS), "All threads should complete"); + + assertEquals(0, failures.get(), "No exceptions should occur during concurrent state checks"); + assertTrue(successfulChecks.get() >= MEDIUM_THREAD_COUNT * OPERATIONS_PER_THREAD, + "All state checks should succeed"); + } + + @Test + @Timeout(5) + void test_adapterIdAccess_isThreadSafe() throws Exception { + executor = Executors.newFixedThreadPool(LARGE_THREAD_COUNT); + final CountDownLatch startLatch = new CountDownLatch(1); + final AtomicInteger correctReads = new AtomicInteger(0); + + for (int i = 0; i < LARGE_THREAD_COUNT; i++) { + requireNonNull(executor).submit(() -> { + try { + startLatch.await(); + + for (int j = 0; j < OPERATIONS_PER_THREAD; j++) { + final String adapterId = requireNonNull(wrapper).getId(); + if ("test-adapter".equals(adapterId)) { + correctReads.incrementAndGet(); + } + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + } + + startLatch.countDown(); + requireNonNull(executor).shutdown(); + assertTrue(requireNonNull(executor).awaitTermination(5, TimeUnit.SECONDS), "All threads should complete"); + + assertEquals(LARGE_THREAD_COUNT * OPERATIONS_PER_THREAD, + correctReads.get(), + "All adapter ID reads should be correct"); + } + + @Test + @Timeout(10) + void test_concurrentStartAsync_properSerialization() throws Exception { + runConcurrentOperations(SMALL_THREAD_COUNT, () -> { + try { + requireNonNull(wrapper).startAsync(requireNonNull(mockModuleServices)).get(); + } catch (final Exception e) { + // Expected - concurrent operations may fail + } + }); + + final var state = requireNonNull(wrapper).currentState(); + assertNotNull(state); + assertNotNull(state.adapter()); + } + + @Test + @Timeout(10) + void test_concurrentStopAsync_properSerialization() throws Exception { + requireNonNull(wrapper).startAsync(requireNonNull(mockModuleServices)); + Thread.sleep(100); + + runConcurrentOperations(SMALL_THREAD_COUNT, () -> requireNonNull(wrapper).stopAsync()); + + final var state = requireNonNull(wrapper).currentState(); + assertNotNull(state); + assertNotNull(state.adapter()); + } + + @Test + @Timeout(10) + void test_statusQueriesDuringTransitions_noExceptions() throws Exception { + final int READER_THREADS = 5; + final int WRITER_THREADS = 3; + final AtomicInteger totalReads = new AtomicInteger(0); + final AtomicInteger exceptions = new AtomicInteger(0); + + // Test both runtime and connection status reads during transitions + runReaderWriterPattern(READER_THREADS, WRITER_THREADS, () -> { + try { + requireNonNull(wrapper).getRuntimeStatus(); // May return null with mocks + requireNonNull(wrapper).getConnectionStatus(); // May return null with mocks + totalReads.incrementAndGet(); + } catch (final Exception e) { + exceptions.incrementAndGet(); + } + }, () -> { + try { + requireNonNull(wrapper).startAdapter(); + } catch (final IllegalStateException ignored) { + // Expected + } + }, totalReads); + + assertEquals(0, exceptions.get(), "No exceptions during status queries"); + } +} diff --git a/modules/hivemq-edge-module-databases/src/main/java/com/hivemq/edge/adapters/databases/DatabaseConnection.java b/modules/hivemq-edge-module-databases/src/main/java/com/hivemq/edge/adapters/databases/DatabaseConnection.java index c601f37206..bf445c3b39 100644 --- a/modules/hivemq-edge-module-databases/src/main/java/com/hivemq/edge/adapters/databases/DatabaseConnection.java +++ b/modules/hivemq-edge-module-databases/src/main/java/com/hivemq/edge/adapters/databases/DatabaseConnection.java @@ -22,27 +22,32 @@ import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import java.sql.Connection; import java.sql.SQLException; import java.util.Properties; +import java.util.concurrent.atomic.AtomicBoolean; public class DatabaseConnection { private static final @NotNull Logger log = LoggerFactory.getLogger(DatabaseConnection.class); private final @NotNull HikariConfig config; - private @Nullable HikariDataSource ds; + private final @NotNull AtomicBoolean connected; + private volatile @Nullable HikariDataSource ds; - public DatabaseConnection(final @NotNull DatabaseType dbType, - final @NotNull String server, - final @NotNull Integer port, - final @NotNull String database, - final @NotNull String username, - final @NotNull String password, - final int connectionTimeout, - final boolean encrypt) { + public DatabaseConnection( + final @NotNull DatabaseType dbType, + final @NotNull String server, + final @NotNull Integer port, + final @NotNull String database, + final @NotNull String username, + final @NotNull String password, + final int connectionTimeout, + final boolean encrypt) { config = new HikariConfig(); + connected = new AtomicBoolean(false); - switch (dbType){ + switch (dbType) { case POSTGRESQL -> { config.setDataSourceClassName("org.postgresql.ds.PGSimpleDataSource"); config.addDataSourceProperty("serverName", server); @@ -77,27 +82,45 @@ public DatabaseConnection(final @NotNull DatabaseType dbType, if (encrypt) { properties.setProperty("encrypt", "true"); properties.setProperty("trustServerCertificate", "true"); // Trust the server certificate implicitly - } else properties.setProperty("encrypt", "false"); + } else { + properties.setProperty("encrypt", "false"); + } config.setDataSourceProperties(properties); } } } public void connect() { + if (!connected.compareAndSet(false, true)) { + log.debug("Database connection already established, skipping connect"); + return; // Already connected + } log.debug("Connection settings : {}", config.toString()); this.ds = new HikariDataSource(config); } public @NotNull Connection getConnection() throws SQLException { - if (ds == null) { + if (!connected.get()) { throw new IllegalStateException("Hikari Connection Pool must be started before usage."); } return ds.getConnection(); } public void close() { - if (ds != null) { - ds.close(); + if (!connected.compareAndSet(true, false)) { + log.debug("Database connection already closed or not connected"); + return; + } + if (ds != null && !ds.isClosed()) { + log.debug("Closing HikariCP datasource"); + try { + ds.close(); + log.debug("HikariCP datasource closed successfully"); + } catch (final Exception e) { + log.error("Error closing HikariCP datasource", e); + } finally { + ds = null; + } } } } diff --git a/modules/hivemq-edge-module-databases/src/main/java/com/hivemq/edge/adapters/databases/DatabasesPollingProtocolAdapter.java b/modules/hivemq-edge-module-databases/src/main/java/com/hivemq/edge/adapters/databases/DatabasesPollingProtocolAdapter.java index a77486625b..f1b6940a52 100644 --- a/modules/hivemq-edge-module-databases/src/main/java/com/hivemq/edge/adapters/databases/DatabasesPollingProtocolAdapter.java +++ b/modules/hivemq-edge-module-databases/src/main/java/com/hivemq/edge/adapters/databases/DatabasesPollingProtocolAdapter.java @@ -18,7 +18,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import com.hivemq.adapter.sdk.api.ProtocolAdapterInformation; -import com.hivemq.adapter.sdk.api.config.PollingContext; import com.hivemq.adapter.sdk.api.factories.AdapterFactories; import com.hivemq.adapter.sdk.api.factories.DataPointFactory; import com.hivemq.adapter.sdk.api.model.ProtocolAdapterInput; @@ -26,20 +25,15 @@ import com.hivemq.adapter.sdk.api.model.ProtocolAdapterStartOutput; import com.hivemq.adapter.sdk.api.model.ProtocolAdapterStopInput; import com.hivemq.adapter.sdk.api.model.ProtocolAdapterStopOutput; -import com.hivemq.adapter.sdk.api.polling.PollingInput; -import com.hivemq.adapter.sdk.api.polling.PollingOutput; -import com.hivemq.adapter.sdk.api.polling.PollingProtocolAdapter; import com.hivemq.adapter.sdk.api.polling.batch.BatchPollingInput; import com.hivemq.adapter.sdk.api.polling.batch.BatchPollingOutput; import com.hivemq.adapter.sdk.api.polling.batch.BatchPollingProtocolAdapter; import com.hivemq.adapter.sdk.api.state.ProtocolAdapterState; import com.hivemq.adapter.sdk.api.tag.Tag; -import com.hivemq.edge.adapters.databases.config.DatabaseType; import com.hivemq.edge.adapters.databases.config.DatabasesAdapterConfig; import com.hivemq.edge.adapters.databases.config.DatabasesAdapterTag; import com.hivemq.edge.adapters.databases.config.DatabasesAdapterTagDefinition; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,15 +45,15 @@ import java.sql.Types; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import static com.hivemq.adapter.sdk.api.state.ProtocolAdapterState.ConnectionStatus.STATELESS; - public class DatabasesPollingProtocolAdapter implements BatchPollingProtocolAdapter { + public static final int TIMEOUT = 30; private static final @NotNull Logger log = LoggerFactory.getLogger(DatabasesPollingProtocolAdapter.class); private static final @NotNull ObjectMapper OBJECT_MAPPER = new ObjectMapper(); - public static final int TIMEOUT = 30; private final @NotNull DatabasesAdapterConfig adapterConfig; private final @NotNull ProtocolAdapterInformation adapterInformation; @@ -68,6 +62,7 @@ public class DatabasesPollingProtocolAdapter implements BatchPollingProtocolAdap private final @NotNull List tags; private final @NotNull DatabaseConnection databaseConnection; private final @NotNull AdapterFactories adapterFactories; + private final @NotNull AtomicBoolean started; public DatabasesPollingProtocolAdapter( final @NotNull ProtocolAdapterInformation adapterInformation, @@ -78,9 +73,8 @@ public DatabasesPollingProtocolAdapter( this.protocolAdapterState = input.getProtocolAdapterState(); this.tags = input.getTags(); this.adapterFactories = input.adapterFactories(); - + this.started = new AtomicBoolean(false); log.debug("Building connection string"); - this.databaseConnection = new DatabaseConnection(adapterConfig.getType(), adapterConfig.getServer(), adapterConfig.getPort(), @@ -98,48 +92,64 @@ public DatabasesPollingProtocolAdapter( @Override public void start( - final @NotNull ProtocolAdapterStartInput input, final @NotNull ProtocolAdapterStartOutput output) { - log.debug("Loading PostgreSQL Driver"); - try { - Class.forName("org.postgresql.Driver"); - } catch (final ClassNotFoundException e) { - output.failStart(e, null); + final @NotNull ProtocolAdapterStartInput input, + final @NotNull ProtocolAdapterStartOutput output) { + if (!started.compareAndSet(false, true)) { + log.debug("Database adapter {} already started, returning success", adapterId); + output.startedSuccessfully(); return; } - log.debug("Loading MariaDB Driver (for MySQL)"); try { - Class.forName("org.mariadb.jdbc.Driver"); - } catch (final ClassNotFoundException e) { - output.failStart(e, null); - return; - } - - log.debug("Loading MS SQL Driver"); - try { - Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDataSource"); - } catch (final ClassNotFoundException e) { - output.failStart(e, null); - return; - } + log.debug("Loading PostgreSQL Driver"); + try { + Class.forName("org.postgresql.Driver"); + } catch (final ClassNotFoundException e) { + output.failStart(e, null); + started.set(false); + return; + } + log.debug("Loading MariaDB Driver (for MySQL)"); + try { + Class.forName("org.mariadb.jdbc.Driver"); + } catch (final ClassNotFoundException e) { + output.failStart(e, null); + started.set(false); + return; + } + log.debug("Loading MS SQL Driver"); + try { + Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDataSource"); + } catch (final ClassNotFoundException e) { + output.failStart(e, null); + started.set(false); + return; + } - databaseConnection.connect(); + databaseConnection.connect(); - try { - log.debug("Starting connection to the database instance"); - if (databaseConnection.getConnection().isValid(TIMEOUT)) { - output.startedSuccessfully(); - protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.CONNECTED); - } else { - output.failStart(new Throwable("Error connecting database, please check the configuration"), - "Error connecting database, please check the configuration"); + try { + log.debug("Starting connection to the database instance"); + if (databaseConnection.getConnection().isValid(TIMEOUT)) { + output.startedSuccessfully(); + protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.CONNECTED); + } else { + output.failStart(new Throwable("Error connecting database, please check the configuration"), + "Error connecting database, please check the configuration"); + protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.DISCONNECTED); + started.set(false); + } + } catch (final Exception e) { + output.failStart(e, null); protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.DISCONNECTED); + started.set(false); } } catch (final Exception e) { + log.error("Unexpected error during adapter start", e); output.failStart(e, null); - protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.DISCONNECTED); + started.set(false); } } @@ -147,10 +157,21 @@ public void start( public void stop( final @NotNull ProtocolAdapterStopInput protocolAdapterStopInput, final @NotNull ProtocolAdapterStopOutput protocolAdapterStopOutput) { + if (!started.compareAndSet(true, false)) { + log.debug("Database adapter {} already stopped, returning success", adapterId); + protocolAdapterStopOutput.stoppedSuccessfully(); + return; + } databaseConnection.close(); protocolAdapterStopOutput.stoppedSuccessfully(); } + @Override + public void destroy() { + log.debug("Destroying database adapter with id '{}'", adapterId); + // Ensure connection pool is fully closed to prevent thread leaks + databaseConnection.close(); + } @Override public @NotNull ProtocolAdapterInformation getProtocolAdapterInformation() { @@ -250,5 +271,4 @@ public int getPollingIntervalMillis() { public int getMaxPollingErrorsBeforeRemoval() { return adapterConfig.getMaxPollingErrorsBeforeRemoval(); } - } diff --git a/modules/hivemq-edge-module-etherip/src/main/java/com/hivemq/edge/adapters/etherip/EipPollingProtocolAdapter.java b/modules/hivemq-edge-module-etherip/src/main/java/com/hivemq/edge/adapters/etherip/EipPollingProtocolAdapter.java index 20f3f4f0e6..68496ac773 100644 --- a/modules/hivemq-edge-module-etherip/src/main/java/com/hivemq/edge/adapters/etherip/EipPollingProtocolAdapter.java +++ b/modules/hivemq-edge-module-etherip/src/main/java/com/hivemq/edge/adapters/etherip/EipPollingProtocolAdapter.java @@ -38,6 +38,8 @@ import java.util.List; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; @@ -50,8 +52,9 @@ public class EipPollingProtocolAdapter implements BatchPollingProtocolAdapter { private final @NotNull ProtocolAdapterState protocolAdapterState; protected final @NotNull AdapterFactories adapterFactories; private final @NotNull String adapterId; - private volatile @Nullable EtherNetIP etherNetIP; - private final @NotNull PublishChangedDataOnlyHandler lastSamples = new PublishChangedDataOnlyHandler(); + private final @NotNull Lock connectionLock; + private @Nullable EtherNetIP etherNetIP; // GuardedBy connectionLock + private final @NotNull PublishChangedDataOnlyHandler lastSamples; private final @NotNull DataPointFactory dataPointFactory; private final @NotNull Map tags; @@ -69,6 +72,8 @@ public EipPollingProtocolAdapter( .collect(Collectors.toMap(tag -> tag.getDefinition().getAddress(), tag -> tag)); this.protocolAdapterState = input.getProtocolAdapterState(); this.adapterFactories = input.adapterFactories(); + this.connectionLock = new ReentrantLock(); + this.lastSamples = new PublishChangedDataOnlyHandler(); } @Override @@ -79,15 +84,24 @@ public EipPollingProtocolAdapter( @Override public void start( final @NotNull ProtocolAdapterStartInput input, final @NotNull ProtocolAdapterStartOutput output) { - // any setup which should be done before the adapter starts polling comes here. + connectionLock.lock(); try { - final EtherNetIP etherNetIP = new EtherNetIP(adapterConfig.getHost(), adapterConfig.getSlot()); - etherNetIP.connectTcp(); - this.etherNetIP = etherNetIP; - output.startedSuccessfully(); + if (etherNetIP != null) { + log.warn("Adapter {} is already started, ignoring start request", adapterId); + output.startedSuccessfully(); + return; + } + + final EtherNetIP newConnection = new EtherNetIP(adapterConfig.getHost(), adapterConfig.getSlot()); + newConnection.connectTcp(); + etherNetIP = newConnection; protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.CONNECTED); + output.startedSuccessfully(); } catch (final Exception e) { + protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.DISCONNECTED); output.failStart(e, null); + } finally { + connectionLock.unlock(); } } @@ -95,20 +109,27 @@ public void start( public void stop( final @NotNull ProtocolAdapterStopInput protocolAdapterStopInput, final @NotNull ProtocolAdapterStopOutput protocolAdapterStopOutput) { + connectionLock.lock(); try { final EtherNetIP etherNetIPTemp = etherNetIP; etherNetIP = null; + protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.DISCONNECTED); if (etherNetIPTemp != null) { - etherNetIPTemp.close(); - protocolAdapterStopOutput.stoppedSuccessfully(); - log.info("Stopped"); + try { + etherNetIPTemp.close(); + log.info("Stopped adapter {}", adapterId); + } catch (final Exception e) { + log.warn("Error closing EtherNetIP connection for adapter {}", adapterId, e); + } } else { - protocolAdapterStopOutput.stoppedSuccessfully(); - log.info("Stopped without an open connection"); + log.info("Stopped adapter {} without an open connection", adapterId); } + protocolAdapterStopOutput.stoppedSuccessfully(); } catch (final Exception e) { protocolAdapterStopOutput.failStop(e, "Unable to stop Ethernet IP connection"); - log.error("Unable to stop", e); + log.error("Unable to stop adapter {}", adapterId, e); + } finally { + connectionLock.unlock(); } } @@ -121,10 +142,16 @@ public void stop( @Override public void poll( final @NotNull BatchPollingInput pollingInput, final @NotNull BatchPollingOutput pollingOutput) { - final var client = etherNetIP; - if (client == null) { - pollingOutput.fail("Polling failed because adapter wasn't started."); - return; + final EtherNetIP client; + connectionLock.lock(); + try { + client = etherNetIP; + if (client == null) { + pollingOutput.fail("Polling failed because adapter wasn't started."); + return; + } + } finally { + connectionLock.unlock(); } final var tagAddresses = tags.values().stream().map(v -> v.getDefinition().getAddress()).toArray(String[]::new); @@ -148,14 +175,14 @@ public void poll( pollingOutput.finish(); } catch (final CipException e) { if (e.getStatusCode() == 0x04) { - log.warn("A Tag doesn't exist on device.", e); + log.warn("A Tag doesn't exist on device for adapter {}", adapterId, e); pollingOutput.fail(e, "Tag doesn't exist on device"); } else { - log.warn("Problem accessing tag on device.", e); + log.warn("Problem accessing tag on device for adapter {}", adapterId, e); pollingOutput.fail(e, "Problem accessing tag on device."); } } catch (final Exception e) { - log.warn("An exception occurred while reading tags '{}'.", tagAddresses, e); + log.warn("An exception occurred while reading tags '{}' for adapter {}", tagAddresses, adapterId, e); pollingOutput.fail(e, "An exception occurred while reading tags '" + tagAddresses + "'."); } } @@ -169,5 +196,4 @@ public int getPollingIntervalMillis() { public int getMaxPollingErrorsBeforeRemoval() { return adapterConfig.getEipToMqttConfig().getMaxPollingErrorsBeforeRemoval(); } - } diff --git a/modules/hivemq-edge-module-etherip/src/main/java/com/hivemq/edge/adapters/etherip/PublishChangedDataOnlyHandler.java b/modules/hivemq-edge-module-etherip/src/main/java/com/hivemq/edge/adapters/etherip/PublishChangedDataOnlyHandler.java index 24b4987ccf..4bfa6548eb 100644 --- a/modules/hivemq-edge-module-etherip/src/main/java/com/hivemq/edge/adapters/etherip/PublishChangedDataOnlyHandler.java +++ b/modules/hivemq-edge-module-etherip/src/main/java/com/hivemq/edge/adapters/etherip/PublishChangedDataOnlyHandler.java @@ -39,7 +39,9 @@ public boolean replaceIfValueIsNew(final @NotNull String tagName, final @NotNull } }); - return newValue != computedValue; + // Return true if the value was actually replaced (i.e., computedValue is the new value) + // When values are equal, compute() returns the old value, so references will differ + return computedValue == newValue; } public void clear() { diff --git a/modules/hivemq-edge-module-file/src/main/java/com/hivemq/edge/adapters/file/FilePollingProtocolAdapter.java b/modules/hivemq-edge-module-file/src/main/java/com/hivemq/edge/adapters/file/FilePollingProtocolAdapter.java index 1278d31e56..d5a5767c51 100644 --- a/modules/hivemq-edge-module-file/src/main/java/com/hivemq/edge/adapters/file/FilePollingProtocolAdapter.java +++ b/modules/hivemq-edge-module-file/src/main/java/com/hivemq/edge/adapters/file/FilePollingProtocolAdapter.java @@ -37,6 +37,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; public class FilePollingProtocolAdapter implements BatchPollingProtocolAdapter { @@ -48,6 +49,7 @@ public class FilePollingProtocolAdapter implements BatchPollingProtocolAdapter { private final @NotNull ProtocolAdapterInformation adapterInformation; private final @NotNull ProtocolAdapterState protocolAdapterState; private final @NotNull List tags; + private final @NotNull AtomicBoolean started; public FilePollingProtocolAdapter( final @NotNull String adapterId, @@ -58,6 +60,7 @@ public FilePollingProtocolAdapter( this.adapterConfig = input.getConfig(); this.tags = input.getTags().stream().map(tag -> (FileTag)tag).toList(); this.protocolAdapterState = input.getProtocolAdapterState(); + this.started = new AtomicBoolean(false); } @Override @@ -68,11 +71,17 @@ public FilePollingProtocolAdapter( @Override public void start( final @NotNull ProtocolAdapterStartInput input, final @NotNull ProtocolAdapterStartOutput output) { + if (!started.compareAndSet(false, true)) { + LOG.debug("File adapter {} already started, returning success", adapterId); + output.startedSuccessfully(); + return; + } // any setup which should be done before the adapter starts polling comes here. try { protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.STATELESS); output.startedSuccessfully(); } catch (final Exception e) { + started.set(false); output.failStart(e, null); } } @@ -81,6 +90,11 @@ public void start( public void stop( final @NotNull ProtocolAdapterStopInput protocolAdapterStopInput, final @NotNull ProtocolAdapterStopOutput protocolAdapterStopOutput) { + if (!started.compareAndSet(true, false)) { + LOG.debug("File adapter {} already stopped, returning success", adapterId); + protocolAdapterStopOutput.stoppedSuccessfully(); + return; + } protocolAdapterStopOutput.stoppedSuccessfully(); } diff --git a/modules/hivemq-edge-module-http/src/main/java/com/hivemq/edge/adapters/http/HttpProtocolAdapter.java b/modules/hivemq-edge-module-http/src/main/java/com/hivemq/edge/adapters/http/HttpProtocolAdapter.java index 45df1d5a4c..9c5172a263 100644 --- a/modules/hivemq-edge-module-http/src/main/java/com/hivemq/edge/adapters/http/HttpProtocolAdapter.java +++ b/modules/hivemq-edge-module-http/src/main/java/com/hivemq/edge/adapters/http/HttpProtocolAdapter.java @@ -60,16 +60,13 @@ import java.util.Base64; import java.util.List; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicBoolean; import static com.hivemq.adapter.sdk.api.state.ProtocolAdapterState.ConnectionStatus.ERROR; import static com.hivemq.adapter.sdk.api.state.ProtocolAdapterState.ConnectionStatus.STATELESS; import static com.hivemq.edge.adapters.http.config.HttpSpecificAdapterConfig.JSON_MIME_TYPE; import static com.hivemq.edge.adapters.http.config.HttpSpecificAdapterConfig.PLAIN_MIME_TYPE; -/** - * @author HiveMQ Adapter Generator - */ public class HttpProtocolAdapter implements BatchPollingProtocolAdapter { private static final @NotNull Logger log = LoggerFactory.getLogger(HttpProtocolAdapter.class); @@ -77,7 +74,6 @@ public class HttpProtocolAdapter implements BatchPollingProtocolAdapter { private static final @NotNull String CONTENT_TYPE_HEADER = "Content-Type"; private static final @NotNull String BASE64_ENCODED_VALUE = "data:%s;base64,%s"; private static final @NotNull String USER_AGENT_HEADER = "User-Agent"; - private static final @NotNull String RESPONSE_DATA = "httpResponseData"; private final @NotNull ProtocolAdapterInformation adapterInformation; private final @NotNull HttpSpecificAdapterConfig adapterConfig; @@ -89,6 +85,7 @@ public class HttpProtocolAdapter implements BatchPollingProtocolAdapter { private final @NotNull String adapterId; private final @Nullable ObjectMapper objectMapper; + private final @NotNull AtomicBoolean started; private volatile @Nullable HttpClient httpClient = null; public HttpProtocolAdapter( @@ -97,12 +94,17 @@ public HttpProtocolAdapter( this.adapterId = input.getAdapterId(); this.adapterInformation = adapterInformation; this.adapterConfig = input.getConfig(); - this.tags = input.getTags().stream().map(tag -> (HttpTag)tag).toList(); + this.tags = input.getTags().stream().map(tag -> (HttpTag) tag).toList(); this.version = input.getVersion(); this.protocolAdapterState = input.getProtocolAdapterState(); this.moduleServices = input.moduleServices(); this.adapterFactories = input.adapterFactories(); this.objectMapper = new ObjectMapper(); + this.started = new AtomicBoolean(false); + } + + private static boolean isSuccessStatusCode(final int statusCode) { + return statusCode >= 200 && statusCode <= 299; } @Override @@ -114,6 +116,11 @@ public HttpProtocolAdapter( public void start( final @NotNull ProtocolAdapterStartInput input, final @NotNull ProtocolAdapterStartOutput output) { + if (!started.compareAndSet(false, true)) { + // Already started, idempotent - just return success + output.startedSuccessfully(); + return; + } try { protocolAdapterState.setConnectionStatus(STATELESS); if (httpClient == null) { @@ -128,25 +135,36 @@ public void start( } output.startedSuccessfully(); } catch (final Exception e) { + started.set(false); output.failStart(e, "Unable to start http protocol adapter."); } } @Override public void stop(final @NotNull ProtocolAdapterStopInput input, final @NotNull ProtocolAdapterStopOutput output) { + if (!started.compareAndSet(true, false)) { + // Already stopped, idempotent - just return success + output.stoppedSuccessfully(); + return; + } httpClient = null; output.stoppedSuccessfully(); } + @Override + public void destroy() { + log.debug("Destroying HTTP adapter with id '{}'", adapterId); + // Clear the HTTP client reference to allow GC to clean up the internal executor + httpClient = null; + } + @Override public @NotNull ProtocolAdapterInformation getProtocolAdapterInformation() { return adapterInformation; } @Override - public void poll( - final @NotNull BatchPollingInput pollingInput, final @NotNull BatchPollingOutput pollingOutput) { - + public void poll(final @NotNull BatchPollingInput pollingInput, final @NotNull BatchPollingOutput pollingOutput) { final HttpClient httpClient = this.httpClient; if (httpClient == null) { pollingOutput.fail(new ProtocolAdapterException(), "No response was created, because the client is null."); @@ -156,46 +174,46 @@ public void poll( final List> pollingFutures = tags.stream().map(tag -> pollHttp(httpClient, tag)).toList(); - CompletableFuture.allOf(pollingFutures.toArray(new CompletableFuture[]{})) - .whenComplete((result, throwable) -> { - if(throwable != null) { - pollingOutput.fail(throwable, "Error while polling tags."); - } else { - try { - for (final CompletableFuture future : pollingFutures) { - final var data = future.get(); - if (data.isSuccessStatusCode()) { - protocolAdapterState.setConnectionStatus(STATELESS); - } else { - protocolAdapterState.setConnectionStatus(ERROR); - } - if (data.isSuccessStatusCode() || - !adapterConfig.getHttpToMqttConfig().isHttpPublishSuccessStatusCodeOnly()) { - data.getDataPoints().forEach(pollingOutput::addDataPoint); - } - } - pollingOutput.finish(); - } catch (final InterruptedException | ExecutionException e) { - pollingOutput.fail(e, "Exception while accessing data of completed future."); + CompletableFuture.allOf(pollingFutures.toArray(new CompletableFuture[]{})).whenComplete((result, throwable) -> { + if (throwable != null) { + pollingOutput.fail(throwable, "Error while polling tags."); + } else { + try { + for (final CompletableFuture future : pollingFutures) { + final var data = future.get(); + // Update connection status to ERROR if HTTP request failed + if (!data.isSuccessStatusCode()) { + protocolAdapterState.setConnectionStatus(ERROR); + } else { + protocolAdapterState.setConnectionStatus(STATELESS); + } + if (data.isSuccessStatusCode() || + !adapterConfig.getHttpToMqttConfig().isHttpPublishSuccessStatusCodeOnly()) { + data.getDataPoints().forEach(pollingOutput::addDataPoint); } } - }); + pollingOutput.finish(); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + pollingOutput.fail(e, "Interrupted while accessing data of completed future."); + } catch (final Throwable e) { + pollingOutput.fail(e, "Exception while accessing data of completed future."); + } + } + }); } - private CompletableFuture pollHttp( + private @NotNull CompletableFuture pollHttp( final @NotNull HttpClient httpClient, final @NotNull HttpTag httpTag) { - final HttpRequest.Builder builder = HttpRequest.newBuilder(); - final String url = httpTag.getDefinition().getUrl(); final HttpTagDefinition tagDef = httpTag.getDefinition(); - builder.uri(URI.create(url)); - builder.timeout(Duration.ofSeconds(httpTag.getDefinition().getHttpRequestTimeoutSeconds())); + final HttpRequest.Builder builder = HttpRequest.newBuilder(); + builder.uri(URI.create(tagDef.getUrl())); + builder.timeout(Duration.ofSeconds(tagDef.getHttpRequestTimeoutSeconds())); builder.setHeader(USER_AGENT_HEADER, String.format("HiveMQ-Edge; %s", version)); - tagDef.getHttpHeaders().forEach(hv -> builder.setHeader(hv.getName(), hv.getValue())); - switch (tagDef.getHttpRequestMethod()) { case GET: builder.GET(); @@ -217,31 +235,29 @@ private CompletableFuture pollHttp( builder.header(CONTENT_TYPE_HEADER, tagDef.getHttpRequestBodyContentType().getMimeType()); break; default: - return CompletableFuture - .failedFuture( - new IllegalStateException("There was an unexpected value present in the request config: " + tagDef.getHttpRequestMethod())); + return CompletableFuture.failedFuture(new IllegalStateException( + "There was an unexpected value present in the request config: " + + tagDef.getHttpRequestMethod())); } - - return httpClient - .sendAsync(builder.build(), HttpResponse.BodyHandlers.ofString()) - .thenApply(httpResponse -> getHttpData(httpResponse, url, httpTag.getName())); + return httpClient.sendAsync(builder.build(), HttpResponse.BodyHandlers.ofString()) + .thenApply(httpResponse -> getHttpData(httpResponse, tagDef.getUrl(), httpTag.getName())); } - private @NotNull HttpData getHttpData(final HttpResponse httpResponse, final String url, - final @NotNull String tagName) { + private @NotNull HttpData getHttpData( + final @NotNull HttpResponse httpResponse, + final @NotNull String url, + final @NotNull String tagName) { Object payloadData = null; - String responseContentType = null; - + String contentType = null; if (isSuccessStatusCode(httpResponse.statusCode())) { final String bodyData = httpResponse.body(); //-- if the content type is json, then apply the JSON to the output data, //-- else encode using base64 (as we dont know what the content is). if (bodyData != null) { - responseContentType = httpResponse.headers().firstValue(CONTENT_TYPE_HEADER).orElse(null); - responseContentType = adapterConfig.getHttpToMqttConfig().isAssertResponseIsJson() ? - JSON_MIME_TYPE : - responseContentType; - if (JSON_MIME_TYPE.equals(responseContentType)) { + contentType = httpResponse.headers().firstValue(CONTENT_TYPE_HEADER).orElse(null); + contentType = + adapterConfig.getHttpToMqttConfig().isAssertResponseIsJson() ? JSON_MIME_TYPE : contentType; + if (JSON_MIME_TYPE.equals(contentType)) { try { payloadData = objectMapper.readTree(bodyData); } catch (final Exception e) { @@ -258,23 +274,20 @@ private CompletableFuture pollHttp( throw new RuntimeException("unable to parse JSON data from HTTP response"); } } else { - if (responseContentType == null) { - responseContentType = PLAIN_MIME_TYPE; + if (contentType == null) { + contentType = PLAIN_MIME_TYPE; } - final String base64 = - Base64.getEncoder().encodeToString(bodyData.getBytes(StandardCharsets.UTF_8)); - payloadData = String.format(BASE64_ENCODED_VALUE, responseContentType, base64); + payloadData = String.format(BASE64_ENCODED_VALUE, + contentType, + Base64.getEncoder().encodeToString(bodyData.getBytes(StandardCharsets.UTF_8))); } } } - final HttpData data = new HttpData(url, - httpResponse.statusCode(), - responseContentType, - adapterFactories.dataPointFactory()); - //When the body is empty, just include the metadata + final HttpData data = + new HttpData(url, httpResponse.statusCode(), contentType, adapterFactories.dataPointFactory()); if (payloadData != null) { - data.addDataPoint(tagName, payloadData); + data.addDataPoint(tagName, payloadData); // when the body is empty, just include the metadata } return data; } @@ -291,26 +304,25 @@ public int getMaxPollingErrorsBeforeRemoval() { @Override public void createTagSchema( - final @NotNull TagSchemaCreationInput input, final @NotNull TagSchemaCreationOutput output) { + final @NotNull TagSchemaCreationInput input, + final @NotNull TagSchemaCreationOutput output) { output.finish(JsonSchema.createJsonSchema()); } - private static boolean isSuccessStatusCode(final int statusCode) { - return statusCode >= 200 && statusCode <= 299; - } - - protected @NotNull SSLContext createTrustAllContext() { + private @NotNull SSLContext createTrustAllContext() { try { - final SSLContext sslContext = SSLContext.getInstance("TLS"); - final X509ExtendedTrustManager trustManager = new X509ExtendedTrustManager() { + final @NotNull SSLContext sslContext = SSLContext.getInstance("TLS"); + final @NotNull X509ExtendedTrustManager trustManager = new X509ExtendedTrustManager() { @Override public void checkClientTrusted( - final X509Certificate @NotNull [] x509Certificates, final @NotNull String s) { + final X509Certificate @NotNull [] x509Certificates, + final @NotNull String s) { } @Override public void checkServerTrusted( - final X509Certificate @NotNull [] x509Certificates, final @NotNull String s) { + final X509Certificate @NotNull [] x509Certificates, + final @NotNull String s) { } @Override @@ -352,5 +364,4 @@ public void checkServerTrusted( throw new RuntimeException(e); } } - } diff --git a/modules/hivemq-edge-module-modbus/src/main/java/com/hivemq/edge/adapters/modbus/ModbusProtocolAdapter.java b/modules/hivemq-edge-module-modbus/src/main/java/com/hivemq/edge/adapters/modbus/ModbusProtocolAdapter.java index 68fd3e3a4e..c3e917974e 100644 --- a/modules/hivemq-edge-module-modbus/src/main/java/com/hivemq/edge/adapters/modbus/ModbusProtocolAdapter.java +++ b/modules/hivemq-edge-module-modbus/src/main/java/com/hivemq/edge/adapters/modbus/ModbusProtocolAdapter.java @@ -139,33 +139,32 @@ public void start( @Override public void stop(final @NotNull ProtocolAdapterStopInput input, final @NotNull ProtocolAdapterStopOutput output) { - if (startRequested.get() && stopRequested.compareAndSet(false, true)) { + if (stopRequested.compareAndSet(false, true)) { log.info("Stopping Modbus protocol adapter {}", adapterId); publishChangedDataOnlyHandler.clear(); try { - client - .disconnect() - .whenComplete((unused, throwable) -> { - try { - if (throwable == null) { - protocolAdapterState.setConnectionStatus(DISCONNECTED); - output.stoppedSuccessfully(); - log.info("Successfully stopped Modbus protocol adapter {}", adapterId); - } else { - protocolAdapterState.setConnectionStatus(ERROR); - output.failStop(throwable, "Error encountered closing connection to Modbus server."); - log.error("Unable to stop the connection to the Modbus server", throwable); - } - } finally { - startRequested.set(false); - stopRequested.set(false); + client.disconnect().whenComplete((unused, throwable) -> { + try { + if (throwable == null) { + protocolAdapterState.setConnectionStatus(DISCONNECTED); + output.stoppedSuccessfully(); + log.info("Successfully stopped Modbus protocol adapter {}", adapterId); + } else { + protocolAdapterState.setConnectionStatus(ERROR); + output.failStop(throwable, "Error encountered closing connection to Modbus server."); + log.error("Unable to stop the connection to the Modbus server", throwable); } - }) - .toCompletableFuture() - .get(); + } finally { + startRequested.set(false); + stopRequested.set(false); + } + }).toCompletableFuture().get(); } catch (final InterruptedException | ExecutionException e) { log.error("Unable to stop the connection to the Modbus server", e); } + } else { + log.debug("Stop called for Modbus adapter {} but adapter is already stopped or stopping", adapterId); + output.stoppedSuccessfully(); } } diff --git a/modules/hivemq-edge-module-opcua/src/main/java/com/hivemq/edge/adapters/opcua/OpcUaClientConnection.java b/modules/hivemq-edge-module-opcua/src/main/java/com/hivemq/edge/adapters/opcua/OpcUaClientConnection.java index 6984a6ac93..73f2c5f947 100644 --- a/modules/hivemq-edge-module-opcua/src/main/java/com/hivemq/edge/adapters/opcua/OpcUaClientConnection.java +++ b/modules/hivemq-edge-module-opcua/src/main/java/com/hivemq/edge/adapters/opcua/OpcUaClientConnection.java @@ -48,7 +48,6 @@ import java.util.concurrent.atomic.AtomicReference; import static com.hivemq.edge.adapters.opcua.Constants.PROTOCOL_ID_OPCUA; -import static org.eclipse.milo.opcua.stack.core.types.builtin.unsigned.Unsigned.uint; public class OpcUaClientConnection { private static final @NotNull Logger log = LoggerFactory.getLogger(OpcUaClientConnection.class); diff --git a/modules/hivemq-edge-module-opcua/src/main/java/com/hivemq/edge/adapters/opcua/OpcUaProtocolAdapter.java b/modules/hivemq-edge-module-opcua/src/main/java/com/hivemq/edge/adapters/opcua/OpcUaProtocolAdapter.java index f0f5bc6611..f46dd196a5 100644 --- a/modules/hivemq-edge-module-opcua/src/main/java/com/hivemq/edge/adapters/opcua/OpcUaProtocolAdapter.java +++ b/modules/hivemq-edge-module-opcua/src/main/java/com/hivemq/edge/adapters/opcua/OpcUaProtocolAdapter.java @@ -54,7 +54,6 @@ import java.util.List; import java.util.Map; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.stream.Collectors; @@ -114,27 +113,33 @@ public synchronized void start( } final OpcUaClientConnection conn; - if (opcUaClientConnection.compareAndSet(null, conn = new OpcUaClientConnection(adapterId, - tagList, - protocolAdapterState, - input.moduleServices().protocolAdapterTagStreamingService(), - dataPointFactory, - input.moduleServices().eventService(), - protocolAdapterMetricsService, - config, - lastSubscriptionId))) { + if (opcUaClientConnection.compareAndSet(null, + conn = new OpcUaClientConnection(adapterId, + tagList, + protocolAdapterState, + input.moduleServices().protocolAdapterTagStreamingService(), + dataPointFactory, + input.moduleServices().eventService(), + protocolAdapterMetricsService, + config, + lastSubscriptionId))) { protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.DISCONNECTED); - CompletableFuture.supplyAsync(() -> conn.start(parsedConfig)).whenComplete((success, throwable) -> { - if (!success || throwable != null) { - this.opcUaClientConnection.set(null); + try { + if (conn.start(parsedConfig)) { + log.info("Successfully started OPC UA protocol adapter {}", adapterId); + output.startedSuccessfully(); + } else { + opcUaClientConnection.set(null); protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.ERROR); - log.error("Failed to start OPC UA client", throwable); + output.failStart(new IllegalStateException("check the logs"), "Failed to start OPC UA client"); } - }); - - log.info("Successfully started OPC UA protocol adapter {}", adapterId); - output.startedSuccessfully(); + } catch (final Exception e) { + opcUaClientConnection.set(null); + protocolAdapterState.setConnectionStatus(ProtocolAdapterState.ConnectionStatus.ERROR); + log.error("Failed to start OPC UA client", e); + output.failStart(e.getCause(), "Failed to start OPC UA client"); + } } else { log.warn("Tried starting already started OPC UA protocol adapter {}", adapterId); } @@ -159,10 +164,12 @@ public void destroy() { log.info("Destroying OPC UA protocol adapter {}", adapterId); final OpcUaClientConnection conn = opcUaClientConnection.getAndSet(null); if (conn != null) { - CompletableFuture.runAsync(() -> { + try { conn.destroy(); log.info("Destroyed OPC UA protocol adapter {}", adapterId); - }); + } catch (final Exception e) { + log.error("Error destroying OPC UA protocol adapter {}", adapterId, e); + } } else { log.info("Tried destroying stopped OPC UA protocol adapter {}", adapterId); }