Skip to content

Commit 3747b39

Browse files
dsilvamDaniel Silvaclaude
authored
fix(content-type): accept custom Page content types as valid detail pages (#35175)
## Summary - Replaces hardcoded `"htmlpageasset"` string check in `DetailPageTransformerImpl.validateIdentifier()` with a proper `BaseContentType.HTMLPAGE` base-type lookup - Any content type of base type `Page` (regardless of velocity variable name) is now accepted as a valid detail page - Existing constructors and callers are unaffected; method signature updated to declare `DotDataException` / `DotSecurityException` ## Root cause `assetSubType` on an `Identifier` is populated from the content type's velocity variable name. Only the built-in "Web Page Content" type uses `htmlpageasset` — all custom Page content types have different variable names and were always rejected by the hardcoded check. ## Test plan - [ ] `DetailPageTransformerImplTest#uriToId_withStandardHtmlpageasset_returnsIdentifier` — regression guard - [ ] `DetailPageTransformerImplTest#uriToId_withCustomPageContentType_returnsIdentifier` — bug-fix scenario - [ ] `DetailPageTransformerImplTest#uriToId_withNonPageIdentifier_throwsIllegalArgumentException` — negative case ```bash ./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false -Dit.test=DetailPageTransformerImplTest ``` Closes #35149 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Daniel Silva <[email protected]> Co-authored-by: Claude Sonnet 4.6 <[email protected]>
1 parent 2102f1b commit 3747b39

File tree

2 files changed

+158
-16
lines changed

2 files changed

+158
-16
lines changed

dotCMS/src/main/java/com/dotcms/contenttype/transform/contenttype/DetailPageTransformerImpl.java

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.dotcms.contenttype.transform.contenttype;
22

33
import com.dotcms.api.web.HttpServletRequestThreadLocal;
4+
import com.dotcms.contenttype.model.type.BaseContentType;
45
import com.dotcms.contenttype.model.type.ContentType;
56
import com.dotcms.rest.api.v1.contenttype.ContentTypeHelper;
67
import com.dotmarketing.beans.Host;
@@ -20,7 +21,6 @@
2021

2122
public class DetailPageTransformerImpl implements DetailPageTransformer {
2223

23-
private static final String PAGE_SUBTYPE = "htmlpageasset";
2424

2525
private final ContentType contentType;
2626
private final User user;
@@ -123,25 +123,26 @@ public Optional<String> idToUri() throws DotDataException, DotSecurityException
123123
* @throws IllegalArgumentException if the identifier is invalid.
124124
*/
125125
private Optional<String> validateIdentifier(
126-
String detailPage, Identifier foundDetailPageIdentifier) {
127-
128-
if (null != foundDetailPageIdentifier &&
129-
foundDetailPageIdentifier.exists() &&
130-
foundDetailPageIdentifier.getAssetSubType().equals(PAGE_SUBTYPE)) {
131-
return Optional.of(foundDetailPageIdentifier.getId());
132-
} else {
133-
if (null != foundDetailPageIdentifier &&
134-
foundDetailPageIdentifier.exists() &&
135-
!foundDetailPageIdentifier.getAssetSubType().equals(PAGE_SUBTYPE)) {
136-
throw new IllegalArgumentException(
137-
String.format("[%s] in Content Type [%s] is not a valid detail page.",
138-
detailPage, contentType.name()));
126+
String detailPage, Identifier foundDetailPageIdentifier)
127+
throws DotDataException, DotSecurityException {
128+
129+
if (null != foundDetailPageIdentifier && foundDetailPageIdentifier.exists()) {
130+
final String assetSubType = foundDetailPageIdentifier.getAssetSubType();
131+
if (UtilMethods.isSet(assetSubType)) {
132+
final ContentType detailPageType = APILocator.getContentTypeAPI(
133+
APILocator.getUserAPI().getSystemUser()).find(assetSubType);
134+
if (null != detailPageType && BaseContentType.HTMLPAGE == detailPageType.baseType()) {
135+
return Optional.of(foundDetailPageIdentifier.getId());
136+
}
139137
}
140-
141138
throw new IllegalArgumentException(
142-
String.format("Detail page [%s] in Content Type [%s] does not exist.",
139+
String.format("[%s] in Content Type [%s] is not a valid detail page.",
143140
detailPage, contentType.name()));
144141
}
142+
143+
throw new IllegalArgumentException(
144+
String.format("Detail page [%s] in Content Type [%s] does not exist.",
145+
detailPage, contentType.name()));
145146
}
146147

147148
}
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package com.dotcms.contenttype.transform.contenttype;
2+
3+
import com.dotcms.IntegrationTestBase;
4+
import com.dotcms.contenttype.model.type.BaseContentType;
5+
import com.dotcms.contenttype.model.type.ContentType;
6+
import com.dotcms.datagen.ContentletDataGen;
7+
import com.dotcms.datagen.ContentTypeDataGen;
8+
import com.dotcms.datagen.HTMLPageDataGen;
9+
import com.dotcms.datagen.SiteDataGen;
10+
import com.dotcms.datagen.TemplateDataGen;
11+
import com.dotcms.util.IntegrationTestInitService;
12+
import com.dotmarketing.beans.Host;
13+
import com.dotmarketing.business.APILocator;
14+
import com.dotmarketing.portlets.contentlet.model.Contentlet;
15+
import com.dotmarketing.portlets.htmlpageasset.business.HTMLPageAssetAPI;
16+
import com.dotmarketing.portlets.htmlpageasset.model.HTMLPageAsset;
17+
import com.dotmarketing.portlets.templates.model.Template;
18+
import com.liferay.portal.model.User;
19+
import org.junit.Assert;
20+
import org.junit.BeforeClass;
21+
import org.junit.Test;
22+
23+
import java.util.Optional;
24+
25+
/**
26+
* Integration tests for {@link DetailPageTransformerImpl}.
27+
*
28+
* <p>Specifically verifies that {@link DetailPageTransformerImpl#uriToId()} accepts pages of any
29+
* content type whose base type is {@link BaseContentType#HTMLPAGE}, not just the default
30+
* {@code htmlpageasset} type.
31+
*/
32+
public class DetailPageTransformerImplTest extends IntegrationTestBase {
33+
34+
private static User user;
35+
private static Host host;
36+
private static Template template;
37+
38+
@BeforeClass
39+
public static void prepare() throws Exception {
40+
IntegrationTestInitService.getInstance().init();
41+
user = APILocator.systemUser();
42+
host = new SiteDataGen().nextPersisted();
43+
template = new TemplateDataGen().host(host).nextPersisted();
44+
}
45+
46+
/**
47+
* Method to test: {@link DetailPageTransformerImpl#uriToId()}
48+
* Given: A ContentType whose detailPage is the identifier of a standard {@code htmlpageasset} page
49+
* When: {@code uriToId()} is called
50+
* Should: Return the page identifier (regression guard — must keep working after the fix)
51+
*/
52+
@Test
53+
public void uriToId_withStandardHtmlpageasset_returnsIdentifier() throws Exception {
54+
final HTMLPageAsset page = new HTMLPageDataGen(host, template).nextPersisted();
55+
final ContentType contentType = new ContentTypeDataGen()
56+
.detailPage(page.getIdentifier())
57+
.urlMapPattern("/test-standard-{urlTitle}")
58+
.next();
59+
60+
final Optional<String> result = new DetailPageTransformerImpl(contentType, user).uriToId();
61+
62+
Assert.assertTrue("Expected a non-empty result for standard htmlpageasset", result.isPresent());
63+
Assert.assertEquals(page.getIdentifier(), result.get());
64+
}
65+
66+
/**
67+
* Method to test: {@link DetailPageTransformerImpl#uriToId()}
68+
* Given: A ContentType whose detailPage is the identifier of a page using a custom HTMLPAGE content type
69+
* When: {@code uriToId()} is called
70+
* Should: Return the page identifier — custom HTMLPAGE types must be accepted as valid detail pages
71+
*
72+
* <p>This is the bug-fix scenario: before the fix, {@code validateIdentifier()} compared
73+
* {@code assetSubType} against the hardcoded string {@code "htmlpageasset"}, causing any page
74+
* with a custom content type (e.g. {@code landingPage}) to be rejected with
75+
* {@link IllegalArgumentException}.
76+
*/
77+
@Test
78+
public void uriToId_withCustomPageContentType_returnsIdentifier() throws Exception {
79+
final long time = System.currentTimeMillis();
80+
final ContentType customPageType = new ContentTypeDataGen()
81+
.baseContentType(BaseContentType.HTMLPAGE)
82+
.host(host)
83+
.name("CustomPageType_" + time)
84+
.velocityVarName("customPageType" + time)
85+
.nextPersisted();
86+
87+
Contentlet customPage = null;
88+
try {
89+
customPage = new ContentletDataGen(customPageType)
90+
.host(host)
91+
.setProperty(HTMLPageAssetAPI.URL_FIELD, "custom-page-" + time)
92+
.setProperty(HTMLPageAssetAPI.TITLE_FIELD, "Custom Page " + time)
93+
.setProperty(HTMLPageAssetAPI.TEMPLATE_FIELD, template.getIdentifier())
94+
.setProperty(HTMLPageAssetAPI.FRIENDLY_NAME_FIELD, "Custom Page")
95+
.setProperty(HTMLPageAssetAPI.CACHE_TTL_FIELD, "0")
96+
.nextPersisted();
97+
98+
final ContentType contentType = new ContentTypeDataGen()
99+
.detailPage(customPage.getIdentifier())
100+
.urlMapPattern("/test-custom-{urlTitle}")
101+
.next();
102+
103+
final Optional<String> result = new DetailPageTransformerImpl(contentType, user).uriToId();
104+
105+
Assert.assertTrue("Expected a non-empty result for custom HTMLPAGE content type", result.isPresent());
106+
Assert.assertEquals(customPage.getIdentifier(), result.get());
107+
} finally {
108+
if (customPage != null) {
109+
APILocator.getContentletAPI().destroy(customPage, user, false);
110+
}
111+
ContentTypeDataGen.remove(customPageType);
112+
}
113+
}
114+
115+
/**
116+
* Method to test: {@link DetailPageTransformerImpl#uriToId()}
117+
* Given: A ContentType whose detailPage identifier points to a non-page contentlet
118+
* When: {@code uriToId()} is called
119+
* Should: Throw {@link IllegalArgumentException}
120+
*/
121+
@Test(expected = IllegalArgumentException.class)
122+
public void uriToId_withNonPageIdentifier_throwsIllegalArgumentException() throws Exception {
123+
final ContentType simpleType = new ContentTypeDataGen().nextPersisted();
124+
Contentlet contentlet = null;
125+
try {
126+
contentlet = new ContentletDataGen(simpleType).host(host).nextPersisted();
127+
128+
final ContentType contentType = new ContentTypeDataGen()
129+
.detailPage(contentlet.getIdentifier())
130+
.urlMapPattern("/test-nonpage-{urlTitle}")
131+
.next();
132+
133+
new DetailPageTransformerImpl(contentType, user).uriToId();
134+
} finally {
135+
if (contentlet != null) {
136+
APILocator.getContentletAPI().destroy(contentlet, user, false);
137+
}
138+
ContentTypeDataGen.remove(simpleType);
139+
}
140+
}
141+
}

0 commit comments

Comments
 (0)