From dd739e16c089f9f1d8ec78f5410ca273f98aca48 Mon Sep 17 00:00:00 2001 From: Mariusz Pacek Date: Thu, 12 Feb 2026 22:50:37 +0100 Subject: [PATCH] chore: harden lifecycle, health checks and CI - add PR trigger and Maven cache in CI - centralize shutdown cleanup in Main - make HealthMonitor failure counter thread-safe - fix UTF-8 response length handling in health endpoint - improve Influx username null-safety - reduce expected modbus read failures from ERROR to WARN --- .gitea/workflows/ci.yml | 10 +++++----- src/main/java/home/automation/Main.java | 17 +++++++++++++---- .../automation/health/HealthCheckServer.java | 10 ++++++---- .../home/automation/health/HealthMonitor.java | 13 +++++++------ .../automation/influxdb/InfluxConnection.java | 12 +++--------- .../automation/modbus/ModbusConnection.java | 7 ------- .../modbus/ModbusRtuTcpRepository.java | 3 ++- 7 files changed, 36 insertions(+), 36 deletions(-) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 93b749c..4c56bf7 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -4,6 +4,9 @@ on: push: branches: - "**" + pull_request: + branches: + - "**" jobs: backend: @@ -23,10 +26,7 @@ jobs: uses: actions/setup-java@v5 with: distribution: temurin - java-version: '21' - - name: Set up Maven - uses: stCarolas/setup-maven@v5 - with: - maven-version: 3.9.6 + java-version: "21" + cache: maven - name: Run Maven tests (parallel) run: mvn -B -T 1C test diff --git a/src/main/java/home/automation/Main.java b/src/main/java/home/automation/Main.java index b726bc3..93c3d52 100644 --- a/src/main/java/home/automation/Main.java +++ b/src/main/java/home/automation/Main.java @@ -41,12 +41,14 @@ public class Main { final var useMock = config.getUseMock() != null && config.getUseMock(); ModbusService service; + ModbusConnection modbusConnection = null; if (useMock) { service = new ModbusService(new home.automation.modbus.MockRepository(registryMappings)); } else { + modbusConnection = new ModbusConnection(jsonFileBasedConfiguration); service = new ModbusService( new ModbusRtuTcpRepository( - new ModbusConnection(jsonFileBasedConfiguration), + modbusConnection, registryMappings)); } @@ -57,12 +59,21 @@ public class Main { healthCheckServer.start(); // Shutdown hook for graceful cleanup + ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); + ScheduledExecutorService healthScheduler = Executors.newSingleThreadScheduledExecutor(); + Runtime.getRuntime().addShutdownHook(new Thread(() -> { LOGGER.info("Shutdown signal received, cleaning up resources..."); try { + scheduler.shutdownNow(); + healthScheduler.shutdownNow(); healthCheckServer.close(); + if (modbusConnection != null) { + modbusConnection.close(); + } + connection.close(); } catch (Exception e) { - LOGGER.error("Error closing health check server", e); + LOGGER.error("Error during shutdown cleanup", e); } })); @@ -70,7 +81,6 @@ public class Main { ? 300 : config.getInfluxSendIntervalSeconds(); - ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); scheduler.scheduleAtFixedRate( new InfluxMetricModbusSender(connection.getConnection(), pollingService), 0, @@ -78,7 +88,6 @@ public class Main { TimeUnit.SECONDS); // Monitor health and trigger restart if needed - ScheduledExecutorService healthScheduler = Executors.newSingleThreadScheduledExecutor(); healthScheduler.scheduleAtFixedRate(() -> { if (healthMonitor.hasExceededFailureThreshold()) { LOGGER.error("Health monitor threshold exceeded ({} consecutive failures). Initiating shutdown for restart.", diff --git a/src/main/java/home/automation/health/HealthCheckServer.java b/src/main/java/home/automation/health/HealthCheckServer.java index ccd68b8..56074c1 100644 --- a/src/main/java/home/automation/health/HealthCheckServer.java +++ b/src/main/java/home/automation/health/HealthCheckServer.java @@ -36,22 +36,24 @@ public class HealthCheckServer implements AutoCloseable { final HealthStatus status = healthMonitor.getHealthStatus(); final HealthResponse response = HealthResponse.fromHealthStatus(status); final String jsonResponse = response.toJson(); + final byte[] body = jsonResponse.getBytes(StandardCharsets.UTF_8); exchange.getResponseHeaders().set(CONTENT_TYPE, CONTENT_TYPE_JSON); - exchange.sendResponseHeaders(response.getHttpStatusCode(), jsonResponse.length()); + exchange.sendResponseHeaders(response.getHttpStatusCode(), body.length); try (OutputStream os = exchange.getResponseBody()) { - os.write(jsonResponse.getBytes(StandardCharsets.UTF_8)); + os.write(body); } } catch (Exception e) { LOGGER.error("Error handling health check", e); final HealthResponse errorResponse = HealthResponse.error("Internal error"); final String jsonResponse = errorResponse.toJson(); + final byte[] body = jsonResponse.getBytes(StandardCharsets.UTF_8); exchange.getResponseHeaders().set(CONTENT_TYPE, CONTENT_TYPE_JSON); - exchange.sendResponseHeaders(errorResponse.getHttpStatusCode(), jsonResponse.length()); + exchange.sendResponseHeaders(errorResponse.getHttpStatusCode(), body.length); try (OutputStream os = exchange.getResponseBody()) { - os.write(jsonResponse.getBytes(StandardCharsets.UTF_8)); + os.write(body); } } } diff --git a/src/main/java/home/automation/health/HealthMonitor.java b/src/main/java/home/automation/health/HealthMonitor.java index a3c68ee..a5d200e 100644 --- a/src/main/java/home/automation/health/HealthMonitor.java +++ b/src/main/java/home/automation/health/HealthMonitor.java @@ -5,11 +5,12 @@ import static home.automation.health.HealthState.*; import java.time.Instant; import java.util.Comparator; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; public class HealthMonitor { private final List indicators; - private int consecutiveFailures = 0; + private final AtomicInteger consecutiveFailures = new AtomicInteger(0); private final int failureThreshold; public HealthMonitor(List indicators) { @@ -33,9 +34,9 @@ public class HealthMonitor { // Track consecutive failures if (overallState != UP) { - consecutiveFailures++; + consecutiveFailures.incrementAndGet(); } else { - consecutiveFailures = 0; + consecutiveFailures.set(0); } final Instant lastSuccessfulRead = componentHealth.stream() @@ -53,18 +54,18 @@ public class HealthMonitor { .map(ComponentHealth::getComponent) .collect(Collectors.joining(", ")); message = String.format("Unhealthy components: %s (consecutive failures: %d/%d)", affected, - consecutiveFailures, failureThreshold); + consecutiveFailures.get(), failureThreshold); } return new HealthStatus(overallState, lastSuccessfulRead, message, componentHealth); } public boolean hasExceededFailureThreshold() { - return consecutiveFailures >= failureThreshold; + return consecutiveFailures.get() >= failureThreshold; } public int getConsecutiveFailures() { - return consecutiveFailures; + return consecutiveFailures.get(); } private static int statePriority(HealthState state) { diff --git a/src/main/java/home/automation/influxdb/InfluxConnection.java b/src/main/java/home/automation/influxdb/InfluxConnection.java index 136f98f..1fa1f6d 100644 --- a/src/main/java/home/automation/influxdb/InfluxConnection.java +++ b/src/main/java/home/automation/influxdb/InfluxConnection.java @@ -11,21 +11,15 @@ public class InfluxConnection implements AutoCloseable { private final InfluxDB connection; public InfluxConnection(InfluxDBConfiguration configuration) { - if (configuration.getUsername().isBlank()) { + final String username = configuration.getUsername(); + if (username == null || username.isBlank()) { this.connection = InfluxDBFactory.connect(configuration.getAddress()); } else { this.connection = InfluxDBFactory.connect( configuration.getAddress(), - configuration.getUsername(), + username, configuration.getPassword()); } - Runtime.getRuntime().addShutdownHook(new Thread(() -> { - try { - this.close(); - } catch (Exception e) { - LOGGER.error("Error closing InfluxDB connection during shutdown", e); - } - })); } public InfluxDB getConnection() { diff --git a/src/main/java/home/automation/modbus/ModbusConnection.java b/src/main/java/home/automation/modbus/ModbusConnection.java index a509a4a..bbecbc1 100644 --- a/src/main/java/home/automation/modbus/ModbusConnection.java +++ b/src/main/java/home/automation/modbus/ModbusConnection.java @@ -18,13 +18,6 @@ public class ModbusConnection implements Connection, AutoCloseable { public ModbusConnection(ConfigurationStrategy config) { this.config = config; - Runtime.getRuntime().addShutdownHook(new Thread(() -> { - try { - this.close(); - } catch (Exception e) { - LOGGER.warn("Error closing Modbus connection during shutdown: {}", e.getMessage()); - } - })); } private Pair getPortAndAddressFromConfig(ConfigurationStrategy config) { diff --git a/src/main/java/home/automation/modbus/ModbusRtuTcpRepository.java b/src/main/java/home/automation/modbus/ModbusRtuTcpRepository.java index 406d017..7f3427e 100644 --- a/src/main/java/home/automation/modbus/ModbusRtuTcpRepository.java +++ b/src/main/java/home/automation/modbus/ModbusRtuTcpRepository.java @@ -145,7 +145,8 @@ public class ModbusRtuTcpRepository implements Repository { LOGGER.info("Registry {}: calculated value = {}", register, value); return new Registry(register, mapping.getDesc(), value); } catch (Exception e) { - LOGGER.error("Cannot get registry {} due to: ", register, e); + LOGGER.warn("Cannot get registry {}: {}", register, e.getMessage()); + LOGGER.debug("Cannot get registry {} stack trace", register, e); throw new RuntimeException("Cannot get registry " + register, e); } }