From fdc1eb47ced95a11b5b0f7885c5085a1579d8046 Mon Sep 17 00:00:00 2001 From: Jose Castro Date: Thu, 2 Jul 2026 12:35:23 -0600 Subject: [PATCH 1/7] Default null country/countryCode to blank in LanguageViewStrategy.mapLanguage Guava's ImmutableMap.Builder.put() throws NullPointerException on a null value. Language-only locales (no country code) legitimately have a null country/countryCode, which crashed any transform path that maps a contentlet's Language (new Edit Content save, History tab, GraphQL). Adds LanguageViewStrategyTest covering the null-country regression, the wrapAsMap=true path, the transform() entry point, and a non-regression check for locales that do have a country. Fixes #36106 --- .../strategy/LanguageViewStrategy.java | 44 +++++-- .../strategy/LanguageViewStrategyTest.java | 110 ++++++++++++++++++ 2 files changed, 142 insertions(+), 12 deletions(-) create mode 100644 dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategyTest.java diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategy.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategy.java index a52bf3c70b7..565e2b6bcf8 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategy.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategy.java @@ -1,9 +1,7 @@ package com.dotmarketing.portlets.contentlet.transform.strategy; -import static com.dotmarketing.portlets.contentlet.transform.strategy.TransformOptions.AVOID_MAP_SUFFIX_FOR_VIEWS; -import static com.liferay.portal.language.LanguageUtil.getLiteralLocale; - import com.dotcms.api.APIProvider; +import com.dotcms.util.CollectionsUtils; import com.dotmarketing.portlets.contentlet.model.Contentlet; import com.dotmarketing.portlets.languagesmanager.model.Language; import com.dotmarketing.util.UtilMethods; @@ -11,12 +9,26 @@ import com.google.common.collect.ImmutableMap.Builder; import com.liferay.portal.model.User; import com.liferay.util.StringPool; + import java.util.Collections; import java.util.Map; import java.util.Set; +import static com.dotmarketing.portlets.contentlet.transform.strategy.TransformOptions.AVOID_MAP_SUFFIX_FOR_VIEWS; +import static com.liferay.portal.language.LanguageUtil.getLiteralLocale; +import static com.liferay.util.StringPool.BLANK; + /** - * Language view transformer strategy + * Language view transformer strategy. + *

+ * Maps a Contentlet's {@link Language} into the response Map used by the various content + * transformer presets (e.g. {@code webAssetOptions}, {@code dotAssetOptions}) as well as + * {@link HistoryViewStrategy}. Locales are allowed to be language-only (no country code), so + * the {@code country}/{@code countryCode} entries default to an empty string rather than + * {@code null} to avoid tripping Guava's {@link Builder}, which rejects null values. + * + * @author Fabrizzio Araya + * @since Jun 11th, 2020 */ public class LanguageViewStrategy extends AbstractTransformStrategy{ @@ -50,10 +62,18 @@ public static Map mapLanguage(final Language language, final boo } /** - * Lang functions now relocated here. - * @param language - * @param wrapAsMap - * @return + * Maps a {@link Language} into a Map of view-friendly properties (id, language/country names + * and codes, ISO code, flag). Since a Language may be defined with only a language code and + * no country (e.g. {@code es}), {@code country} and {@code countryCode} default to an empty + * string when unset rather than {@code null} — a null value would blow up the underlying + * {@link Builder}, which does not accept null entries. + * + * @param language the Language to map; its {@code country}/{@code countryCode} may be null + * @param wrapAsMap if {@code true}, wraps the result under a {@code language}/{@code languageMap} + * key (see {@code AVOID_MAP_SUFFIX_FOR_VIEWS}); if {@code false}, returns the + * properties unwrapped + * @param options the active {@link TransformOptions}, used to decide the wrapper key suffix + * @return a Map of the language's view properties */ public static Map mapLanguage(final Language language, final boolean wrapAsMap, final Set options) { @@ -64,8 +84,8 @@ public static Map mapLanguage(final Language language, final boo .put("languageId", language.getId()) .put("language", language.getLanguage()) .put("languageCode", language.getLanguageCode()) - .put("country", language.getCountry()) - .put("countryCode", language.getCountryCode()) + .put("country", CollectionsUtils.orElseGet(language.getCountry(), BLANK)) + .put("countryCode", CollectionsUtils.orElseGet(language.getCountryCode(), BLANK)) .put("languageFlag", getLiteralLocale(language.getLanguageCode(), language.getCountryCode())); final String iso = UtilMethods.isSet(language.getCountryCode()) @@ -75,9 +95,9 @@ public static Map mapLanguage(final Language language, final boo if(wrapAsMap){ builder.put("id", language.getId()); - final String sufix = options.contains(AVOID_MAP_SUFFIX_FOR_VIEWS) + final String suffix = options.contains(AVOID_MAP_SUFFIX_FOR_VIEWS) ? "" : "Map"; - return ImmutableMap.of("language" + sufix, builder.build()); + return ImmutableMap.of("language" + suffix, builder.build()); } return builder.build(); } diff --git a/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategyTest.java b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategyTest.java new file mode 100644 index 00000000000..65ab37000f0 --- /dev/null +++ b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategyTest.java @@ -0,0 +1,110 @@ +package com.dotmarketing.portlets.contentlet.transform.strategy; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import com.dotcms.api.APIProvider; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.portlets.languagesmanager.business.LanguageAPI; +import com.dotmarketing.portlets.languagesmanager.model.Language; +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import org.junit.Test; +import org.mockito.Mockito; + +/** + * Unit tests for {@link LanguageViewStrategy}, covering how it maps a {@link Language} into a + * view Map — including the language-only locale (no country code) regression from + * Issue #36106. + * + * @author Jose Castro + * @since Jul 2nd, 2026 + */ +public class LanguageViewStrategyTest { + + /** + * Regression test for Issue #36106. + *

+ * dotCMS allows a {@code Language} to be defined with only a language code and no country + * (e.g. {@code es}), leaving {@code country}/{@code countryCode} {@code null}. {@code mapLanguage} + * builds its result with a Guava {@code ImmutableMap.Builder}, which throws + * {@code NullPointerException("null value in entry: country=null")} when a null value is put. + * Language-only locales must map to an empty string instead of null. + */ + @Test + public void testMapLanguage_whenCountryIsNull_doesNotThrowAndDefaultsToBlank() { + final Language language = new Language(1L, "es", null, "Spanish", null); + + final Map result = LanguageViewStrategy.mapLanguage(language, false); + + assertEquals("", result.get("country")); + assertEquals("", result.get("countryCode")); + assertEquals("es", result.get("isoCode")); + } + + /** + * Same regression as above, but through the {@code wrapAsMap=true} path, which is what + * {@link LanguageViewStrategy#transform} actually uses in production. + */ + @Test + public void testMapLanguage_whenWrapAsMapAndCountryIsNull_wrapsBlankCountryUnderLanguageMapKey() { + final Language language = new Language(2L, "fr", null, "French", null); + + final Map result = LanguageViewStrategy.mapLanguage(language, true); + + assertTrue(result.containsKey("languageMap")); + @SuppressWarnings("unchecked") + final Map inner = (Map) result.get("languageMap"); + assertEquals("", inner.get("country")); + assertEquals("", inner.get("countryCode")); + assertEquals(2L, inner.get("id")); + } + + /** + * Non-regression check: a locale with both language and country must keep mapping those + * values through unchanged (no fallback to blank). + */ + @Test + public void testMapLanguage_whenCountryIsSet_mapsCountryAndIsoCode() { + final Language language = new Language(3L, "es", "CR", "Spanish", "Costa Rica"); + + final Map result = LanguageViewStrategy.mapLanguage(language, false); + + assertEquals("Costa Rica", result.get("country")); + assertEquals("CR", result.get("countryCode")); + assertEquals("es-cr", result.get("isoCode")); + } + + /** + * Exercises the actual {@code transform} entry point used by {@code DefaultTransformStrategy} + * and {@code HistoryViewStrategy} callers, confirming a language-only locale doesn't throw + * when resolved through {@link APIProvider#languageAPI}. + */ + @Test + public void testTransform_whenCountryIsNull_doesNotThrowAndPutsBlankCountryIntoMap() + throws Exception { + final Language language = new Language(4L, "es", null, "Spanish", null); + final LanguageAPI languageAPI = Mockito.mock(LanguageAPI.class); + Mockito.when(languageAPI.getLanguage(4L)).thenReturn(language); + + final APIProvider toolBox = Mockito.mock(APIProvider.class); + final Field languageApiField = APIProvider.class.getDeclaredField("languageAPI"); + languageApiField.setAccessible(true); + languageApiField.set(toolBox, languageAPI); + + final LanguageViewStrategy strategy = new LanguageViewStrategy(toolBox); + + final Contentlet contentlet = Mockito.mock(Contentlet.class); + Mockito.when(contentlet.getLanguageId()).thenReturn(4L); + + final Map outputMap = new HashMap<>(); + strategy.transform(contentlet, outputMap, Collections.emptySet(), null); + + @SuppressWarnings("unchecked") + final Map languageMap = (Map) outputMap.get("languageMap"); + assertEquals("", languageMap.get("country")); + assertEquals("", languageMap.get("countryCode")); + } +} From 0156e31f1c0e30dda3b081cad1f2c4d637faea4f Mon Sep 17 00:00:00 2001 From: Jose Castro Date: Thu, 2 Jul 2026 12:35:52 -0600 Subject: [PATCH 2/7] Defer Velocity health check probe until dotCMS startup completes Not related to the #36106 language locale fix in this PR. This addresses a separate, unrelated startup-time exception: VelocityUtil.getEngine() triggers a lazy Velocity engine init that touches the DB/company context, which on a fresh install can fire before Task00001LoadSchema has run, throwing a "No Company!" exception during boot. Deferring the probe until InitServlet sets dotcms.started.up=true avoids that noisy exception. --- .../com/dotcms/health/checks/cdi/VelocityHealthCheck.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dotCMS/src/main/java/com/dotcms/health/checks/cdi/VelocityHealthCheck.java b/dotCMS/src/main/java/com/dotcms/health/checks/cdi/VelocityHealthCheck.java index 1a2ba0533c8..3ca316438ef 100644 --- a/dotCMS/src/main/java/com/dotcms/health/checks/cdi/VelocityHealthCheck.java +++ b/dotCMS/src/main/java/com/dotcms/health/checks/cdi/VelocityHealthCheck.java @@ -78,6 +78,14 @@ protected CheckResult performCheck() throws Exception { "Velocity health check skipped during shutdown"); } + // VelocityUtil.getEngine() triggers a lazy init that touches the DB/company context. + // On a fresh install this fires before Task00001LoadSchema runs, throwing "No Company!". + // Defer until InitServlet has set dotcms.started.up=true. + if (!"true".equals(System.getProperty("dotcms.started.up"))) { + return new CheckResult(false, 0L, + "Velocity probe deferred: dotCMS startup not yet complete"); + } + return measureExecution(() -> { final VelocityEngine engine = VelocityUtil.getEngine(); final StringWriter writer = new StringWriter(); From 526c73b8ef6853022c0f324f3778e188962ca14d Mon Sep 17 00:00:00 2001 From: Jose Castro Date: Thu, 2 Jul 2026 14:58:22 -0600 Subject: [PATCH 3/7] Fix NPE building visitor Locale for language-only locales VisitorAPIImpl.createVisitor() built a Locale via new Locale(language, country), which throws NullPointerException when country is null. Language-only locales (no country code) hit this on every page render, since VisitorAPIImpl.getVisitor() runs on every request. Language.asLocale() already null-guards this exact case (falls back to the 1-arg Locale constructor), so use it instead of constructing the Locale manually. Same underlying null-country data shape as the LanguageViewStrategy fix in this PR, different code path (page rendering / visitor creation vs. content transform). --- .../main/java/com/dotcms/visitor/business/VisitorAPIImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotCMS/src/main/java/com/dotcms/visitor/business/VisitorAPIImpl.java b/dotCMS/src/main/java/com/dotcms/visitor/business/VisitorAPIImpl.java index b0bf7aa5fe2..f73340dcac0 100644 --- a/dotCMS/src/main/java/com/dotcms/visitor/business/VisitorAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/visitor/business/VisitorAPIImpl.java @@ -147,7 +147,7 @@ private Visitor createVisitor(final HttpServletRequest request) { } final Language selectedLanguage = languageWebAPI.getLanguage(request); - final Locale locale = new Locale(selectedLanguage.getLanguageCode(), selectedLanguage.getCountryCode()); + final Locale locale = selectedLanguage.asLocale(); final UserAgent userAgent = UserAgent.parseUserAgentString(request.getHeader("User-Agent")); final UUID dmid = lookupDMID(request); final boolean isNewVisitor = isNewVisitor(request); From 2e8e989d7892128e2b9ece0383f8f191cde069a5 Mon Sep 17 00:00:00 2001 From: Jose Castro Date: Thu, 2 Jul 2026 15:50:08 -0600 Subject: [PATCH 4/7] Fix NPE building Push Publishing queue map for language-only locales PublishQueueElementTransformer.getMapForLanguage() built a java.util.Map.of(...) directly from language.getCountryCode(), which is null for a language-only locale (e.g. "es", no country). Map.of() throws NullPointerException on any null value, same failure mode as Guava's ImmutableMap.Builder fixed elsewhere in this PR. This crashed the Push Publishing queue view for any queued Language asset in a language-only locale. Defaults countryCode to blank before building the map, and adds PublishQueueElementTransformerTest covering the null-country regression and a non-regression check for locales with a country. --- .../PublishQueueElementTransformer.java | 25 +++--- .../PublishQueueElementTransformerTest.java | 76 +++++++++++++++++++ 2 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 dotCMS/src/test/java/com/dotcms/publisher/business/PublishQueueElementTransformerTest.java diff --git a/dotCMS/src/main/java/com/dotcms/publisher/business/PublishQueueElementTransformer.java b/dotCMS/src/main/java/com/dotcms/publisher/business/PublishQueueElementTransformer.java index 23b2495f7ad..13687e00afc 100644 --- a/dotCMS/src/main/java/com/dotcms/publisher/business/PublishQueueElementTransformer.java +++ b/dotCMS/src/main/java/com/dotcms/publisher/business/PublishQueueElementTransformer.java @@ -126,15 +126,22 @@ private static Map getMapForLanguage(final String id) { final Language language = APILocator.getLanguageAPI().getLanguage(id); - return new HashMap<>(UtilMethods.isSet(language) ? - Map.of( - TITLE_KEY, String.format( "%s(%s)", language.getLanguage(), language.getCountryCode()), - LANGUAGE_CODE_KEY, language.getLanguageCode(), - COUNTRY_CODE_KEY, language.getCountryCode(), - CONTENT_TYPE_NAME_KEY, CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_CAMEL, - PusheableAsset.LANGUAGE.getType()) - ) : - Map.of(TITLE_KEY, id)); + if (!UtilMethods.isSet(language)) { + return new HashMap<>(Map.of(TITLE_KEY, id)); + } + + // language-only locales (e.g. "es", no country) have a null countryCode; Map.of() + // throws NullPointerException on a null value, so default it to blank. + final String countryCode = UtilMethods.isSet(language.getCountryCode()) + ? language.getCountryCode() : StringPool.BLANK; + + return new HashMap<>(Map.of( + TITLE_KEY, String.format("%s(%s)", language.getLanguage(), countryCode), + LANGUAGE_CODE_KEY, language.getLanguageCode(), + COUNTRY_CODE_KEY, countryCode, + CONTENT_TYPE_NAME_KEY, CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_CAMEL, + PusheableAsset.LANGUAGE.getType()) + )); } private static Map getMapForCategory(){ diff --git a/dotCMS/src/test/java/com/dotcms/publisher/business/PublishQueueElementTransformerTest.java b/dotCMS/src/test/java/com/dotcms/publisher/business/PublishQueueElementTransformerTest.java new file mode 100644 index 00000000000..555edc52124 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/publisher/business/PublishQueueElementTransformerTest.java @@ -0,0 +1,76 @@ +package com.dotcms.publisher.business; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mockStatic; + +import com.dotcms.publisher.util.PusheableAsset; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.portlets.languagesmanager.business.LanguageAPI; +import com.dotmarketing.portlets.languagesmanager.model.Language; +import java.util.Map; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +/** + * Unit tests for {@link PublishQueueElementTransformer}. + * + * @author Jose Castro + * @since Jul 2, 2026 + */ +public class PublishQueueElementTransformerTest { + + private MockedStatic apiLocatorMock; + private LanguageAPI languageAPI; + + @Before + public void setUp() { + languageAPI = Mockito.mock(LanguageAPI.class); + apiLocatorMock = mockStatic(APILocator.class); + apiLocatorMock.when(APILocator::getLanguageAPI).thenReturn(languageAPI); + } + + @After + public void tearDown() { + apiLocatorMock.close(); + } + + /** + * Regression test for the Push Publishing queue view crashing on language-only locales. + *

+ * {@code getMapForLanguage} used to build a {@code java.util.Map.of(...)} directly from + * {@code language.getCountryCode()}, which is legitimately null for a language-only locale + * (e.g. {@code es}, no country). {@code Map.of()} throws {@code NullPointerException} on any + * null value, the same failure mode as Guava's {@code ImmutableMap.Builder}. + */ + @Test + public void testGetMap_whenLanguageIsLanguageOnly_doesNotThrowAndDefaultsCountryToBlank() { + final Language language = new Language(1L, "es", null, "Spanish", null); + Mockito.when(languageAPI.getLanguage("1")).thenReturn(language); + + final PublishQueueElementTransformer transformer = new PublishQueueElementTransformer(); + final Map result = transformer.getMap("1", PusheableAsset.LANGUAGE.getType()); + + assertEquals("", result.get(PublishQueueElementTransformer.COUNTRY_CODE_KEY)); + assertEquals("es", result.get(PublishQueueElementTransformer.LANGUAGE_CODE_KEY)); + assertEquals("Spanish()", result.get(PublishQueueElementTransformer.TITLE_KEY)); + } + + /** + * Non-regression check: a locale with both language and country must keep mapping the + * country code through unchanged. + */ + @Test + public void testGetMap_whenLanguageHasCountry_mapsCountryCode() { + final Language language = new Language(2L, "es", "CR", "Spanish", "Costa Rica"); + Mockito.when(languageAPI.getLanguage("2")).thenReturn(language); + + final PublishQueueElementTransformer transformer = new PublishQueueElementTransformer(); + final Map result = transformer.getMap("2", PusheableAsset.LANGUAGE.getType()); + + assertEquals("CR", result.get(PublishQueueElementTransformer.COUNTRY_CODE_KEY)); + assertEquals("Spanish(CR)", result.get(PublishQueueElementTransformer.TITLE_KEY)); + } +} From 6d48400c268ad0aa63576d2e03e0d6048a93b79e Mon Sep 17 00:00:00 2001 From: Jose Castro Date: Fri, 3 Jul 2026 07:36:14 -0600 Subject: [PATCH 5/7] Assert isoCode in LanguageViewStrategyTest null-country cases Addresses dotBot review feedback on PR #36405: the wrapAsMap=true and transform() test cases checked country/countryCode but not isoCode, leaving a gap in coverage for the ISO code formatting when country is absent. --- .../contentlet/transform/strategy/LanguageViewStrategyTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategyTest.java b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategyTest.java index 65ab37000f0..b4c0d0abb05 100644 --- a/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategyTest.java +++ b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategyTest.java @@ -59,6 +59,7 @@ public void testMapLanguage_whenWrapAsMapAndCountryIsNull_wrapsBlankCountryUnder final Map inner = (Map) result.get("languageMap"); assertEquals("", inner.get("country")); assertEquals("", inner.get("countryCode")); + assertEquals("fr", inner.get("isoCode")); assertEquals(2L, inner.get("id")); } @@ -106,5 +107,6 @@ public void testTransform_whenCountryIsNull_doesNotThrowAndPutsBlankCountryIntoM final Map languageMap = (Map) outputMap.get("languageMap"); assertEquals("", languageMap.get("country")); assertEquals("", languageMap.get("countryCode")); + assertEquals("es", languageMap.get("isoCode")); } } From 56bc6fd2e1ce3731f2cef4f77a5ad05704eb9a01 Mon Sep 17 00:00:00 2001 From: Jose Castro Date: Fri, 3 Jul 2026 08:33:34 -0600 Subject: [PATCH 6/7] Fix VelocityHealthCheckTest broken by the startup-deferral guard VelocityHealthCheck.performCheck() now defers until dotcms.started.up=true before probing the engine. The test never set that property, so every test short-circuited on the deferred branch before reaching the mocked engine: returnsUpWhenRenderMarksResolves failed outright (expected UP, got DOWN), and the other two passed for the wrong reason, no longer exercising the literal-marker/engine-exception logic they're meant to cover. Sets dotcms.started.up=true in setUp() (restored in tearDown()) so the probe logic actually runs, and adds returnsDownWhenStartupNotComplete to cover the deferred branch itself, which had no test before. --- .../checks/cdi/VelocityHealthCheckTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/dotCMS/src/test/java/com/dotcms/health/checks/cdi/VelocityHealthCheckTest.java b/dotCMS/src/test/java/com/dotcms/health/checks/cdi/VelocityHealthCheckTest.java index 4fd6f3aa48e..5ce92694f96 100644 --- a/dotCMS/src/test/java/com/dotcms/health/checks/cdi/VelocityHealthCheckTest.java +++ b/dotCMS/src/test/java/com/dotcms/health/checks/cdi/VelocityHealthCheckTest.java @@ -27,16 +27,29 @@ */ public class VelocityHealthCheckTest { + private static final String STARTED_UP_PROPERTY = "dotcms.started.up"; + private MockedStatic velocityUtilMock; + private String previousStartedUpProperty; @Before public void setUp() { + // The probe defers until dotCMS startup completes, which InitServlet signals via this + // property in production. Set it here so the probe logic under test actually runs + // instead of short-circuiting on every test. + previousStartedUpProperty = System.getProperty(STARTED_UP_PROPERTY); + System.setProperty(STARTED_UP_PROPERTY, "true"); velocityUtilMock = mockStatic(VelocityUtil.class); } @After public void tearDown() { velocityUtilMock.close(); + if (previousStartedUpProperty == null) { + System.clearProperty(STARTED_UP_PROPERTY); + } else { + System.setProperty(STARTED_UP_PROPERTY, previousStartedUpProperty); + } } @Test @@ -48,6 +61,18 @@ public void returnsUpWhenRenderMarksResolves() { assertEquals(HealthStatus.UP, result.status()); } + @Test + public void returnsDownWhenStartupNotComplete() { + // Even a healthy-looking engine shouldn't matter: the probe must defer before touching + // VelocityUtil.getEngine() at all until InitServlet marks startup complete. + System.clearProperty(STARTED_UP_PROPERTY); + stubEngineToWrite(""); + + final HealthCheckResult result = new VelocityHealthCheck().check(); + + assertEquals(HealthStatus.DOWN, result.status()); + } + @Test public void returnsDownWhenRenderMarksRendersLiterally() { // When the global macro library failed to load, Velocity renders the From a1299d2a669833aa2c8290f96087276ddb454491 Mon Sep 17 00:00:00 2001 From: Jose Castro Date: Fri, 3 Jul 2026 08:51:27 -0600 Subject: [PATCH 7/7] Pushing fixes for unit test --- .../health/checks/cdi/VelocityHealthCheck.java | 5 +++-- .../checks/cdi/VelocityHealthCheckTest.java | 17 +++++++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/health/checks/cdi/VelocityHealthCheck.java b/dotCMS/src/main/java/com/dotcms/health/checks/cdi/VelocityHealthCheck.java index 3ca316438ef..59880ea663c 100644 --- a/dotCMS/src/main/java/com/dotcms/health/checks/cdi/VelocityHealthCheck.java +++ b/dotCMS/src/main/java/com/dotcms/health/checks/cdi/VelocityHealthCheck.java @@ -3,13 +3,14 @@ import com.dotcms.health.util.HealthCheckBase; import com.dotcms.rendering.velocity.util.VelocityUtil; import com.dotmarketing.util.Logger; - import org.apache.velocity.VelocityContext; import org.apache.velocity.app.VelocityEngine; import javax.enterprise.context.ApplicationScoped; import java.io.StringWriter; +import static com.dotmarketing.util.WebKeys.DOTCMS_STARTED_UP; + /** * CDI-based health check that verifies the Velocity engine's global macro * library is loaded and resolvable. @@ -81,7 +82,7 @@ protected CheckResult performCheck() throws Exception { // VelocityUtil.getEngine() triggers a lazy init that touches the DB/company context. // On a fresh install this fires before Task00001LoadSchema runs, throwing "No Company!". // Defer until InitServlet has set dotcms.started.up=true. - if (!"true".equals(System.getProperty("dotcms.started.up"))) { + if (!"true".equals(System.getProperty(DOTCMS_STARTED_UP))) { return new CheckResult(false, 0L, "Velocity probe deferred: dotCMS startup not yet complete"); } diff --git a/dotCMS/src/test/java/com/dotcms/health/checks/cdi/VelocityHealthCheckTest.java b/dotCMS/src/test/java/com/dotcms/health/checks/cdi/VelocityHealthCheckTest.java index 5ce92694f96..6eac9631480 100644 --- a/dotCMS/src/test/java/com/dotcms/health/checks/cdi/VelocityHealthCheckTest.java +++ b/dotCMS/src/test/java/com/dotcms/health/checks/cdi/VelocityHealthCheckTest.java @@ -12,12 +12,11 @@ import java.io.Writer; +import static com.dotmarketing.util.WebKeys.DOTCMS_STARTED_UP; import static org.junit.Assert.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.*; /** * Unit tests for {@link VelocityHealthCheck}. Exercises the probe via a mocked @@ -27,8 +26,6 @@ */ public class VelocityHealthCheckTest { - private static final String STARTED_UP_PROPERTY = "dotcms.started.up"; - private MockedStatic velocityUtilMock; private String previousStartedUpProperty; @@ -37,8 +34,8 @@ public void setUp() { // The probe defers until dotCMS startup completes, which InitServlet signals via this // property in production. Set it here so the probe logic under test actually runs // instead of short-circuiting on every test. - previousStartedUpProperty = System.getProperty(STARTED_UP_PROPERTY); - System.setProperty(STARTED_UP_PROPERTY, "true"); + previousStartedUpProperty = System.getProperty(DOTCMS_STARTED_UP); + System.setProperty(DOTCMS_STARTED_UP, "true"); velocityUtilMock = mockStatic(VelocityUtil.class); } @@ -46,9 +43,9 @@ public void setUp() { public void tearDown() { velocityUtilMock.close(); if (previousStartedUpProperty == null) { - System.clearProperty(STARTED_UP_PROPERTY); + System.clearProperty(DOTCMS_STARTED_UP); } else { - System.setProperty(STARTED_UP_PROPERTY, previousStartedUpProperty); + System.setProperty(DOTCMS_STARTED_UP, previousStartedUpProperty); } } @@ -65,7 +62,7 @@ public void returnsUpWhenRenderMarksResolves() { public void returnsDownWhenStartupNotComplete() { // Even a healthy-looking engine shouldn't matter: the probe must defer before touching // VelocityUtil.getEngine() at all until InitServlet marks startup complete. - System.clearProperty(STARTED_UP_PROPERTY); + System.clearProperty(DOTCMS_STARTED_UP); stubEngineToWrite(""); final HealthCheckResult result = new VelocityHealthCheck().check();