Issue 29501 optimize sql (#30345)

This pull request includes various changes to the `TemplateFactoryImpl`
and `TemplateSQL` classes to improve code consistency and performance.
The most significant changes include the addition of a new method to
check for index existence, refactoring of SQL query strings, and the
introduction of a new startup task for indexing.

### New Features:
* Added a new method `checkIndex` in `DotDatabaseMetaData` to verify if
an index exists in a table.
(`dotCMS/src/main/java/com/dotmarketing/common/db/DotDatabaseMetaData.java`)

### Code Refactoring:
* Refactored SQL query strings in `TemplateFactoryImpl` to use constants
from `TemplateSQL` for better maintainability.
(`dotCMS/src/main/java/com/dotmarketing/portlets/templates/business/TemplateFactoryImpl.java`)

### Performance Improvements:
* Introduced a new startup task
`Task241014AddTemplateValueOnContentletIndex` to add an index on the
template value of a contentlet, significantly speeding up queries.
(`dotCMS/src/main/java/com/dotmarketing/startup/runonce/Task241014AddTemplateValueOnContentletIndex.java`)
* Added the new startup task to `TaskLocatorUtil`.
(`dotCMS/src/main/java/com/dotmarketing/util/TaskLocatorUtil.java`)
* Updated the PostgreSQL schema to include the new index for contentlet
template values. (`dotCMS/src/main/resources/postgres.sql`)

### Bug Fixes:
* Corrected a typo in a constant name from
`CONTENTKET_INODE_TABLE_FIELD` to `CONTENTLET_INODE_TABLE_FIELD` in
`TemplateFactoryImpl`.
(`dotCMS/src/main/java/com/dotmarketing/portlets/templates/business/TemplateFactoryImpl.java`)

### Test Updates:
* Updated test suite `MainSuite2b` to include new and existing tasks.
(`dotcms-integration/src/test/java/com/dotcms/MainSuite2b.java`)
* Modified `TemplateAPITest` to remove redundant assertions.
(`dotcms-integration/src/test/java/com/dotmarketing/portlets/templates/business/TemplateAPITest.java`)

dotCMS/src/main/java/com/dotmarketing/common/db/DotDatabaseMetaData.java

@@ -846,5 +846,30 @@ public boolean isColumnLengthExpected(final String tableName, final String colum
         }
         return isColumnLengthExpected;
     }
+
+    /**
+     * Checks if an index exists in a table
+     * @param tableName
+     * @param indexName
+     * @return boolean - true if the index exists in the table
+     */
+    public boolean checkIndex(final String tableName, final String indexName) {
+        final DotDatabaseMetaData dbMetadata = new DotDatabaseMetaData();
+
+        try {
+            final ResultSet indicesInfo = dbMetadata.getIndices(DbConnectionFactory.getConnection(),
+                    null, tableName, false);
+            while (indicesInfo.next()) {
+                final String currentIndex = indicesInfo.getString("INDEX_NAME");
+
+                if (currentIndex.equals(indexName)) {
+                    return true;
+                }
+            }
+        } catch (final SQLException e) {
+            Logger.error(this, e);
+        }
+        return false;
+    }
 } // E:O:F:DotDatabaseMetaData.
 

dotCMS/src/main/java/com/dotmarketing/portlets/templates/business/TemplateFactoryImpl.java

@@ -41,9 +41,9 @@
 
 public class TemplateFactoryImpl implements TemplateFactory {
 
-	public static final String CONTENTKET_INODE_TABLE_FIELD = "inode";
+	public static final String CONTENTLET_INODE_TABLE_FIELD = "inode";
+	private static final String FILTER_STRING = "filter";
 	private static TemplateCache templateCache = CacheLocator.getTemplateCache();
-	private static TemplateSQL templateSQL = TemplateSQL.getInstance();
 
 	@SuppressWarnings("unchecked")
 
@@ -53,7 +53,7 @@ public Template find(final String inode) throws DotStateException, DotDataExcept
 
 		if(template==null){
 			final List<Map<String, Object>> templateResults = new DotConnect()
-					.setSQL(templateSQL.FIND_BY_INODE)
+					.setSQL(TemplateSQL.FIND_BY_INODE)
 					.addParam(inode)
 					.loadObjectResults();
 			if (templateResults.isEmpty()) {
@@ -76,8 +76,8 @@ public List<Template> findTemplatesAssignedTo(final Host parentHost, final boole
 			throws DotDataException {
 		final DotConnect dc = new DotConnect();
 		final String query = !includeArchived ?
-				templateSQL.FIND_TEMPLATES_BY_HOST_INODE + " and vi.deleted = "
-						+ DbConnectionFactory.getDBFalse() : templateSQL.FIND_TEMPLATES_BY_HOST_INODE;
+				TemplateSQL.FIND_TEMPLATES_BY_HOST_INODE + " and vi.deleted = "
+						+ DbConnectionFactory.getDBFalse() : TemplateSQL.FIND_TEMPLATES_BY_HOST_INODE;
 		dc.setSQL(query);
 		dc.addParam(parentHost.getIdentifier());
 
@@ -91,7 +91,7 @@ public List<Template> findTemplatesAssignedTo(final Host parentHost, final boole
 	@SuppressWarnings("unchecked")
 	public List<Template> findTemplatesUserCanUse(final User user, final String hostId, final String query, final boolean searchHost , final int offset, final int limit) throws DotDataException, DotSecurityException {
 		return findTemplates(user, false,
-				UtilMethods.isSet(query) ? Map.of("filter", query.toLowerCase())
+				UtilMethods.isSet(query) ? Map.of(FILTER_STRING, query.toLowerCase())
 						: null, hostId, null, null, null, offset, limit, "title");
 	}
 
@@ -135,7 +135,7 @@ public void save(final Template template, final String inode) throws DotDataExce
 
 	private void insertInodeInDB(final Template template) throws DotDataException{
 		DotConnect dc = new DotConnect();
-		dc.setSQL(templateSQL.INSERT_INODE);
+		dc.setSQL(TemplateSQL.INSERT_INODE);
 		dc.addParam(template.getInode());
 		dc.addParam(template.getiDate());
 		dc.addParam(template.getOwner());
@@ -144,7 +144,7 @@ private void insertInodeInDB(final Template template) throws DotDataException{
 
 	private void insertTemplateInDB(final Template template) throws DotDataException {
 		DotConnect dc = new DotConnect();
-		dc.setSQL(templateSQL.INSERT_TEMPLATE);
+		dc.setSQL(TemplateSQL.INSERT_TEMPLATE);
 		dc.addParam(template.getInode());
 		dc.addParam(template.isShowOnMenu());
 		dc.addParam(template.getTitle());
@@ -177,7 +177,7 @@ public void deleteFromCache(final Template template) throws DotDataException {
 	@SuppressWarnings("unchecked")
 	public Template findWorkingTemplateByName(String name, Host host) throws DotDataException {
 		DotConnect dc = new DotConnect();
-		dc.setSQL(templateSQL.FIND_WORKING_TEMPLATE_BY_HOST_INODE_AND_TITLE);
+		dc.setSQL(TemplateSQL.FIND_WORKING_TEMPLATE_BY_HOST_INODE_AND_TITLE);
 		dc.addParam(host.getIdentifier());
 		dc.addParam(name);
 		try{
@@ -266,15 +266,15 @@ public List<Template> findTemplates(User user, boolean includeArchived,
 			dc.setSQL(query.toString());
 
 			if(params!=null && params.size()>0){
-				final String filter = "%"+params.get("filter").toString().toLowerCase()+"%";
+				final String filter = "%"+params.get(FILTER_STRING).toString().toLowerCase()+"%";
 				dc.addParam(filter);
 				dc.addParam(filter);
 				dc.addParam(filter);
 			}
 
 			// adding the templates from the site browser
 			toReturn.addAll(this.findFolderAssetTemplate(user, hostId, orderBy,
-					includeArchived, params != null ? params.get("filter").toString(): null));
+					includeArchived, params != null ? params.get(FILTER_STRING).toString(): null));
 
 			if(countLimit > 0 && toReturn.size() < countLimit + offset) {//If haven't reach the amount of templates requested
 				while (!done) {
@@ -287,7 +287,7 @@ public List<Template> findTemplates(User user, boolean includeArchived,
 					//Search by inode
 					if (resultList.isEmpty()) {
 						final Template templateInode =
-								params != null ? find(params.get("filter").toString()) : null;
+								params != null ? find(params.get(FILTER_STRING).toString()) : null;
 						resultList =
 								templateInode != null ? List.of(templateInode)
 										: Collections.emptyList();
@@ -549,7 +549,7 @@ public void updateThemeWithoutVersioning(final String templateInode, final Strin
 
 	private void updateInodeInDB(final Template template) throws DotDataException{
 		DotConnect dc = new DotConnect();
-		dc.setSQL(templateSQL.UPDATE_INODE);
+		dc.setSQL(TemplateSQL.UPDATE_INODE);
 		dc.addParam(template.getiDate());
 		dc.addParam(template.getOwner());
 		dc.addParam(template.getInode());
@@ -558,7 +558,7 @@ private void updateInodeInDB(final Template template) throws DotDataException{
 
 	private void updateTemplateInDB(final Template template) throws DotDataException {
 		DotConnect dc = new DotConnect();
-		dc.setSQL(templateSQL.UPDATE_TEMPLATE);
+		dc.setSQL(TemplateSQL.UPDATE_TEMPLATE);
 		dc.addParam(template.isShowOnMenu());
 		dc.addParam(template.getTitle());
 		dc.addParam(template.getModDate());
@@ -593,22 +593,22 @@ public void updateUserReferences(String userId, String replacementUserId)throws
 		DotConnect dc = new DotConnect();
 
 		try {
-			dc.setSQL(templateSQL.FIND_TEMPLATES_BY_MOD_USER);
+			dc.setSQL(TemplateSQL.FIND_TEMPLATES_BY_MOD_USER);
 			dc.addParam(userId);
 			List<HashMap<String, String>> templates = dc.loadResults();
 
-			dc.setSQL(templateSQL.UPDATE_MOD_USER_BY_MOD_USER);
+			dc.setSQL(TemplateSQL.UPDATE_MOD_USER_BY_MOD_USER);
 			dc.addParam(replacementUserId);
 			dc.addParam(userId);
 			dc.loadResult();
 
-			dc.setSQL(templateSQL.UPDATE_LOCKED_BY);
+			dc.setSQL(TemplateSQL.UPDATE_LOCKED_BY);
 			dc.addParam(replacementUserId);
 			dc.addParam(userId);
 			dc.loadResult();
 
 			for(HashMap<String, String> ident:templates){
-				String inode = ident.get("inode");
+				String inode = ident.get(CONTENTLET_INODE_TABLE_FIELD);
 				Template template = find(inode);
 				deleteFromCache(template);
 				new TemplateLoader().invalidate(template);
@@ -629,18 +629,18 @@ public List<Template> findAllVersions(final Identifier identifier, final boolean
 		final StringBuffer query = new StringBuffer();
 
 		if(bringOldVersions) {
-			query.append(templateSQL.FIND_ALL_VERSIONS_BY_IDENTIFIER);
+			query.append(TemplateSQL.FIND_ALL_VERSIONS_BY_IDENTIFIER);
 
 		} else {//This only brings the inode of the working and live version
-			query.append(templateSQL.FIND_WORKING_LIVE_VERSION_BY_IDENTIFIER);
+			query.append(TemplateSQL.FIND_WORKING_LIVE_VERSION_BY_IDENTIFIER);
 		}
 
 		dc.setSQL(query.toString());
 		dc.addParam(identifier.getId());
 		final List<Map<String,Object>> results=dc.loadObjectResults();
 		final List<Template> templateAllVersions = new ArrayList<>();
 		for(Map<String,Object> result : results) {
-			final Template template = find(result.get("inode").toString());
+			final Template template = find(result.get(CONTENTLET_INODE_TABLE_FIELD).toString());
 			templateAllVersions.add(template);
 		}
 		return templateAllVersions;
@@ -653,21 +653,21 @@ public void deleteTemplateByInode(final String templateInode) throws DotDataExce
 
 	private void deleteInodeInDB(final String inode) throws DotDataException{
 		DotConnect dc = new DotConnect();
-		dc.setSQL(templateSQL.DELETE_INODE);
+		dc.setSQL(TemplateSQL.DELETE_INODE);
 		dc.addParam(inode);
 		dc.loadResult();
 	}
 
 	private void deleteTemplateInDB(final String inode) throws DotDataException{
 		DotConnect dc = new DotConnect();
-		dc.setSQL(templateSQL.DELETE_TEMPLATE_BY_INODE);
+		dc.setSQL(TemplateSQL.DELETE_TEMPLATE_BY_INODE);
 		dc.addParam(inode);
 		dc.loadResult();
 	}
 
 	public List<Template> findTemplatesByContainerInode(final String containerInode) throws DotDataException{
 		DotConnect dc = new DotConnect();
-		dc.setSQL(templateSQL.FIND_TEMPLATES_BY_CONTAINER_INODE);
+		dc.setSQL(TemplateSQL.FIND_TEMPLATES_BY_CONTAINER_INODE);
 		dc.addParam(containerInode);
 		return TransformerLocator.createTemplateTransformer(dc.loadObjectResults()).asList();
 	}
@@ -734,24 +734,12 @@ public List<HTMLPageVersion> getPages(final String templateId)
 			throws DotDataException, DotSecurityException {
 
 		final DotConnect dotConnect = new DotConnect();
-
-		if (DbConnectionFactory.isMsSql()) {
-			dotConnect.setSQL("SELECT identifier,inode,language_id, variant_id as variant "
-					+ "FROM contentlet "
-					+ "WHERE JSON_VALUE(contentlet_as_json, '$.fields.template.value') = ?");
-		} else {
-			dotConnect.setSQL("SELECT identifier,inode,language_id,variant_id as variant "
-					+ "FROM contentlet "
-					+ "WHERE contentlet_as_json->'fields'->'template'->>'value' =  ?");
-		}
-
+		dotConnect.setSQL(TemplateSQL.GET_PAGES_BY_TEMPLATE_ID);
 		dotConnect.addParam(templateId);
 
 		return ((List<Map<String, String>>) dotConnect.loadResults()).stream()
 				.map(mapEntry -> new HTMLPageVersion.Builder().identifier(mapEntry.get("identifier"))
-						.inode(mapEntry.get(CONTENTKET_INODE_TABLE_FIELD))
 						.variantName(mapEntry.get("variant"))
-						.languageId(Long.parseLong(mapEntry.get("language_id")))
 						.build()
 				)
 				.collect(Collectors.toList());
@@ -841,14 +829,14 @@ private List<Folder> findTemplatesAssetsByHost(final Host host, final User user,
 		}
 
 		final DotConnect dc = new DotConnect().setSQL(sqlQuery.toString());
-		parameters.forEach(param -> dc.addParam(param));
+		parameters.forEach(dc::addParam);
 
 		try {
 			final List<Map<String,String>> inodesMapList =  dc.loadResults();
 
 			final List<String> inodes = new ArrayList<>();
 			for (final Map<String, String> versionInfoMap : inodesMapList) {
-				inodes.add(versionInfoMap.get("inode"));
+				inodes.add(versionInfoMap.get(CONTENTLET_INODE_TABLE_FIELD));
 			}
 
 			final List<Contentlet> contentletList  = APILocator.getContentletAPI().findContentlets(inodes);
@@ -957,6 +945,7 @@ private Collection<? extends Permissionable> findFolderAssetTemplate(final User
 
 			if (UtilMethods.isSet(orderByParam)) {
 				switch (orderByParam.toLowerCase()) {
+					default:
 					case "title asc":
 						templates.sort(Comparator.comparing(Template::getTitle));
 						break;
@@ -980,4 +969,4 @@ private Collection<? extends Permissionable> findFolderAssetTemplate(final User
 			return Collections.emptyList();
 		}
 	}
-}
+}

dotCMS/src/main/java/com/dotmarketing/portlets/templates/business/TemplateSQL.java

@@ -4,7 +4,17 @@
 
 public class TemplateSQL {
 
-    public static TemplateSQL getInstance(){ return new TemplateSQL(); }
+    private static TemplateSQL instance;
+
+    private TemplateSQL(){
+    }
+    public static TemplateSQL getInstance() {
+        if (instance == null) {
+            instance = new TemplateSQL();
+        }
+
+        return instance;
+    }
 
     public static final String FIND_TEMPLATES_BY_HOST_INODE =
             "select template.*, template_identifier.*  from " + Type.TEMPLATE.getTableName() + " template, inode template_1_, " +
@@ -55,4 +65,7 @@ public class TemplateSQL {
 
     public static final String UPDATE_LOCKED_BY = "update " + Type.TEMPLATE.getVersionTableName() + " set locked_by=? where locked_by  = ?";
 
+    public static final String GET_PAGES_BY_TEMPLATE_ID = " SELECT c.identifier, c.variant_id as variant FROM contentlet c WHERE EXISTS " +
+            "(SELECT 1 FROM structure s WHERE s.inode = c.structure_inode AND s.structuretype = '5') AND c.contentlet_as_json->'fields'->'template'->>'value' = ? ";
+
 }

dotCMS/src/main/java/com/dotmarketing/startup/runonce/Task241014AddTemplateValueOnContentletIndex.java

@@ -0,0 +1,23 @@
+package com.dotmarketing.startup.runonce;
+
+import com.dotmarketing.common.db.DotConnect;
+import com.dotmarketing.exception.DotDataException;
+import com.dotmarketing.exception.DotRuntimeException;
+import com.dotmarketing.startup.StartupTask;
+
+/**
+ * This upgrade task adds an index on the template value of a contentlet. This
+ * GREATLY speeds up queries.
+ */
+public class Task241014AddTemplateValueOnContentletIndex implements StartupTask {
+
+    public boolean forceRun() {
+        return true;
+    }
+
+    @Override
+    public void executeUpgrade() throws DotDataException, DotRuntimeException {
+        new DotConnect().setSQL("CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_contentlet_template_value ON contentlet((contentlet_as_json->'fields'->'template'->>'value'))").loadResult();
+
+    }
+}

dotCMS/src/main/java/com/dotmarketing/util/TaskLocatorUtil.java

@@ -348,8 +348,9 @@ public static List<Class<?>> getStartupRunOnceTaskClasses() {
 		.add(Task240513UpdateContentTypesSystemField.class)
 		.add(Task240530AddDotAIPortletToLayout.class)
 		.add(Task240606AddVariableColumnToWorkflow.class)
-		.add(Task241013RemoveFullPathLcColumnFromIdentifier.class)
 		.add(Task241009CreatePostgresJobQueueTables.class)
+		.add(Task241013RemoveFullPathLcColumnFromIdentifier.class)
+		.add(Task241014AddTemplateValueOnContentletIndex.class)
 		.build();
         return ret.stream().sorted(classNameComparator).collect(Collectors.toList());
 	}

dotCMS/src/main/resources/postgres.sql

@@ -2353,6 +2353,7 @@ create index containers_ident on dot_containers (identifier);
 create index template_ident on template (identifier);
 create index contentlet_moduser on contentlet (mod_user);
 create index contentlet_lang on contentlet (language_id);
+CREATE INDEX CONCURRENTLY idx_contentlet_template_value ON contentlet((contentlet_as_json->'fields'->'template'->>'value'));
 -- end of fk indicies --
 
 -- Notifications Table

dotcms-integration/src/test/java/com/dotcms/MainSuite2b.java

@@ -385,7 +385,8 @@
         SimpleInjectionIT.class,
         LegacyJSONObjectRenderTest.class,
         Task241013RemoveFullPathLcColumnFromIdentifierTest.class,
-        Task241009CreatePostgresJobQueueTablesTest.class
+        Task241009CreatePostgresJobQueueTablesTest.class,
+        Task241013RemoveFullPathLcColumnFromIdentifierTest.class
 })
 
 public class MainSuite2b {

dotcms-integration/src/test/java/com/dotmarketing/portlets/templates/business/TemplateAPITest.java

@@ -1285,8 +1285,6 @@ public void justOnePageVersion() throws DotDataException, DotSecurityException {
 
         assertFalse(pages.isEmpty());
         assertEquals(1, pages.size());
-        assertEquals(htmlPageAsset.getInode(), pages.get(0).getInode());
-        assertEquals(htmlPageAsset.getLanguageId(), pages.get(0).getLanguageId());
         assertEquals(htmlPageAsset.getVariantId(), pages.get(0).getVariantName());
         assertEquals(htmlPageAsset.getIdentifier(), pages.get(0).getIdentifier());
     }
@@ -1386,11 +1384,11 @@ private void checkFor(final List<HTMLPageVersion> pagesTemplate, final HTMLPageA
         assertFalse(pagesTemplate.isEmpty());
         assertEquals(htmlPageAssets.length, pagesTemplate.size());
 
-        final List<String> inodesFromTemplate = pagesTemplate.stream()
-                .map(pageVersion -> pageVersion.getInode()).collect(Collectors.toList());
+        final List<String> idsFromTemplate = pagesTemplate.stream()
+                .map(pageVersion -> pageVersion.getIdentifier()).collect(Collectors.toList());
 
         for (final HTMLPageAsset htmlPageAsset : htmlPageAssets) {
-            assertTrue(inodesFromTemplate.contains(htmlPageAsset.getInode()));
+            assertTrue(idsFromTemplate.contains(htmlPageAsset.getIdentifier()));
         }
     }