From 0b665e2f4cde82e4c3bfa801ba47de597d6348e2 Mon Sep 17 00:00:00 2001 From: "daniel.solis" <2894221+dsolistorres@users.noreply.github.com> Date: Thu, 22 Aug 2024 08:23:01 -0600 Subject: [PATCH] fix(hosts) : cache non existing hosts (#27366) (#29390) ### Proposed Changes * Non existing host added to host cache to improve performance ### Checklist - [x] Tests --------- Co-authored-by: erickgonzalez --- .../business/ContentletCacheImpl.java | 2 +- .../contentlet/business/HostAPIImpl.java | 49 +++++++---- .../contentlet/business/HostCache.java | 22 ++++- .../contentlet/business/HostCacheImpl.java | 82 ++++++++++++++++--- .../contentlet/business/HostFactoryImpl.java | 28 +++++-- .../business/HostFactoryImplTest.java | 50 ++++++++--- 6 files changed, 188 insertions(+), 45 deletions(-) diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/ContentletCacheImpl.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/ContentletCacheImpl.java index 25abb2a82089..a53a6a6f3b8b 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/ContentletCacheImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/ContentletCacheImpl.java @@ -135,7 +135,7 @@ public void remove(final String key){ Logger.debug(this, "Cache not able to be removed", e); } - final Host host = CacheLocator.getHostCache().get(key); + final Host host = CacheLocator.getHostCache().getById(key); if(host != null){ CacheLocator.getHostCache().remove(host); } diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostAPIImpl.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostAPIImpl.java index ffa71a1a0709..504083e15fbe 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostAPIImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostAPIImpl.java @@ -121,11 +121,15 @@ public Host findDefaultHost(User user, boolean respectFrontendRoles) throws DotS @Override @CloseDBIfOpened public Host resolveHostName(String serverName, User user, boolean respectFrontendRoles) throws DotDataException, DotSecurityException { - Host host = hostCache.getHostByAlias(serverName); - User systemUser = APILocator.systemUser(); - - if(host == null){ - + Host host; + final Host cachedHostByAlias = hostCache.getHostByAlias(serverName); + if (UtilMethods.isSet(() -> cachedHostByAlias.getIdentifier())) { + if (HostCache.CACHE_404_HOST.equals(cachedHostByAlias.getIdentifier())) { + return null; + } + host = cachedHostByAlias; + } else { + final User systemUser = APILocator.systemUser(); try { final Optional optional = resolveHostNameWithoutDefault(serverName, systemUser, respectFrontendRoles); host = optional.isPresent() ? optional.get() : findDefaultHost(systemUser, respectFrontendRoles); @@ -133,12 +137,15 @@ public Host resolveHostName(String serverName, User user, boolean respectFronten host = findDefaultHost(systemUser, respectFrontendRoles); } - if(host != null){ + if (host != null) { hostCache.addHostAlias(serverName, host); + } else { + hostCache.addHostAlias(serverName, HostCache.cache404Contentlet); } } - - checkSitePermission(user, respectFrontendRoles, host); + if (host != null) { + checkSitePermission(user, respectFrontendRoles, host); + } return host; } @@ -146,17 +153,24 @@ public Host resolveHostName(String serverName, User user, boolean respectFronten @CloseDBIfOpened public Optional resolveHostNameWithoutDefault(String serverName, User user, boolean respectFrontendRoles) throws DotDataException, DotSecurityException { - Host host = hostCache.getHostByAlias(serverName); - User systemUser = APILocator.systemUser(); - - if(host == null){ + Host host; + final Host cachedHostByAlias = hostCache.getHostByAlias(serverName); + if (UtilMethods.isSet(() -> cachedHostByAlias.getIdentifier())) { + if (HostCache.CACHE_404_HOST.equals(cachedHostByAlias.getIdentifier())) { + return Optional.empty(); + } + host = cachedHostByAlias; + } else { + User systemUser = APILocator.systemUser(); host = findByNameNotDefault(serverName, systemUser, respectFrontendRoles); if(host == null){ host = findByAlias(serverName, systemUser, respectFrontendRoles); } - if(host != null){ + if (host == null) { + hostCache.addHostAlias(serverName, HostCache.cache404Contentlet); + } else { hostCache.addHostAlias(serverName, host); } } @@ -297,7 +311,14 @@ public Host find(final String id, final User user, return findSystemHost(); } - Host site = hostCache.get(id); + Host site = null; + Host cachedSiteById = hostCache.getById(id); + if (UtilMethods.isSet(() -> cachedSiteById.getIdentifier())) { + if (HostCache.CACHE_404_HOST.equals(cachedSiteById.getIdentifier())) { + return null; + } + site = cachedSiteById; + } if (site == null) { site = DBSearch(id,user,respectFrontendRoles); diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostCache.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostCache.java index a11feef5d0c2..780970871eae 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostCache.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostCache.java @@ -8,10 +8,23 @@ public abstract class HostCache implements Cachable{ protected static String PRIMARY_GROUP = "HostCache"; protected static String ALIAS_GROUP = "HostAliasCache"; - + protected static String NOT_FOUND_BY_ID_GROUP = "Host404ByIdCache"; + protected static String NOT_FOUND_BY_NAME_GROUP = "Host404ByNameCache"; + + public static final String CACHE_404_HOST = "CACHE_404_HOST"; + + protected static final Host cache404Contentlet = new Host() { + @Override + public String getIdentifier() { + return CACHE_404_HOST; + } + }; + abstract protected Host add(Host host); - abstract protected Host get(String key); + abstract protected Host getById(String id); + + abstract protected Host getByName(String name); abstract public void clearCache(); @@ -26,5 +39,10 @@ public abstract class HostCache implements Cachable{ abstract protected Host getHostByAlias(String alias); abstract protected void addHostAlias(String alias, Host host); + + abstract protected void add404HostById(String id); + + abstract protected void add404HostByName(String name); + abstract protected void clearAliasCache() ; } \ No newline at end of file diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostCacheImpl.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostCacheImpl.java index 983678e2d903..38d7d363e2c0 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostCacheImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostCacheImpl.java @@ -23,7 +23,9 @@ public class HostCacheImpl extends HostCache { // region's name for the cache - private String[] groupNames = {PRIMARY_GROUP, ALIAS_GROUP}; + private final String[] groupNames = { + PRIMARY_GROUP, ALIAS_GROUP, NOT_FOUND_BY_ID_GROUP, NOT_FOUND_BY_NAME_GROUP + }; public HostCacheImpl() { cache = CacheLocator.getCacheAdministrator(); @@ -37,6 +39,9 @@ protected Host add(Host host) { String key = host.getIdentifier(); String key2 =host.getHostname(); + cache.remove(key, NOT_FOUND_BY_ID_GROUP); + cache.remove(key2, NOT_FOUND_BY_NAME_GROUP); + // Add the key to the cache cache.put(key, host,PRIMARY_GROUP); cache.put(key2, host,PRIMARY_GROUP); @@ -72,7 +77,10 @@ protected Host getHostByAlias(String key) { Host host = null; try{ String hostId = (String) cache.get(key,ALIAS_GROUP); - host = get(hostId); + if (CACHE_404_HOST.equals(hostId)) { + return cache404Contentlet; + } + host = get(hostId, NOT_FOUND_BY_ID_GROUP); if(host == null){ cache.remove(key, ALIAS_GROUP); } @@ -83,10 +91,17 @@ protected Host getHostByAlias(String key) { return host; } - protected Host get(String key) { + private Host get(String key, String notFoundCacheGroup) { Host host = null; try{ - host = (Host) cache.get(key,PRIMARY_GROUP); + if (UtilMethods.isSet(notFoundCacheGroup)) { + host = (Host) cache.get(key, notFoundCacheGroup); + } + if (host != null && CACHE_404_HOST.equals(host.getIdentifier())) { + return cache404Contentlet; + } else { + host = (Host) cache.get(key, PRIMARY_GROUP); + } }catch (DotCacheException e) { Logger.debug(this, "Cache Entry not found", e); } @@ -94,13 +109,37 @@ protected Host get(String key) { return host; } - /* (non-Javadoc) + /** + * Get a host by id + * @param id the id of the host + * @return the host or 404 if the host is in the not found cache, + * null if the host is not found in the cache + */ + @Override + protected Host getById(final String id) { + return get(id, NOT_FOUND_BY_ID_GROUP); + } + + /** + * Get a host by name + * @param name the name of the host + * @return the host or 404 if the host is in the not found cache, + * null if the host is not found in the cache + */ + @Override + protected Host getByName(final String name) { + return get(name, NOT_FOUND_BY_NAME_GROUP); + } + + /* (non-Javadoc) * @see com.dotmarketing.business.PermissionCache#clearCache() */ public void clearCache() { // clear the cache cache.flushGroup(PRIMARY_GROUP); cache.flushGroup(ALIAS_GROUP); + cache.flushGroup(NOT_FOUND_BY_ID_GROUP); + cache.flushGroup(NOT_FOUND_BY_NAME_GROUP); } /* (non-Javadoc) @@ -114,21 +153,19 @@ protected void remove(Host host){ String _defaultHost =PRIMARY_GROUP +DEFAULT_HOST; cache.remove(_defaultHost,PRIMARY_GROUP); - //remove aliases from host in cache - Host h = get(host.getIdentifier()); - - String key = host.getIdentifier(); String key2 = host.getHostname(); try{ cache.remove(key,PRIMARY_GROUP); + cache.remove(key,NOT_FOUND_BY_ID_GROUP); }catch (Exception e) { Logger.debug(this, "Cache not able to be removed", e); } try{ cache.remove(key2,PRIMARY_GROUP); + cache.remove(key2, NOT_FOUND_BY_NAME_GROUP); }catch (Exception e) { Logger.debug(this, "Cache not able to be removed", e); } @@ -147,7 +184,7 @@ public String getPrimaryGroup() { protected Host getDefaultHost(){ - return get(DEFAULT_HOST); + return get(DEFAULT_HOST, null); } protected void addHostAlias(String alias, Host host){ @@ -155,8 +192,29 @@ protected void addHostAlias(String alias, Host host){ cache.put(alias, host.getIdentifier(),ALIAS_GROUP); } } - - + + /** + * Add the host id to the 404 (not found) cache + * @param id the id of the host + */ + @Override + protected void add404HostById(String id) { + if (id != null) { + cache.put(id, cache404Contentlet, NOT_FOUND_BY_ID_GROUP); + } + } + + /** + * Add the host name to the 404 (not found) cache + * @param name the name of the host + */ + @Override + protected void add404HostByName(String name) { + if (name != null) { + cache.put(name, cache404Contentlet, NOT_FOUND_BY_NAME_GROUP); + } + } + protected void clearAliasCache() { // clear the alias cache cache.flushGroup(ALIAS_GROUP); diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostFactoryImpl.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostFactoryImpl.java index e1901a3a1f23..3c4e7132ad9f 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostFactoryImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/HostFactoryImpl.java @@ -189,8 +189,14 @@ protected ContentletAPI getContentletAPI() { @Override public Host bySiteName(final String siteName) { - Host site = siteCache.get(siteName); - if (null == site || !UtilMethods.isSet(site.getIdentifier())) { + Host site; + final Host cachedSiteByName = siteCache.getByName(siteName);; + if (UtilMethods.isSet(() -> cachedSiteByName.getIdentifier())) { + if (HostCache.CACHE_404_HOST.equals(cachedSiteByName.getIdentifier())) { + return null; + } + site = cachedSiteByName; + } else { final DotConnect dc = new DotConnect(); final StringBuilder sqlQuery = new StringBuilder().append(SELECT_SITE_INODE) .append(WHERE); @@ -200,6 +206,7 @@ public Host bySiteName(final String siteName) { try { final List> dbResults = dc.loadResults(); if (dbResults.isEmpty()) { + siteCache.add404HostByName(siteName); return null; } final String siteInode = dbResults.get(0).get("inode"); @@ -227,8 +234,14 @@ public Host bySiteName(final String siteName) { @Override public Host byAlias(String alias) { - Host site = this.siteCache.getHostByAlias(alias); - if (null == site) { + Host site = null; + Host cachedSiteByAlias = this.siteCache.getHostByAlias(alias); + if (UtilMethods.isSet(() -> cachedSiteByAlias.getIdentifier())) { + if (HostCache.CACHE_404_HOST.equals(cachedSiteByAlias.getIdentifier())) { + return null; + } + site = cachedSiteByAlias; + } else { final DotConnect dc = new DotConnect(); final StringBuilder sqlQuery = new StringBuilder().append(SELECT_SITE_INODE_AND_ALIASES) .append(WHERE) @@ -248,6 +261,7 @@ public Host byAlias(String alias) { try { final List> dbResults = dc.loadResults(); if (dbResults.isEmpty()) { + siteCache.addHostAlias(alias, HostCache.cache404Contentlet); return null; } if (dbResults.size() == 1) { @@ -335,7 +349,7 @@ public List findAll(final int limit, final int offset, final String orderB @Override public Host findSystemHost(final User user, final boolean respectFrontendRoles) throws DotDataException, DotSecurityException { - Host systemHost = this.siteCache.get(Host.SYSTEM_HOST); + Host systemHost = this.siteCache.getById(Host.SYSTEM_HOST); if (null != systemHost) { return systemHost; } @@ -409,6 +423,10 @@ public Host DBSearch(final String id, final boolean respectFrontendRoles) throws this.siteCache.add(site); } } + if (null == site && !Host.SYSTEM_HOST.equals(id)) { + this.siteCache.add404HostById(id); + Logger.warn(HostAPIImpl.class, String.format("Site with id '%s' not found", id)); + } return site; } diff --git a/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/HostFactoryImplTest.java b/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/HostFactoryImplTest.java index d0ccdb13ae5d..de7eb7ba4c6e 100644 --- a/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/HostFactoryImplTest.java +++ b/dotcms-integration/src/test/java/com/dotmarketing/portlets/contentlet/business/HostFactoryImplTest.java @@ -1,36 +1,28 @@ package com.dotmarketing.portlets.contentlet.business; import com.dotcms.IntegrationTestBase; -import com.dotcms.LicenseTestUtil; import com.dotcms.datagen.SiteDataGen; import com.dotcms.datagen.UserDataGen; import com.dotcms.util.IntegrationTestInitService; -import com.dotcms.util.pagination.OrderDirection; import com.dotmarketing.beans.Host; import com.dotmarketing.business.APILocator; import com.dotmarketing.beans.Permission; import com.dotmarketing.business.*; -import com.dotmarketing.db.HibernateUtil; import com.dotmarketing.exception.DotDataException; -import com.dotmarketing.exception.DotHibernateException; import com.dotmarketing.exception.DotSecurityException; -import com.dotmarketing.init.DotInitScheduler; -import com.dotmarketing.portlets.contentlet.model.IndexPolicy; -import com.dotmarketing.util.Logger; -import com.dotmarketing.util.PaginatedArrayList; import com.liferay.portal.model.User; -import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; import static com.dotmarketing.portlets.contentlet.business.HostFactoryImpl.SITE_IS_LIVE_OR_STOPPED; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.Optional; -import java.util.concurrent.ExecutionException; public class HostFactoryImplTest extends IntegrationTestBase { @@ -161,6 +153,42 @@ public void test_search_shouldIncludeSystemHost() throws DotDataException, DotSe //now the user has permissions over the system host assertTrue(hostsList.get().contains(APILocator.systemHost())); } + + /** + * Method to test: {@link HostFactoryImpl#bySiteName(String)} + * Given Scenario: get host by name for a non-existing host + * ExpectedResult: host cache should return 404 for not existing host by name + */ + @Test + public void test_get_404_for_not_existing_host_by_name() throws DotDataException, DotSecurityException { + + Host site = null; + final HostFactoryImpl hostFactory = new HostFactoryImpl(); + + // Create and remove test host + final String hostName = "NotExistingSite" + System.currentTimeMillis(); + try { + site = new SiteDataGen().name(hostName).nextPersisted(true); + final Host hostByName = APILocator.getHostAPI().findByName( + hostName, APILocator.systemUser(), false); + assertNotNull(hostByName); + assertNotEquals(HostCache.CACHE_404_HOST, hostByName.getIdentifier()); + } finally { + if (site != null) { + APILocator.getHostAPI().archive(site, APILocator.systemUser(), false); + APILocator.getHostAPI().delete(site, APILocator.systemUser(), false); + } + } + + // Check 404 after deletion of test host + final HostCache hostCache = new HostCacheImpl(); + final Host nonExistingHost = hostFactory.bySiteName(hostName); + final Host cached404Host = hostCache.getByName(hostName); + + assertNull(nonExistingHost); + assertNotNull(cached404Host); + assertEquals(HostCache.CACHE_404_HOST, cached404Host.getIdentifier()); + } }