Skip to content

perf(relationships): bulk tree persistence, lean hydration and delta-only reindex when saving related content #36092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jcastro-dotcms wants to merge 8 commits into from
Closed

perf(relationships): bulk tree persistence, lean hydration and delta-only reindex when saving related content #36092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e2ecab5
Creating draft PR with potential fix
jcastro-dotcms Jun 9, 2026
80cd1c9
Adding more potential changes
jcastro-dotcms Jun 11, 2026
ec92fa6
Merge branch 'main' into support-36004-save-content-with-related-cont…
jcastro-dotcms Jun 11, 2026
6b8cb38
Merge branch 'main' into support-36004-save-content-with-related-cont…
jcastro-dotcms Jun 11, 2026
9144940
Merge branch 'main' into support-36004-save-content-with-related-cont…
jcastro-dotcms Jun 11, 2026
f1946e1
Adding more potential changes
jcastro-dotcms Jun 11, 2026
48bc346
Implementing feedback from Claude Bot.
jcastro-dotcms Jun 12, 2026
f648126
Merge branch 'main' into support-36004-save-content-with-related-cont…
jcastro-dotcms Jun 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
300 changes: 228 additions & 72 deletions dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java
View file
Open in desktop

Large diffs are not rendered by default.

144 changes: 81 additions & 63 deletions dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESMappingAPIImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,5 @@
package com.dotcms.content.elasticsearch.business;

import static com.dotcms.content.elasticsearch.business.ESIndexAPI.INDEX_OPERATIONS_TIMEOUT_IN_MS;
import static com.dotcms.content.elasticsearch.constants.ESMappingConstants.CREATION_DATE;
import static com.dotcms.content.elasticsearch.constants.ESMappingConstants.PERSONA_KEY_TAG;
import static com.dotcms.content.elasticsearch.constants.ESMappingConstants.SYS_PUBLISH_USER;
import static com.dotcms.contenttype.model.field.LegacyFieldTypes.CUSTOM_FIELD;
import static com.dotcms.contenttype.model.type.PersonaContentType.PERSONA_KEY_TAG_FIELD_VAR;
import static com.dotcms.util.DotPreconditions.checkNotEmpty;
import static com.dotmarketing.business.PermissionAPI.PERMISSION_PUBLISH;
import static com.dotmarketing.business.PermissionAPI.PERMISSION_READ;
import static com.dotmarketing.business.PermissionAPI.PERMISSION_WRITE;
import static com.dotmarketing.util.UtilMethods.isNotSet;
import static com.liferay.util.StringPool.BLANK;
import static com.liferay.util.StringPool.COMMA;
import static com.liferay.util.StringPool.PERIOD;

import com.dotcms.business.CloseDBIfOpened;
import com.dotcms.cdi.CDIUtils;
import com.dotcms.content.business.ContentMappingAPI;
Expand Down Expand Up @@ -52,6 +37,7 @@
import com.dotmarketing.business.VersionableAPI;
import com.dotmarketing.cache.FieldsCache;
import com.dotmarketing.common.db.DotConnect;
import com.dotmarketing.common.model.ContentletSearch;
import com.dotmarketing.exception.DotCorruptedDataException;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotSecurityException;
Expand Down Expand Up @@ -79,6 +65,7 @@
import com.dotmarketing.util.Config;
import com.dotmarketing.util.InodeUtils;
import com.dotmarketing.util.Logger;
import com.dotmarketing.util.PaginatedArrayList;
import com.dotmarketing.util.ThreadSafeSimpleDateFormat;
import com.dotmarketing.util.UtilMethods;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -87,6 +74,9 @@
import com.liferay.portal.model.User;
import io.vavr.Lazy;
import io.vavr.control.Try;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang.time.FastDateFormat;

import java.io.IOException;
import java.io.StringWriter;
import java.text.DecimalFormat;
Expand All @@ -96,9 +86,11 @@
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
Expand All @@ -107,8 +99,20 @@
import java.util.TimeZone;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang.time.FastDateFormat;

import static com.dotcms.content.elasticsearch.constants.ESMappingConstants.CREATION_DATE;
import static com.dotcms.content.elasticsearch.constants.ESMappingConstants.PERSONA_KEY_TAG;
import static com.dotcms.content.elasticsearch.constants.ESMappingConstants.SYS_PUBLISH_USER;
import static com.dotcms.contenttype.model.field.LegacyFieldTypes.CUSTOM_FIELD;
import static com.dotcms.contenttype.model.type.PersonaContentType.PERSONA_KEY_TAG_FIELD_VAR;
import static com.dotcms.util.DotPreconditions.checkNotEmpty;
import static com.dotmarketing.business.PermissionAPI.PERMISSION_PUBLISH;
import static com.dotmarketing.business.PermissionAPI.PERMISSION_READ;
import static com.dotmarketing.business.PermissionAPI.PERMISSION_WRITE;
import static com.dotmarketing.util.UtilMethods.isNotSet;
import static com.liferay.util.StringPool.BLANK;
import static com.liferay.util.StringPool.COMMA;
import static com.liferay.util.StringPool.PERIOD;

/**
* Implementation class for the {@link ContentMappingAPI}.
Expand Down Expand Up @@ -1233,74 +1237,88 @@ public String toJsonString(Map<String, Object> map) throws IOException{
return mapper.writeValueAsString(map);
}

/**
* Returns the identifiers of the contentlets whose search index documents became stale
* because of changes to the given contentlet's relationships — i.e., the contents that must
* be re-indexed along with it.
*
* <p>The method compares the relationships referencing this contentlet in the search index
* (the state previous to the current save) against the rows in the {@code tree} table (the
* just-saved state). The symmetric difference of both sets — relationships that were removed
* plus relationships that were added — is the complete set of stale documents: only PARENT
* documents store relationship references (see
* {@link #loadRelationshipFields(Contentlet, Map, StringWriter)}), so contents whose
* relationship to this contentlet did not change never need re-indexing.</p>
*
* @param contentlet The {@link Contentlet} being re-indexed
* @return Identifiers of the related contents that must be re-indexed along with it
*/
@CloseDBIfOpened
public List<String> dependenciesLeftToReindex(final Contentlet contentlet) throws DotStateException, DotDataException, DotSecurityException {
final List<String> dependenciesToReindex = new ArrayList<>();


final String relatedSQL = "select tree.* from tree where child = ? order by tree_order";
final DotConnect db = new DotConnect();
db.setSQL(relatedSQL);
db.addParam(contentlet.getIdentifier());

final List<HashMap<String, String>> relatedContentlets = db.loadResults();

if(relatedContentlets.size()>0) {
if (!relatedContentlets.isEmpty()) {

final List<Relationship> relationships = relationshipAPI
.byContentType(contentlet.getContentType());

for(final Relationship relationship : relationships) {

final List<Contentlet> oldDocs;
final List<String> oldRelatedIds = new ArrayList<>();
final List<String> newRelatedIds = new ArrayList<>();

oldDocs = contentletAPI.getRelatedContent(contentlet, relationship,
userAPI.getSystemUser(), false);

if(oldDocs.size() > 0) {
for(Contentlet oldDoc : oldDocs) {
oldRelatedIds.add(oldDoc.getIdentifier());
}
for (final Relationship relationship : relationships) {
// Both sides are collected into Sets of identifiers: the index holds one
// document per language/variant of the same identifier, and only deduped
// collections make the disjunction below a true symmetric difference
final Collection<String> oldRelatedIds = new LinkedHashSet<>();
final Collection<String> newRelatedIds = new LinkedHashSet<>();

final String relatedQuery = "+" + relationship.getRelationTypeValue()
+ ":" + contentlet.getIdentifier();
List<ContentletSearch> oldSearchResults = contentletAPI.searchIndex(
relatedQuery, ESContentletAPIImpl.MAX_LIMIT, 0, null,
userAPI.getSystemUser(), false);
if (oldSearchResults instanceof PaginatedArrayList
&& ((PaginatedArrayList<?>) oldSearchResults).getTotalResults()
> oldSearchResults.size()) {
// More documents reference this contentlet than a single search page can
// return; a limit above MAX_LIMIT makes searchIndex switch to a scroll
// search that returns them all
Logger.warn(this, () -> "More than " + ESContentletAPIImpl.MAX_LIMIT
+ " index documents reference '" + contentlet.getIdentifier()
+ "' through '" + relationship.getRelationTypeValue()
+ "'. Falling back to a scroll search");
oldSearchResults = contentletAPI.searchIndex(relatedQuery,
ESContentletAPIImpl.MAX_LIMIT + 1, 0, null,
userAPI.getSystemUser(), false);
}
for (final ContentletSearch result : oldSearchResults) {
oldRelatedIds.add(result.getIdentifier());
}

relatedContentlets.stream().filter(map -> map.get(ESMappingConstants.RELATION_TYPE)
.equals(relationship.getRelationTypeValue())).forEach(
entry -> replaceExistingRelatedContent(entry, contentlet,
oldRelatedIds, newRelatedIds));

//Taking the disjunction of both collections will give the old list of dependencies that need to be removed from the
//re-indexation and the list of new dependencies no re-indexed yet
relatedContentlets.stream()
.filter(map -> map.get(ESMappingConstants.RELATION_TYPE).equals(relationship.getRelationTypeValue()))
.forEach(entry -> {
final String childId = entry.get(ESMappingConstants.CHILD);
final String parentId = entry.get(ESMappingConstants.PARENT);
newRelatedIds.add(contentlet.getIdentifier().equalsIgnoreCase(childId)
? parentId
: childId);
});

// The symmetric difference of both Sets is the actual dependency delta: the
// contents that lost the relationship (their index document still references
// this contentlet) plus the ones that just gained it (not indexed yet).
// Contents whose relationship did NOT change are excluded — re-indexing them
// on every save is what made saves with heavily related content so expensive
dependenciesToReindex.addAll(
CollectionUtils.disjunction(oldRelatedIds, newRelatedIds));
}
}
return dependenciesToReindex;
}

/**
*
* @param relatedEntry
* @param con
* @param oldRelatedIds
* @param newRelatedIds
*/
private void replaceExistingRelatedContent(final Map<String, String> relatedEntry,
final Contentlet con, final List<String> oldRelatedIds,
final List<String> newRelatedIds) {

final String childId = relatedEntry.get(ESMappingConstants.CHILD);
final String parentId = relatedEntry.get(ESMappingConstants.PARENT);
if (con.getIdentifier().equalsIgnoreCase(childId)) {
newRelatedIds.add(parentId);
oldRelatedIds.remove(parentId);
} else {
newRelatedIds.add(childId);
oldRelatedIds.remove(childId);
}
}

/**
* @deprecated Use {@link ESMappingAPIImpl#loadRelationshipFields(Contentlet, Map, StringWriter)} instead
* @param contentlet
Expand Down
22 changes: 18 additions & 4 deletions dotCMS/src/main/java/com/dotmarketing/business/VersionableFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
package com.dotmarketing.business;

import com.dotcms.variant.model.Variant;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import com.dotmarketing.beans.Identifier;
import com.dotmarketing.beans.VersionInfo;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo;

import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;

/**
* Provides access to information regarding the different versions (working,
* live, and version info) of a dotCMS content object -i.e., contents,
Expand Down Expand Up @@ -277,6 +278,19 @@ protected abstract List<ContentletVersionInfo> findAllContentletVersionInfos(Str
protected abstract List<ContentletVersionInfo> findAllContentletVersionInfos(String identifier, String variantName)
throws DotDataException, DotStateException ;

/**
* Returns ALL the {@link ContentletVersionInfo} rows — every language and variant — for the
* given set of identifiers, reading them from the database in batches of bound parameters.
* Unlike {@link #findAllContentletVersionInfos(String)}, results are NOT cached: this method
* is meant for bulk read-only operations, such as collecting the working versions of related
* content that must be re-indexed.
*
* @param identifiers Identifiers of the contentlets whose version info rows are returned
* @return The {@link ContentletVersionInfo} rows found for the given identifiers
*/
public abstract List<ContentletVersionInfo> findAllContentletVersionInfos(
Collection<String> identifiers) throws DotDataException;

/**
* Return all versions of the {@link com.dotmarketing.portlets.contentlet.model.Contentlet}
* for the specific {@link com.dotcms.variant.model.Variant} no matter the {@Link Language}
Expand Down
37 changes: 30 additions & 7 deletions dotCMS/src/main/java/com/dotmarketing/business/VersionableFactoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
package com.dotmarketing.business;

import static com.dotcms.util.CollectionsUtils.set;
import static com.dotcms.variant.VariantAPI.DEFAULT_VARIANT;

import com.dotcms.concurrent.DotConcurrentFactory;
import com.dotcms.concurrent.lock.IdentifierStripedLock;
import com.google.common.annotations.VisibleForTesting;
import com.dotcms.util.transform.TransformerLocator;
import com.dotcms.variant.model.Variant;
import com.dotmarketing.beans.Identifier;
Expand All @@ -16,7 +12,6 @@
import com.dotmarketing.db.HibernateUtil;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotRuntimeException;
import com.dotmarketing.exception.DotSecurityException;
import com.dotmarketing.portlets.containers.business.ContainerAPI;
import com.dotmarketing.portlets.containers.model.Container;
import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo;
Expand All @@ -26,19 +21,23 @@
import com.dotmarketing.util.Logger;
import com.dotmarketing.util.UUIDGenerator;
import com.dotmarketing.util.UtilMethods;
import com.google.common.annotations.VisibleForTesting;
import com.liferay.portal.model.User;
import org.apache.commons.beanutils.BeanUtils;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;

import org.apache.commons.beanutils.BeanUtils;
import static com.dotcms.util.CollectionsUtils.set;
import static com.dotcms.variant.VariantAPI.DEFAULT_VARIANT;

/**
* Implementation class for the {@link VersionableFactory} class.
Expand Down Expand Up @@ -460,6 +459,30 @@ private List<ContentletVersionInfo> findContentletVersionInfosInDB(final String
: versionInfos;
}

private static final int VERSION_INFO_LOOKUP_CHUNK_SIZE = 500;

@Override
public List<ContentletVersionInfo> findAllContentletVersionInfos(
final Collection<String> identifiers) throws DotDataException {
if (identifiers == null || identifiers.isEmpty()) {
return Collections.emptyList();
}
final List<String> idList = List.copyOf(identifiers);
final List<ContentletVersionInfo> versionInfos = new ArrayList<>();
for (int from = 0; from < idList.size(); from += VERSION_INFO_LOOKUP_CHUNK_SIZE) {
final List<String> chunk = idList.subList(from,
Math.min(from + VERSION_INFO_LOOKUP_CHUNK_SIZE, idList.size()));
final DotConnect dotConnect = new DotConnect().setSQL(
"SELECT * FROM contentlet_version_info WHERE identifier IN ("
+ DotConnect.createParametersPlaceholder(chunk.size()) + ")");
Comment on lines +476 to +477

@semgrep-code-dotcms-test semgrep-code-dotcms-test Bot Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semgrep identified a blocking 🔴 issue in your code:
The method identified is susceptible to injection. The input should be validated and properly
escaped.

Why this might be safe to ignore:

This query builds only the number of parameter placeholders for an IN clause, and the actual identifier values are passed separately with addParam, so user input is not concatenated into SQL. The rule matched a SQL string concatenation pattern, but in this context it does not create a meaningful injection risk.

To resolve this comment:

🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.

💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by CUSTOM_INJECTION-2.

If this is a critical or high severity finding, please also link this issue in the #security channel in Slack.

You can view more details about this finding in the Semgrep AppSec Platform.

chunk.forEach(dotConnect::addParam);
versionInfos.addAll(TransformerLocator
.createContentletVersionInfoTransformer(dotConnect.loadObjectResults())
.asList());
}
return versionInfos;
}

@Override
protected List<ContentletVersionInfo> findAllContentletVersionInfos(final String identifier)
throws DotDataException, DotStateException {
Expand Down
Loading
Loading