From 53b0955a9ab6e6e40c3280d87e2a67f4a7643df9 Mon Sep 17 00:00:00 2001 From: Daniel Silva Date: Mon, 7 Sep 2026 22:54:30 +0200 Subject: [PATCH 1/2] fix(content): keep explicitly cleared fields from taking the Content Type default (#35416) ContentletHashMap#put removes a key outright when its value is null -- it extends ConcurrentHashMap, which forbids null values -- so a field the user deliberately cleared became indistinguishable from one that was never submitted. On new content only, checkin's setDefaultValues then reapplied the Content Type's default value, silently re-checking "Show on Menu" for pages whose type defaults it to true. Honour the contentlet's existing nullProperties set, the way validateContentlet already does, so an explicit clear survives checkin. Refs: #35416 Co-Authored-By: Claude Opus 5 --- .../business/ESContentletAPIImpl.java | 10 ++++- .../business/ContentletCheckInTest.java | 43 ++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java index e05dec591d4f..a17a9349c9f2 100644 --- a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java @@ -6213,11 +6213,19 @@ private Contentlet setDefaultValues(final Contentlet contentlet) { final List fields = Try.of(()->contentlet.getContentType().fields()).getOrElse(Collections.emptyList()); final Map map = contentlet.getMap(); + // A property the user explicitly cleared is dropped from the map entirely by + // ContentletHashMap#put (it extends ConcurrentHashMap, which forbids null values), so an + // absent key alone cannot tell "never submitted" from "deliberately emptied". The contentlet + // tracks the latter in its null-properties set -- honour it, as validateContentlet does, so + // an unchecked checkbox is not silently restored from the field's default value. + final Set nullProperties = contentlet.getNullProperties(); Logger.debug(this, ()-> "Setting default values for the contentlet: " + contentlet.getIdentifier()); // check default values for fields not coming on the map for (final com.dotcms.contenttype.model.field.Field field : fields) { - if (!map.containsKey(field.variable()) && UtilMethods.isSet(field.defaultValue())) { + if (!map.containsKey(field.variable()) + && !nullProperties.contains(field.variable()) + && UtilMethods.isSet(field.defaultValue())) { try { this.setContentletProperty(contentlet, field, field.defaultValue()); diff --git a/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/ContentletCheckInTest.java b/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/ContentletCheckInTest.java index d8779862d2f0..b81e307d4d44 100644 --- a/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/ContentletCheckInTest.java +++ b/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/ContentletCheckInTest.java @@ -1,8 +1,11 @@ package com.dotmarketing.portlets.contentlet.business; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import com.dotcms.contenttype.model.field.CheckboxField; +import com.dotcms.contenttype.model.field.DataTypes; import com.dotcms.contenttype.model.field.Field; import com.dotcms.contenttype.model.field.FieldBuilder; import com.dotcms.contenttype.model.field.RelationshipField; @@ -10,7 +13,9 @@ import com.dotcms.contenttype.model.type.ContentType; import com.dotcms.contenttype.model.type.ContentTypeBuilder; import com.dotcms.contenttype.model.type.SimpleContentType; +import com.dotcms.datagen.ContentTypeDataGen; import com.dotcms.datagen.ContentletDataGen; +import com.dotcms.datagen.FieldDataGen; import com.dotcms.rest.AnonymousAccess; import com.dotcms.util.CollectionsUtils; import com.dotmarketing.beans.Host; @@ -29,6 +34,7 @@ import com.dotmarketing.portlets.structure.model.ContentletRelationships; import com.dotmarketing.portlets.structure.model.Relationship; import com.dotmarketing.util.Config; +import com.dotmarketing.util.UtilMethods; import com.dotmarketing.util.WebKeys; import com.dotmarketing.util.WebKeys.Relationship.RELATIONSHIP_CARDINALITY; import com.liferay.portal.model.User; @@ -71,7 +77,42 @@ public void checkin_content_anonymously () throws Exception { assertTrue("the contentlet title was saved", newCon.getTitle().equals(con.getTitle())); assertTrue("contentlet is not live", newCon.isWorking() && !newCon.hasLiveVersion()); - + + } + + /** + * Method to Test: {@link ContentletAPI#checkin(Contentlet, User, boolean)} + * When: A new Contentlet is checked in with a Checkbox field the user explicitly cleared, on a + * Content Type whose field declares a default value of "true" + * Should: Keep the field empty -- an explicit clear must not be mistaken for "never submitted" + * and overwritten with the Content Type's default value + * + * @see Issue #35416 + */ + @Test + public void checkin_new_content_with_cleared_checkbox_does_not_apply_default_value() + throws Exception { + + final ContentType contentType = new ContentTypeDataGen().nextPersisted(); + final Field checkboxField = new FieldDataGen() + .contentTypeId(contentType.id()) + .type(CheckboxField.class) + .dataType(DataTypes.TEXT) + .values("|true") + .defaultValue("true") + .nextPersisted(); + + final Contentlet contentlet = new ContentletDataGen(contentType.id()).next(); + // Clearing a checkbox sets a null, and ContentletHashMap#put drops the key from the map + // entirely -- the same state the edit form produces when the box is left unchecked. + contentlet.setProperty(checkboxField.variable(), null); + + final Contentlet checkedIn = contentletAPI.checkin(contentlet, user, false); + final String persisted = checkedIn.getStringProperty(checkboxField.variable()); + + // Mirrors HTMLPageAsset#isShowOnMenu, which reads the raw value with contains("true") + assertFalse("A cleared checkbox must not be restored from the field's default value", + UtilMethods.isSet(persisted) && persisted.contains("true")); } /** From 6f3026e3787c34d11f9636d34e7daf44230c114c Mon Sep 17 00:00:00 2001 From: Daniel Silva Date: Tue, 8 Sep 2026 15:10:11 +0200 Subject: [PATCH 2/2] fix(content): limit the cleared-field guard to selection fields (#35416) The previous commit honoured nullProperties for every field type, which broke an intentional contract: for text and numeric fields a null means "not provided" and the Content Type default should still be applied. ContentletJsonAPITest's Initialize_Fields_With_Default_Value_Test asserts exactly that, and it failed in CI along with StoryBlockMapTest and ShortyServletAndTitleImageTest, both of which depend on defaults backfilling nulled fields. Restrict the guard to Checkbox and MultiSelect fields, where the value is a set of selected options and the empty set is itself a valid choice. Everything else keeps its previous behaviour. Refs: #35416 Co-Authored-By: Claude Opus 5 --- .../business/ESContentletAPIImpl.java | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java index a17a9349c9f2..093384b26c59 100644 --- a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java @@ -25,6 +25,7 @@ import com.dotcms.contenttype.exception.NotFoundInDbException; import com.dotcms.contenttype.model.field.BinaryField; import com.dotcms.contenttype.model.field.CategoryField; +import com.dotcms.contenttype.model.field.CheckboxField; import com.dotcms.contenttype.model.field.ColumnField; import com.dotcms.contenttype.model.field.ConstantField; import com.dotcms.contenttype.model.field.DataTypes; @@ -32,6 +33,7 @@ import com.dotcms.contenttype.model.field.HostFolderField; import com.dotcms.contenttype.model.field.JSONField; import com.dotcms.contenttype.model.field.LineDividerField; +import com.dotcms.contenttype.model.field.MultiSelectField; import com.dotcms.contenttype.model.field.RelationshipField; import com.dotcms.contenttype.model.field.RowField; import com.dotcms.contenttype.model.field.TabDividerField; @@ -6213,18 +6215,13 @@ private Contentlet setDefaultValues(final Contentlet contentlet) { final List fields = Try.of(()->contentlet.getContentType().fields()).getOrElse(Collections.emptyList()); final Map map = contentlet.getMap(); - // A property the user explicitly cleared is dropped from the map entirely by - // ContentletHashMap#put (it extends ConcurrentHashMap, which forbids null values), so an - // absent key alone cannot tell "never submitted" from "deliberately emptied". The contentlet - // tracks the latter in its null-properties set -- honour it, as validateContentlet does, so - // an unchecked checkbox is not silently restored from the field's default value. final Set nullProperties = contentlet.getNullProperties(); Logger.debug(this, ()-> "Setting default values for the contentlet: " + contentlet.getIdentifier()); // check default values for fields not coming on the map for (final com.dotcms.contenttype.model.field.Field field : fields) { if (!map.containsKey(field.variable()) - && !nullProperties.contains(field.variable()) + && !isClearedSelectionField(field, nullProperties) && UtilMethods.isSet(field.defaultValue())) { try { @@ -6241,6 +6238,31 @@ private Contentlet setDefaultValues(final Contentlet contentlet) { return contentlet; } + /** + * Tells whether the user deliberately emptied a multi-value selection field, rather than never + * submitting it at all. + *

+ * {@code ContentletHashMap#put} removes a key outright when its value is null -- it extends + * {@link java.util.concurrent.ConcurrentHashMap}, which forbids null values -- so an absent key + * alone cannot tell those two cases apart. An explicit clear is recorded in the contentlet's + * null-properties set, which {@link #validateContentlet} already consults. + *

+ * This only covers fields whose value is a set of selected options, where the empty set is + * itself a valid choice -- an unchecked "Show on Menu" on a new page, for instance. For a text + * or numeric field a null still means "not provided" and its default value is applied as + * before, which + * {@code ContentletJsonAPITest#Initialize_Fields_With_Default_Value_Test} relies on. + * + * @param field the Content Type field being considered + * @param nullProperties the contentlet's explicitly-nulled property names + * @return {@code true} when this is a selection field the user cleared on purpose + */ + private boolean isClearedSelectionField(final com.dotcms.contenttype.model.field.Field field, + final Set nullProperties) { + return (field instanceof CheckboxField || field instanceof MultiSelectField) + && nullProperties.contains(field.variable()); + } + private static boolean hasUniqueField(ContentType contentType) { return contentType.fields().stream().anyMatch(field -> field.unique()); }