Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
81 changes: 65 additions & 16 deletions dotCMS/src/main/java/com/dotmarketing/cms/urlmap/URLMapAPIImpl.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.dotmarketing.cms.urlmap;

import com.dotcms.api.web.HttpServletRequestThreadLocal;
import com.dotcms.content.elasticsearch.constants.ESMappingConstants;
import com.dotcms.content.elasticsearch.util.ESUtils;
import com.dotcms.contenttype.business.ContentTypeAPI;
Expand Down Expand Up @@ -28,6 +29,7 @@
import io.vavr.control.Try;
import org.jetbrains.annotations.NotNull;

import javax.servlet.http.HttpServletRequest;
import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.nio.file.PathMatcher;
Expand All @@ -43,6 +45,12 @@ public class URLMapAPIImpl implements URLMapAPI {
private final PermissionAPI permissionAPI = APILocator.getPermissionAPI();
private final IdentifierAPI identifierAPI = APILocator.getIdentifierAPI();
private final ContentTypeAPI typeAPI = APILocator.getContentTypeAPI(APILocator.systemUser());

/** Request-attribute key prefix for caching the resolved contentlet within a single request. */
private static final String REQUEST_CACHE_KEY = URLMapAPIImpl.class.getName() + ".contentlet:";
/** Sentinel stored in the request cache to represent a "not found" result without using null. */
private static final Contentlet CONTENTLET_NOT_FOUND = new Contentlet();

private static final Lazy<PathMatcher[]> ignorePaths = Lazy.of(() -> {
String[] patterns = Config.getStringArrayProperty("urlmap.ignore.glob.patterns", new String[]{"/application/**", "/api/**", "/dA/**", "/dotAdmin/**", "/html/**"});
PathMatcher[] paths = new PathMatcher[patterns.length];
Expand Down Expand Up @@ -87,6 +95,23 @@ public Optional<URLMapInfo> processURLMap(final UrlMapContext context)
*/
private Contentlet getContentlet(final UrlMapContext urlMapContext) throws DotSecurityException {

// isUrlPattern() and processURLMap() are both called on the same HTTP request.
// Cache the resolved Contentlet in request scope so the second call reuses the first result
// (each call issues up to 2 ES queries with the cross-site fallback in place).
final String cacheKey = REQUEST_CACHE_KEY
+ urlMapContext.getUri() + "|"
+ urlMapContext.getHost().getIdentifier() + "|"
+ urlMapContext.getLanguageId() + "|"
+ urlMapContext.getMode().name();

final HttpServletRequest request = HttpServletRequestThreadLocal.INSTANCE.getRequest();
if (request != null) {
final Object cached = request.getAttribute(cacheKey);
if (cached != null) {
return cached == CONTENTLET_NOT_FOUND ? null : (Contentlet) cached;
}
}

Contentlet matchingContentlet = null;

try {
Expand All @@ -112,6 +137,10 @@ private Contentlet getContentlet(final UrlMapContext urlMapContext) throws DotSe
return null;
}

if (request != null) {
request.setAttribute(cacheKey, matchingContentlet != null ? matchingContentlet : CONTENTLET_NOT_FOUND);
}

return matchingContentlet;
}

Expand All @@ -138,10 +167,13 @@ private Optional<Identifier> getDetailPageUri(final ContentType contentType, Hos
// look for it on the current host
final Identifier myHostIdentifier = this.identifierAPI.find(currentHost, identifier.getPath());
if (myHostIdentifier == null || !UtilMethods.isSet(myHostIdentifier.getId())) {
// No page at the same path on the current site — fall back to the configured
// detail page identifier (e.g. a shared page on a global host).
Logger.info(this.getClass(),
"No valid detail page for Content Type '" + contentType.name()
+ "'. Looking for a detail page=" + identifier.getPath() + " on Site " + currentHost.getHostname());
return Optional.empty();
"No detail page found at path '" + identifier.getPath() + "' on Site '"
+ currentHost.getHostname() + "'. Falling back to configured detail page for Content Type '"
+ contentType.name() + "'.");
return Optional.of(identifier);
}

return Optional.of(myHostIdentifier);
Expand Down Expand Up @@ -266,9 +298,20 @@ private Contentlet getContentlet(

Contentlet contentlet = null;

final String query = this.buildContentQuery(matches, contentType, context);
final List<Contentlet> contentletSearches =
ContentUtils.pull(query, 0, 2, "score", this.wuserAPI.getSystemUser(), true);
// First search restricted to current host (and SYSTEM_HOST). If the content lives on a
// different site but is referenced from this site's pages (cross-site URL map scenario),
// the host-restricted query returns nothing. In that case, fall back to a site-agnostic
// query so the content can still be found and rendered against the current site's detail page.
List<Contentlet> contentletSearches =
ContentUtils.pull(this.buildContentQuery(matches, contentType, context, true), 0, 2, "score", this.wuserAPI.getSystemUser(), true);

if (contentletSearches.isEmpty()) {
Logger.debug(this.getClass(), String.format(
"No URL-mapped contentlet found on current site '%s'. Retrying without host restriction.",
context.getHost().getHostname().replaceAll("[\\r\\n\\t]", "_")));
contentletSearches =
ContentUtils.pull(this.buildContentQuery(matches, contentType, context, false), 0, 2, "score", this.wuserAPI.getSystemUser(), true);
}

if (!contentletSearches.isEmpty()) {

Expand Down Expand Up @@ -332,28 +375,34 @@ private void checkContentPermission(final UrlMapContext context, final Contentle
* Builds the Lucene query used to find the specific {@link Contentlet} that matches a given URL Map for a
* Content Type.
*
* @param matches The set of URL Maps that match a specific Content Type.
* @param contentType The Content Type that matches the URL Map.
* @param context The instance of the URL Map Context.
* @param matches The set of URL Maps that match a specific Content Type.
* @param contentType The Content Type that matches the URL Map.
* @param context The instance of the URL Map Context.
* @param restrictToHost When {@code true}, limits results to the current site and SYSTEM_HOST.
* Pass {@code false} for a cross-site fallback that searches all sites.
* @return The Lucene query that will return a potential match for the URL Map.
*/
private String buildContentQuery(
final Matches matches,
final ContentType contentType,
final UrlMapContext context) {
final UrlMapContext context,
final boolean restrictToHost) {

final StringBuilder query = new StringBuilder();

query.append("+contentType:")
.append(contentType.variable())
.append(" +" + ESMappingConstants.VARIANT + ":")
.append(VariantAPI.DEFAULT_VARIANT.name())
.append(" +deleted:false ")
.append(" +(conhost:")
.append(context.getHost().getIdentifier())
.append(" OR conhost:")
.append(Host.SYSTEM_HOST)
.append(")");
.append(" +deleted:false ");

if (restrictToHost) {
query.append(" +(conhost:")
.append(context.getHost().getIdentifier())
.append(" OR conhost:")
.append(Host.SYSTEM_HOST)
.append(")");
}
if (context.getMode().showLive) {
query.append(" +live:true ");
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ public class PageDetailCollectorTest extends IntegrationTestBase {

private static final String PARENT_FOLDER_1_NAME = "news";
private static final String TEST_URL_MAP_PAGE_NAME = "news-detail";
private static final String TEST_PATTERN = "/testpattern/";
private static final String TEST_URL_MAP_DETAIL_PAGE_URL = TEST_PATTERN + "mynews";

private static Host testSite = null;

Expand All @@ -74,17 +72,21 @@ public static void prepare() throws Exception {
*/
@Test
public void testPageDetailCollector() throws DotDataException, UnknownHostException, DotSecurityException {
final String uniqueSuffix = String.valueOf(System.nanoTime());
final String urlTitle = "mynews-" + uniqueSuffix;
final String testPattern = "/page-detail-collector-" + uniqueSuffix + "/";
final String testUrlMapDetailPageUrl = testPattern + urlTitle;

final HttpServletResponse response = mock(HttpServletResponse.class);
final String requestId = UUIDUtil.uuid();
final HttpServletRequest request = Util.mockHttpRequestObj(response,
TEST_URL_MAP_DETAIL_PAGE_URL, requestId,
testUrlMapDetailPageUrl, requestId,
APILocator.getUserAPI().getAnonymousUser());

final HTMLPageAsset testDetailPage = Util.createTestHTMLPage(testSite,
TEST_URL_MAP_PAGE_NAME, PARENT_FOLDER_1_NAME);

final String urlTitle = "mynews";
final String urlMapPatternToUse = TEST_PATTERN + "{urlTitle}";
final String urlMapPatternToUse = testPattern + "{urlTitle}";
final Language language = APILocator.getLanguageAPI().getDefaultLanguage();
final long langId = language.getId();

Expand All @@ -107,11 +109,11 @@ public void testPageDetailCollector() throws DotDataException, UnknownHostExcept
Collector.SITE_NAME, testSite.getHostname(),
Collector.SITE_ID, testSite.getIdentifier(),
Collector.LANGUAGE, language.getIsoCode(),
Collector.URL, TEST_URL_MAP_DETAIL_PAGE_URL,
Collector.URL, testUrlMapDetailPageUrl,
Collector.OBJECT, Map.of(
Collector.ID, testDetailPage.getIdentifier(),
Collector.TITLE, testDetailPage.getTitle(),
Collector.URL, TEST_URL_MAP_DETAIL_PAGE_URL,
Collector.URL, testUrlMapDetailPageUrl,
Collector.CONTENT_TYPE_ID, testDetailPage.getContentTypeId(),
Collector.CONTENT_TYPE_NAME, testDetailPage.getContentType().name(),
Collector.CONTENT_TYPE_VAR_NAME, testDetailPage.getContentType().variable(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ public class PagesCollectorTest extends IntegrationTestBase {

private static final String PARENT_FOLDER_1_NAME = "news";
private static final String TEST_URL_MAP_PAGE_NAME = "news-detail";
private static final String URL_MAP_PATTERN = "/testpattern/";
private static final String TEST_URL_MAP_DETAIL_PAGE_URL = URL_MAP_PATTERN + "mynews";

private static Host testSite = null;

Expand Down Expand Up @@ -125,17 +123,21 @@ public void collectPageData() throws Exception {
*/
@Test
public void collectUrlMapPageData() throws Exception {
final String uniqueSuffix = String.valueOf(System.nanoTime());
final String urlTitle = "mynews-" + uniqueSuffix;
final String urlMapPatternPrefix = "/pages-collector-" + uniqueSuffix + "/";
final String testUrlMapDetailPageUrl = urlMapPatternPrefix + urlTitle;

final HttpServletResponse response = mock(HttpServletResponse.class);
final String requestId = UUIDUtil.uuid();
final HttpServletRequest request = Util.mockHttpRequestObj(response,
TEST_URL_MAP_DETAIL_PAGE_URL, requestId,
testUrlMapDetailPageUrl, requestId,
APILocator.getUserAPI().getAnonymousUser());

final HTMLPageAsset testDetailPage = Util.createTestHTMLPage(testSite,
TEST_URL_MAP_PAGE_NAME, PARENT_FOLDER_1_NAME);

final String urlTitle = "mynews";
final String urlMapPatternToUse = URL_MAP_PATTERN + "{urlTitle}";
final String urlMapPatternToUse = urlMapPatternPrefix + "{urlTitle}";
final long langId = APILocator.getLanguageAPI().getDefaultLanguage().getId();

final ContentType urlMappedContentType = Util.getUrlMapLikeContentType(
Expand All @@ -159,11 +161,11 @@ public void collectUrlMapPageData() throws Exception {
Collector.EVENT_TYPE, EventType.URL_MAP.getType(),
Collector.SITE_NAME, testSite.getHostname(),
Collector.LANGUAGE, APILocator.getLanguageAPI().getDefaultLanguage().getIsoCode(),
Collector.URL, TEST_URL_MAP_DETAIL_PAGE_URL,
Collector.URL, testUrlMapDetailPageUrl,
Collector.OBJECT, Map.of(
Collector.ID, newsTestContent.getIdentifier(),
Collector.TITLE, urlTitle,
Collector.URL, TEST_URL_MAP_DETAIL_PAGE_URL,
Collector.TITLE, newsTestContent.getTitle(),
Collector.URL, testUrlMapDetailPageUrl,
Collector.CONTENT_TYPE_ID, urlMappedContentType.id(),
Collector.CONTENT_TYPE_NAME, urlMappedContentType.name(),
Collector.CONTENT_TYPE_VAR_NAME, urlMappedContentType.variable(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ public class WebEventsCollectorServiceImplTest extends IntegrationTestBase {
private static final String TEST_PAGE_NAME = "index";
private static final String TEST_PAGE_URL = "/" + TEST_PAGE_NAME;
private static final String TEST_URL_MAP_PAGE_NAME = "news-detail";
private static final String TEST_PATTERN = "/testpattern/";
private static final String TEST_URL_MAP_DETAIL_PAGE_URL = TEST_PATTERN + "mynews";
private static final String URI = "/my-test/vanity-url";

private static final String CLIENT_ID = "analytics-customer-customer1";
Expand Down Expand Up @@ -232,10 +230,14 @@ public void testPagesCollector() throws DotDataException, IOException, DotSecuri
*/
@Test
public void testPageDetailCollector() throws Exception {
final String uniqueSuffix = String.valueOf(System.nanoTime());
final String urlTitle = "mynews-" + uniqueSuffix;
final String testPattern = "/web-events-page-detail-" + uniqueSuffix + "/";
final String testUrlMapDetailPageUrl = testPattern + urlTitle;

testDetailPage = null != testDetailPage ? testDetailPage : Util.createTestHTMLPage(testSite, TEST_URL_MAP_PAGE_NAME, PARENT_FOLDER_1_NAME);

final String urlTitle = "mynews";
final String urlMapPatternToUse = TEST_PATTERN + "{urlTitle}";
final String urlMapPatternToUse = testPattern + "{urlTitle}";
final Language language = APILocator.getLanguageAPI().getDefaultLanguage();
final long langId = language.getId();

Expand All @@ -257,11 +259,11 @@ public void testPageDetailCollector() throws Exception {
Collector.EVENT_TYPE, EventType.PAGE_REQUEST.getType(),
Collector.SITE_NAME, testSite.getHostname(),
Collector.LANGUAGE, language.getIsoCode(),
Collector.URL, TEST_URL_MAP_DETAIL_PAGE_URL,
Collector.URL, testUrlMapDetailPageUrl,
Collector.OBJECT, Map.of(
Collector.ID, testDetailPage.getIdentifier(),
Collector.TITLE, testDetailPage.getTitle(),
Collector.URL, TEST_URL_MAP_DETAIL_PAGE_URL,
Collector.URL, testUrlMapDetailPageUrl,
Collector.CONTENT_TYPE_ID, testDetailPage.getContentTypeId(),
Collector.CONTENT_TYPE_NAME, testDetailPage.getContentType().name(),
Collector.CONTENT_TYPE_VAR_NAME, testDetailPage.getContentType().variable(),
Expand All @@ -284,7 +286,7 @@ public void testPageDetailCollector() throws Exception {
final Map<String, Object> requestParams = Map.of(
"host_id", testSite.getIdentifier()
);
final HttpServletRequest request = Util.mockHttpRequestObj(response, TEST_URL_MAP_DETAIL_PAGE_URL,
final HttpServletRequest request = Util.mockHttpRequestObj(response, testUrlMapDetailPageUrl,
UUIDUtil.uuid(), APILocator.getUserAPI().getAnonymousUser(), null, requestParams);

final RequestMatcher requestMatcher = new PagesAndUrlMapsRequestMatcher();
Expand Down
Loading
Loading