Skip to content

Commit

Permalink
Issue 23276 urlmapping content detailed page not found (#24822)
Browse files Browse the repository at this point in the history
* #23276 IAm subtypes have been included.

* Avoid the clash between IndexPages and UrlMaps cases.

* We created a IAm subtype enum to manage different page types instead of creating a new IAm type.

* We use tuples to manage isPageAsset and resourceResolveType methods.

* resolvePageAssetSubtype method has been created in order to avoid changes on third classes that use isPageAsset method.

* #23276 Some improvements.

* Replacing new Tuple2<> constructor statements by Tuple.of() method.

* Replacing condition NOT isPresent() by isEmpty method

* #23276 Refactoring resolvePageAssetSubtype

* #23276 Some test have been added to CMSUrlUtil

* resolveResourceType method

* resolvePageAssetSubType method

* #23276 Test documentation has been added.

* #23276 Some unuseful code has been removed.

* #23276 Some unuseful code has been removed.

---------

Co-authored-by: daniel.colina <daniel.colina@dotcms.com>
Co-authored-by: Nollymar Longa <nollymar.longa@dotcms.com>
  • Loading branch information
3 people authored and manuelrojas committed May 26, 2023
1 parent 0bf228e commit 71284ca
Show file tree
Hide file tree
Showing 5 changed files with 372 additions and 49 deletions.
4 changes: 3 additions & 1 deletion dotCMS/src/integration-test/java/com/dotcms/MainSuite.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@
import com.dotmarketing.cache.FolderCacheImplIntegrationTest;
import com.dotmarketing.common.db.DBTimeZoneCheckTest;
import com.dotmarketing.filters.AutoLoginFilterTest;
import com.dotmarketing.filters.CMSUrlUtilTest;
import com.dotmarketing.image.filter.ImageFilterAPIImplTest;
import com.dotmarketing.image.focalpoint.FocalPointAPITest;
import com.dotmarketing.osgi.GenericBundleActivatorTest;
Expand Down Expand Up @@ -638,7 +639,8 @@
Task230426AlterVarcharLengthOfLockedByColTest.class,
// AnalyticsAPIImplTest.class,
// AccessTokenRenewJobTest.class,
AssetPathResolverImplTest.class
AssetPathResolverImplTest.class,
CMSUrlUtilTest.class
})

public class MainSuite {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,293 @@
package com.dotmarketing.filters;

import static com.dotcms.datagen.TestDataUtils.getDotAssetLikeContentlet;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.powermock.api.mockito.PowerMockito.when;

import com.dotcms.LicenseTestUtil;
import com.dotcms.api.web.HttpServletRequestThreadLocal;
import com.dotcms.contenttype.model.field.DataTypes;
import com.dotcms.contenttype.model.field.Field;
import com.dotcms.contenttype.model.type.ContentType;
import com.dotcms.datagen.ContentTypeDataGen;
import com.dotcms.datagen.ContentletDataGen;
import com.dotcms.datagen.FieldDataGen;
import com.dotcms.datagen.FolderDataGen;
import com.dotcms.datagen.HTMLPageDataGen;
import com.dotcms.datagen.SiteDataGen;
import com.dotcms.datagen.TemplateDataGen;
import com.dotcms.util.IntegrationTestInitService;
import com.dotmarketing.beans.Host;
import com.dotmarketing.business.APILocator;
import com.dotmarketing.filters.CMSFilter.IAm;
import com.dotmarketing.filters.CMSFilter.IAmSubType;
import com.dotmarketing.portlets.contentlet.business.ContentletAPI;
import com.dotmarketing.portlets.contentlet.model.Contentlet;
import com.dotmarketing.portlets.folders.model.Folder;
import com.dotmarketing.portlets.htmlpageasset.model.HTMLPageAsset;
import com.dotmarketing.portlets.templates.model.Template;
import com.liferay.portal.model.User;
import io.vavr.Tuple2;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;

public class CMSUrlUtilTest {
private static final String TEST_PATTERN = "/testpattern";
private static ContentletAPI contentletAPI;
private static Host site;
private static User systemUser;
private static long defaultLanguageId;
private CMSUrlUtil cmsUrlUtil;
private static HTMLPageAsset detailPage1;
private static HTMLPageAsset detailPage2;
private static HttpSession session;

@BeforeClass
public static void prepare() throws Exception {
//Setting web app environment
IntegrationTestInitService.getInstance().init();

/* APIs initialization */
contentletAPI = APILocator.getContentletAPI();

/* Default user */
systemUser = APILocator.getUserAPI().getSystemUser();

/* Default variables */
site = new SiteDataGen().nextPersisted();
defaultLanguageId = APILocator.getLanguageAPI().getDefaultLanguage().getId();

final String parent1Name = "news-events";
final Folder parent1 = new FolderDataGen().name(parent1Name).title(parent1Name).site(site)
.nextPersisted();
final String parent2Name = "news";
final Folder parent2 = new FolderDataGen().name(parent2Name).title(parent2Name).parent(parent1)
.nextPersisted();

final String parent3Name = "news-events2";
final Folder parent3 = new FolderDataGen().name(parent3Name).title(parent3Name).site(site)
.nextPersisted();
final String parent4Name = "news2";
final Folder parent4 = new FolderDataGen().name(parent4Name).title(parent4Name).parent(parent3)
.nextPersisted();

final Template template = new TemplateDataGen().nextPersisted();
detailPage1 = new HTMLPageDataGen(parent2, template)
.pageURL("news-detail")
.title("news-detail")
.nextPersisted();

detailPage2 = new HTMLPageDataGen(parent4, template)
.pageURL("index")
.title("index")
.nextPersisted();


final HttpServletRequest request = mock(HttpServletRequest.class);
session = mock(HttpSession.class);

when(request.getSession()).thenReturn(session);
when(request.getSession(false)).thenReturn(session);

HttpServletRequestThreadLocal.INSTANCE.setRequest(request);
}

@Before
public void prepareTest() {
cmsUrlUtil = CMSUrlUtil.getInstance();
}

/**
* methodToTest {@link CMSUrlUtil#resolvePageAssetSubtype(String, Host, Long)}
* Given Scenario: A Content Type with a URLMap pattern and a detail page exists
* ExpectedResult: Should return a {@link Tuple2} object with true and IAmSubType.PAGE_URL_MAP
*/
@Test
public void shouldReturnATupleWithTrueAndPageUrlMap() {

// Given
// Create a Content Type with a URLMap pattern and a detail page
final String newsPatternPrefix =
TEST_PATTERN + System.currentTimeMillis() + "/";

final Field field = new FieldDataGen().dataType(DataTypes.INTEGER).next();
final String urlMapper = newsPatternPrefix + "{" + field.variable() + "}";

final ContentType contentType = new ContentTypeDataGen()
.field(field)
.detailPage(detailPage1.getIdentifier())
.urlMapPattern(urlMapper)
.nextPersisted();

final Contentlet newsTestContent = new ContentletDataGen(contentType.id())
.setProperty(field.variable(), 2)
.languageId(1)
.host(site)
.nextPersisted();

//When
final Tuple2<Boolean, IAmSubType> pageAssetSubtype =
cmsUrlUtil.resolvePageAssetSubtype(
newsPatternPrefix + newsTestContent.getStringProperty(field.variable()),
site, defaultLanguageId);

assertTrue(pageAssetSubtype._1());
assertEquals(IAmSubType.PAGE_URL_MAP, pageAssetSubtype._2());

}

/**
* methodToTest {@link CMSUrlUtil#resolvePageAssetSubtype(String, Host, Long)}
* Given Scenario: A Content Type without a URLMap pattern
* ExpectedResult: Should return a {@link Tuple2} object with false and IAmSubType.NONE
*/
@Test
public void shouldReturnATupleWithFalseAndNone() {
// Given
// Create a Content Type as a Contentlet Type
final String newsPatternPrefix =
TEST_PATTERN + System.currentTimeMillis() + "/";

final Field field = new FieldDataGen().dataType(DataTypes.INTEGER).next();
final ContentType contentType = new ContentTypeDataGen()
.field(field)
.nextPersisted();

final Contentlet newsTestContent = new ContentletDataGen(contentType.id())
.setProperty(field.variable(), 2)
.languageId(1)
.host(site)
.nextPersisted();

// When
final Tuple2<Boolean, IAmSubType> pageAssetSubtype =
cmsUrlUtil.resolvePageAssetSubtype(
newsPatternPrefix,
site, defaultLanguageId);

assertFalse(pageAssetSubtype._1());
assertEquals(IAmSubType.NONE, pageAssetSubtype._2());
}

/**
* methodToTest {@link CMSUrlUtil#resolvePageAssetSubtype(String, Host, Long)}
* Given Scenario: A HtmlPage with a detail page and without a URLMap pattern
* ExpectedResult: Should return a {@link Tuple2} object with true and IAmSubType.NONE
*/
@Test
public void shouldReturnATupleWithTrueAndNone() {
// Given
final String newsPatternPrefix = "/news-events/news/news-detail";
final Contentlet newsTestContent = getDotAssetLikeContentlet();

// When
final Tuple2<Boolean, IAmSubType> pageAssetSubtype =
cmsUrlUtil.resolvePageAssetSubtype(
newsPatternPrefix,
site, defaultLanguageId);

assertTrue(pageAssetSubtype._1());
assertEquals(IAmSubType.NONE, pageAssetSubtype._2());
}

/**
* methodToTest {@link CMSUrlUtil#resolveResourceType(IAm, String, Host, long)}
* Given Scenario: A Content Type with a URLMap pattern and a detail page exists
* ExpectedResult: Should return a {@link Tuple2} object with IAm.PAGE and IAmSubType.PAGE_URL_MAP
*/
@Test
public void whenResolveResourceType_shouldReturnATupleWithPageAndPageUrlMap() {
// Given
// Create a Content Type with a URLMap pattern and a detail page
final String newsPatternPrefix =
TEST_PATTERN + System.currentTimeMillis() + "/";

final Field field = new FieldDataGen().dataType(DataTypes.INTEGER).next();
final String urlMapper = newsPatternPrefix + "{" + field.variable() + "}";

final ContentType contentType = new ContentTypeDataGen()
.field(field)
.detailPage(detailPage1.getIdentifier())
.urlMapPattern(urlMapper)
.nextPersisted();

final Contentlet newsTestContent = new ContentletDataGen(contentType.id())
.setProperty(field.variable(), 2)
.languageId(1)
.host(site)
.nextPersisted();

final Tuple2<IAm, IAmSubType> type = cmsUrlUtil.resolveResourceType(IAm.NOTHING_IN_THE_CMS,
newsPatternPrefix + newsTestContent.getStringProperty(field.variable()),
site, defaultLanguageId);

assertEquals(IAm.PAGE, type._1());
assertEquals(IAmSubType.PAGE_URL_MAP, type._2());

}

/**
* methodToTest {@link CMSUrlUtil#resolveResourceType(IAm, String, Host, long)}
* Given Scenario: A HtmlPage with a detail page and without a URLMap pattern associated
* ExpectedResult: Should return a {@link Tuple2} object with IAm.PAGE and IAmSubType.NONE
*/
@Test
public void whenResolveResourceType_shouldReturnATupleWithPageAndNone() {
// Given
final String newsPatternPrefix = "/news-events/news/news-detail";
final Contentlet newsTestContent = getDotAssetLikeContentlet();

// When
final Tuple2<IAm, IAmSubType> type = cmsUrlUtil.resolveResourceType(IAm.NOTHING_IN_THE_CMS,
newsPatternPrefix,
site, defaultLanguageId);

assertEquals(IAm.PAGE, type._1());
assertEquals(IAmSubType.NONE, type._2());
}

/**
* methodToTest {@link CMSUrlUtil#resolveResourceType(IAm, String, Host, long)}
* Given Scenario: A site with folder structure exists
* ExpectedResult: Should return a {@link Tuple2} object with IAm.FOLDER and IAmSubType.NONE
*/
@Test
public void whenResolveResourceType_shouldReturnATupleWithFolderAndNone() {
// Given
final String newsPatternPrefix = "/news-events/news/";

// When
final Tuple2<IAm, IAmSubType> type = cmsUrlUtil.resolveResourceType(IAm.NOTHING_IN_THE_CMS,
newsPatternPrefix,
site, defaultLanguageId);

assertEquals(IAm.FOLDER, type._1());
assertEquals(IAmSubType.NONE, type._2());
}

/**
* methodToTest {@link CMSUrlUtil#resolveResourceType(IAm, String, Host, long)}
* Given Scenario: A site with folder structure exists and an index page is inside the folder
* ExpectedResult: Should return a {@link Tuple2} object with IAm.PAGE and IAmSubType.PAGE_INDEX
*/
@Test
public void whenResolveResourceType_shouldReturnATupleWithPageAndPageIndex() {
// Given
final String newsPatternPrefix = "/news-events2/news2/";

// When
final Tuple2<IAm, IAmSubType> type = cmsUrlUtil.resolveResourceType(IAm.NOTHING_IN_THE_CMS,
newsPatternPrefix,
site, defaultLanguageId);

assertEquals(IAm.PAGE, type._1());
assertEquals(IAmSubType.PAGE_INDEX, type._2());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ protected boolean checkAccessFilters(final String uri, final Host host,

private boolean isFile(final String uri, final Host host, final long languageId) {

if (CMSFilter.IAm.FILE != this.cmsUrlUtil.resolveResourceType(null, uri, host, languageId)) {
if (CMSFilter.IAm.FILE != this.cmsUrlUtil.resolveResourceType(null, uri, host, languageId)._1()) {

final String uriWithoutQueryString = this.cmsUrlUtil.getUriWithoutQueryString(uri.toLowerCase());
return uriWithoutQueryString.endsWith(".jpg") ||
Expand Down
30 changes: 21 additions & 9 deletions dotCMS/src/main/java/com/dotmarketing/filters/CMSFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.dotmarketing.portlets.rules.business.RulesEngine;
import com.dotmarketing.portlets.rules.model.Rule;
import com.dotmarketing.util.*;
import io.vavr.Tuple2;
import io.vavr.control.Try;
import org.apache.commons.logging.LogFactory;

Expand All @@ -33,10 +34,21 @@ public class CMSFilter implements Filter {
private final String RELATIVE_ASSET_PATH = APILocator.getFileAssetAPI().getRelativeAssetsRootPath();
public static final String CMS_INDEX_PAGE = Config.getStringProperty("CMS_INDEX_PAGE", "index");

/*
* These enums are used to determine what the current request is.
* It is used to determine if the request is a page, a file, a folder, or nothing in the CMS.
* In some cases we have to know if the request is a page or if it is an index page. For example, if the request is
* an UrlMapping and ends with slash, then it needs to be treated as a page and not as an index page. To achieve this
* we use the IAmSubType enum.
* */
public enum IAm {
PAGE, FOLDER, FILE, NOTHING_IN_THE_CMS
}

public enum IAmSubType {
PAGE_INDEX, PAGE_URL_MAP, NONE
}

@Override
public void init(FilterConfig arg0) throws ServletException {

Expand Down Expand Up @@ -91,23 +103,23 @@ private void doFilterInternal(ServletRequest req, ServletResponse res, FilterCha
}


final IAm iAm = this.urlUtil.resolveResourceType(IAm.NOTHING_IN_THE_CMS, uri, site, languageId);
final Tuple2<IAm,IAmSubType> iAm =
this.urlUtil.resolveResourceType(IAm.NOTHING_IN_THE_CMS, uri, site, languageId);

// if I am a folder without a slash
if (iAm == IAm.FOLDER && !uri.endsWith("/")) {
if (iAm._1() == IAm.FOLDER && !uri.endsWith("/")) {
response.setHeader("Location", UtilMethods.isSet(queryString) ? uri + "/?" + queryString : uri + "/");
Try.run(()->response.setStatus(301));
return;
}

// if I am a Page with a slash
if (iAm == IAm.PAGE && uri.endsWith("/")) {
uri = uri + CMS_INDEX_PAGE;

// if I am a Page with a trailing slash
if (iAm._1() == IAm.PAGE && iAm._2() == IAmSubType.PAGE_INDEX && uri.endsWith("/")) {
uri = uri + CMS_INDEX_PAGE;
}


if (iAm == IAm.PAGE) {
if (iAm._1() == IAm.PAGE) {
countPageVisit(request);
countSiteVisit(request, response);
request.setAttribute(Constants.CMS_FILTER_URI_OVERRIDE,
Expand All @@ -116,7 +128,7 @@ private void doFilterInternal(ServletRequest req, ServletResponse res, FilterCha
this.urlUtil.getQueryStringFromUri (uri):queryString;
}

if (iAm == IAm.FILE) {
if (iAm._1() == IAm.FILE) {
Identifier ident;
try {
// Serving the file through the /dotAsset servlet
Expand All @@ -138,7 +150,7 @@ private void doFilterInternal(ServletRequest req, ServletResponse res, FilterCha
return;
}

if (iAm == IAm.PAGE) {
if (iAm._1() == IAm.PAGE) {

final StringWriter forward = new StringWriter().append("/servlets/VelocityServlet");

Expand Down

0 comments on commit 71284ca

Please sign in to comment.