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..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. @@ -78,6 +79,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(); 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/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); 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/dotcms/health/checks/cdi/VelocityHealthCheckTest.java b/dotCMS/src/test/java/com/dotcms/health/checks/cdi/VelocityHealthCheckTest.java index 4fd6f3aa48e..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 @@ -28,15 +27,26 @@ public class VelocityHealthCheckTest { 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(DOTCMS_STARTED_UP); + System.setProperty(DOTCMS_STARTED_UP, "true"); velocityUtilMock = mockStatic(VelocityUtil.class); } @After public void tearDown() { velocityUtilMock.close(); + if (previousStartedUpProperty == null) { + System.clearProperty(DOTCMS_STARTED_UP); + } else { + System.setProperty(DOTCMS_STARTED_UP, previousStartedUpProperty); + } } @Test @@ -48,6 +58,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(DOTCMS_STARTED_UP); + 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 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)); + } +} 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..b4c0d0abb05 --- /dev/null +++ b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/LanguageViewStrategyTest.java @@ -0,0 +1,112 @@ +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("fr", inner.get("isoCode")); + 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")); + assertEquals("es", languageMap.get("isoCode")); + } +}