From f206fff7bcdc4890548077f4542d4708c3b8e9f9 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Sat, 19 Sep 2026 12:08:26 +0300 Subject: [PATCH] [#217] Do not treat PACKAGES_REFRESHED as a started framework in HealthService HealthService.activate() scheduled its start-up readiness check 2 s after activation whenever FrameworkStatus.isReady() was true. FrameworkStatus was only fed by the listener registered in the same activate() call and counted PACKAGES_REFRESHED, STARTLEVEL_CHANGED, WARNING and INFO as "ready". On samples/workflow the Activiti FileInstall (noInitialDelay=true) installs and refreshes the .bar bundles while the framework is still raising its start level; when the resulting PACKAGES_REFRESHED event landed in the ~25 ms between addFrameworkListener() and the isReady() check, a 2 s timer fired before api-servlet was registered and logged "SEVERE: OpenIDM failure during startup" although the server became ready moments later. Decide "framework already started" by the system bundle state instead, treat that case like a late STARTED event (set frameworkStarted, checkState, then schedule the check after serviceStartMax rather than a hard-coded 2000 ms), and drop FrameworkStatus. Take the BundleContext from the ComponentContext so activate() can be exercised outside OSGi, and add tests for both paths. Fixes #217 --- .../openidm/info/impl/FrameworkStatus.java | 88 ------------- .../openidm/info/impl/HealthService.java | 33 +++-- .../openidm/info/impl/HealthServiceTest.java | 117 ++++++++++++++++++ 3 files changed, 131 insertions(+), 107 deletions(-) delete mode 100644 openidm-infoservice/src/main/java/org/forgerock/openidm/info/impl/FrameworkStatus.java create mode 100644 openidm-infoservice/src/test/java/org/forgerock/openidm/info/impl/HealthServiceTest.java diff --git a/openidm-infoservice/src/main/java/org/forgerock/openidm/info/impl/FrameworkStatus.java b/openidm-infoservice/src/main/java/org/forgerock/openidm/info/impl/FrameworkStatus.java deleted file mode 100644 index 4a69d7850a..0000000000 --- a/openidm-infoservice/src/main/java/org/forgerock/openidm/info/impl/FrameworkStatus.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * The contents of this file are subject to the terms of the Common Development and - * Distribution License (the License). You may not use this file except in compliance with the - * License. - * - * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the - * specific language governing permission and limitations under the License. - * - * When distributing Covered Software, include this CDDL Header Notice in each file and include - * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL - * Header, with the fields enclosed by brackets [] replaced by your own identifying - * information: "Portions copyright [year] [name of copyright owner]". - * - * Copyright 2015 ForgeRock AS. - */ -package org.forgerock.openidm.info.impl; - -import org.osgi.framework.FrameworkEvent; - -/** - * A Framework Status class for storing the framework status indicated - * by the last {@link org.osgi.framework.FrameworkEvent} event published. - */ -public class FrameworkStatus { - - /** - * A framework status instance holder - */ - private static class InstanceHolder { - private static final FrameworkStatus instance = new FrameworkStatus(-1); - } - - /** - * An integer representing the framework status. - * See {@link org.osgi.framework.FrameworkEvent} for - */ - private int frameworkStatus; - - /** - * Constructor - * - * @param frameworkStatus An integer representing the framework status - */ - private FrameworkStatus(int frameworkStatus) { - this.frameworkStatus = frameworkStatus; - } - - /** - * Gets an instance of the framework status. - * - * @return a FrameworkStatus instance - */ - public static synchronized FrameworkStatus getInstance() { - return InstanceHolder.instance; - } - - /** - * Returns the current framework status. - * - * @return an integer representing the framework status. - */ - public int getFrameworkStatus() { - return this.frameworkStatus; - } - - /** - * Sets the current framework status. - * - * @param frameworkStatus an integer representing the framework status. - */ - public void setFrameworkStatus(int frameworkStatus) { - this.frameworkStatus = frameworkStatus; - } - - /** - * Returns true if the framework has been started and is any of the following - * states indicating it is ready: STARTED, PACKAGES_REFRESHED, WARNING, INFO. - * - * @return a boolean indicating if the framework is ready. - */ - public boolean isReady() { - return frameworkStatus == FrameworkEvent.STARTED - || frameworkStatus == FrameworkEvent.PACKAGES_REFRESHED - || frameworkStatus == FrameworkEvent.STARTLEVEL_CHANGED - || frameworkStatus == FrameworkEvent.WARNING - || frameworkStatus == FrameworkEvent.INFO; - } -} diff --git a/openidm-infoservice/src/main/java/org/forgerock/openidm/info/impl/HealthService.java b/openidm-infoservice/src/main/java/org/forgerock/openidm/info/impl/HealthService.java index f8b3a1723f..40b0fe23bf 100644 --- a/openidm-infoservice/src/main/java/org/forgerock/openidm/info/impl/HealthService.java +++ b/openidm-infoservice/src/main/java/org/forgerock/openidm/info/impl/HealthService.java @@ -12,7 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2012-2016 ForgeRock AS. - * Portions Copyrighted 2024 3A Systems LLC. + * Portions Copyrighted 2024-2026 3A Systems LLC. */ package org.forgerock.openidm.info.impl; @@ -56,7 +56,6 @@ import org.osgi.framework.Constants; import org.osgi.framework.FrameworkEvent; import org.osgi.framework.FrameworkListener; -import org.osgi.framework.FrameworkUtil; import org.osgi.framework.ServiceEvent; import org.osgi.framework.ServiceListener; import org.osgi.framework.ServiceReference; @@ -153,11 +152,6 @@ enum AppState { */ private volatile boolean clusterEnabled = true; - /** - * A framework status instance used to store the latest framework event status. - */ - private FrameworkStatus frameworkStatus = null; - /** * The current state of OpenIDM */ @@ -300,11 +294,8 @@ protected void activate(final ComponentContext context) { requiredServices.addAll(Arrays.asList(defaultRequiredServices)); applyPropertyConfig(); - // Get the framework status service instance - frameworkStatus = FrameworkStatus.getInstance(); - // Set up tracker - BundleContext ctx = FrameworkUtil.getBundle(HealthService.class).getBundleContext(); + BundleContext ctx = context.getBundleContext(); tracker = initServiceTracker(ctx); // Handle framework changes @@ -313,9 +304,6 @@ protected void activate(final ComponentContext context) { public void frameworkEvent(FrameworkEvent event) { final int eventType = event.getType(); logger.debug("Handle framework event {} {}", eventType, event.toString()); - - // Store the framework event type as the framework status - frameworkStatus.setFrameworkStatus(eventType); if (eventType == FrameworkEvent.STARTED) { logger.debug("OSGi framework started event."); @@ -392,12 +380,19 @@ public void bundleChanged(BundleEvent event) { router.addRoute(uriTemplate("recon"), new ReconInfoResourceProvider()); router.addRoute(uriTemplate("jdbc"), new DatabaseInfoResourceProvider()); - // Check if the framework has already started. If so, schedule the start up - // thread that checks the state of OpenIDM. - if (frameworkStatus.isReady()) { - scheduleCheckStartup(2000); + // The STARTED event only reaches listeners registered before it is fired. If the framework + // is already active (component re-activated after start-up), treat it as started now and give + // the required services the same grace period as the STARTED path. Framework events other + // than STARTED (e.g. PACKAGES_REFRESHED from a bundle refresh while the start level is still + // being raised) must not be taken as an indication that the framework has started. + if (ctx.getBundle(0).getState() == Bundle.ACTIVE) { + frameworkStarted = true; + checkState(); + if (!stateDetail.state.equals(AppState.ACTIVE_READY)) { + scheduleCheckStartup(serviceStartMax); + } } - + logger.info("OpenIDM Health Service component is activated."); } diff --git a/openidm-infoservice/src/test/java/org/forgerock/openidm/info/impl/HealthServiceTest.java b/openidm-infoservice/src/test/java/org/forgerock/openidm/info/impl/HealthServiceTest.java new file mode 100644 index 0000000000..b1a05510a5 --- /dev/null +++ b/openidm-infoservice/src/test/java/org/forgerock/openidm/info/impl/HealthServiceTest.java @@ -0,0 +1,117 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.forgerock.openidm.info.impl; + +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; + +import org.forgerock.openidm.core.IdentityServer; +import org.forgerock.openidm.core.PropertyAccessor; +import org.osgi.framework.Bundle; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkEvent; +import org.osgi.framework.FrameworkListener; +import org.osgi.framework.FrameworkUtil; +import org.osgi.service.component.ComponentContext; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +/** + * Tests for the start-up readiness check of {@link HealthService}. + */ +public class HealthServiceTest { + + private static final String SERVICE_START_MAX_PROPERTY = "openidm.healthservice.servicestartmax"; + + private Bundle systemBundle; + private BundleContext bundleContext; + private ComponentContext componentContext; + private HealthService healthService; + + @BeforeClass + public void initIdentityServer() { + try { + IdentityServer.initInstance((PropertyAccessor) null); + } catch (IllegalStateException e) { + // already initialised by another test in this JVM + } + } + + @BeforeMethod + public void setUp() throws Exception { + systemBundle = mock(Bundle.class); + bundleContext = mock(BundleContext.class); + when(bundleContext.getBundle(0)).thenReturn(systemBundle); + when(bundleContext.getBundles()).thenReturn(new Bundle[0]); + when(bundleContext.createFilter(anyString())).thenAnswer( + invocation -> FrameworkUtil.createFilter((String) invocation.getArguments()[0])); + componentContext = mock(ComponentContext.class); + when(componentContext.getBundleContext()).thenReturn(bundleContext); + healthService = new HealthService(); + } + + @AfterMethod + public void tearDown() { + System.clearProperty(SERVICE_START_MAX_PROPERTY); + healthService.deactivate(componentContext); + } + + /** + * On samples/workflow the Activiti FileInstall refreshes the freshly installed .bar bundles while + * the framework is still raising its start level. The resulting PACKAGES_REFRESHED event must not + * be mistaken for a started framework, otherwise a premature start-up check reports a failure. + */ + @Test + public void packagesRefreshedDuringActivationDoesNotTriggerStartupCheck() throws Exception { + when(systemBundle.getState()).thenReturn(Bundle.STARTING); + // deliver the event right after the listener is registered, i.e. inside activate() + doAnswer(invocation -> { + FrameworkListener listener = (FrameworkListener) invocation.getArguments()[0]; + listener.frameworkEvent(new FrameworkEvent(FrameworkEvent.PACKAGES_REFRESHED, systemBundle, null)); + return null; + }).when(bundleContext).addFrameworkListener(any(FrameworkListener.class)); + + healthService.activate(componentContext); + Thread.sleep(2500); + + assertEquals(state(), "STARTING"); + } + + /** + * When the framework is already active the STARTED event will never arrive, so the start-up check + * has to be scheduled from activate() — after the configured grace period, not a hard-coded one. + */ + @Test + public void frameworkAlreadyActiveSchedulesStartupCheckAfterServiceStartMax() throws Exception { + System.setProperty(SERVICE_START_MAX_PROPERTY, "200"); + when(systemBundle.getState()).thenReturn(Bundle.ACTIVE); + + healthService.activate(componentContext); + Thread.sleep(1000); + + assertEquals(state(), "ACTIVE_NOT_READY"); + } + + private String state() { + return healthService.getHealthInfo().get("state").asString(); + } +}