Skip to content

Commit

Permalink
feat(GraphQL) publishDate param to GraphQL Refs:#30780 (#30885)
Browse files Browse the repository at this point in the history
### Proposed Changes
* I'm opening the possibility of passing a publishDate through the GQL
API
* Additionally, I saw an opportunity to improve our passing the request
into a method that uses it. Previously, it was being grabbed by the
current thread. We should avoid using the Current Thread to pass around
the request, as it is the source of memory leaks
  • Loading branch information
fabrizzio-dotCMS authored Dec 16, 2024
1 parent 7503306 commit 82ddbb0
Show file tree
Hide file tree
Showing 20 changed files with 629 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,10 @@ public Contentlet findContentletByIdentifier(final String identifier, final long
final Date timeMachineDate, final User user, final boolean respectFrontendRoles)
throws DotDataException, DotSecurityException, DotContentletStateException{
final Contentlet contentlet = contentFactory.findContentletByIdentifier(identifier, languageId, variantId, timeMachineDate);
if (contentlet == null) {
Logger.debug(this, "Contentlet not found for identifier: " + identifier + " lang:" + languageId + " variant:" + variantId + " date:" + timeMachineDate);
return null;
}
if (permissionAPI.doesUserHavePermission(contentlet, PermissionAPI.PERMISSION_READ, user, respectFrontendRoles)) {
return contentlet;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ public Collection<GraphQLFieldDefinition> getFields() throws DotDataException {
.name("site")
.type(GraphQLString)
.build())
.argument(GraphQLArgument.newArgument() //This is time machine
.name("publishDate")
.type(GraphQLString)
.build())
.type(PageAPIGraphQLTypesProvider.INSTANCE.getTypesMap().get(DOT_PAGE))
.dataFetcher(new PageDataFetcher()).build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import graphql.schema.DataFetcher;
import graphql.schema.DataFetchingEnvironment;
import java.util.List;
import javax.servlet.http.HttpServletRequest;

/**
* This DataFetcher returns the {@link TemplateLayout} associated to the requested {@link HTMLPageAsset}.
Expand All @@ -31,8 +30,6 @@ public List<ContainerRaw> get(final DataFetchingEnvironment environment) throws
final String languageId = (String) context.getParam("languageId");

final PageMode mode = PageMode.get(pageModeAsString);
final HttpServletRequest request = context.getHttpServletRequest();

final HTMLPageAsset pageAsset = APILocator.getHTMLPageAssetAPI()
.fromContentlet(page);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.dotcms.graphql.DotGraphQLContext;
import com.dotcms.graphql.exception.PermissionDeniedGraphQLException;
import com.dotcms.rest.api.v1.page.PageResource;
import com.dotcms.variant.VariantAPI;
import com.dotmarketing.beans.Host;
import com.dotmarketing.business.APILocator;
import com.dotmarketing.exception.DotSecurityException;
Expand All @@ -15,13 +17,17 @@
import com.dotmarketing.portlets.htmlpageasset.model.HTMLPageAsset;
import com.dotmarketing.portlets.rules.business.RulesEngine;
import com.dotmarketing.portlets.rules.model.Rule.FireOn;
import com.dotmarketing.util.DateUtil;
import com.dotmarketing.util.Logger;
import com.dotmarketing.util.PageMode;
import com.dotmarketing.util.UtilMethods;
import com.dotmarketing.util.WebKeys;
import com.liferay.portal.model.User;
import graphql.schema.DataFetcher;
import graphql.schema.DataFetchingEnvironment;
import io.vavr.control.Try;
import java.time.Instant;
import java.util.Date;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

Expand Down Expand Up @@ -53,13 +59,15 @@ public Contentlet get(final DataFetchingEnvironment environment) throws Exceptio
final boolean fireRules = environment.getArgument("fireRules");
final String persona = environment.getArgument("persona");
final String site = environment.getArgument("site");
final String publishDate = environment.getArgument("publishDate");

context.addParam("url", url);
context.addParam("languageId", languageId);
context.addParam("pageMode", pageModeAsString);
context.addParam("fireRules", fireRules);
context.addParam("persona", persona);
context.addParam("site", site);
context.addParam("publishDate", publishDate);

final PageMode mode = PageMode.get(pageModeAsString);
PageMode.setPageMode(request, mode);
Expand All @@ -77,6 +85,22 @@ public Contentlet get(final DataFetchingEnvironment environment) throws Exceptio
request.setAttribute(Host.HOST_VELOCITY_VAR_NAME, site);
}

Date publishDateObj = null;

if(UtilMethods.isSet(publishDate)) {
publishDateObj = Try.of(()-> DateUtil.convertDate(publishDate)).getOrElse(() -> {
Logger.error(this, "Invalid publish date: " + publishDate);
return null;
});
if(null != publishDateObj) {
//We get a valid time machine date
final Instant instant = publishDateObj.toInstant();
final long epochMilli = instant.toEpochMilli();
context.addParam(PageResource.TM_DATE, epochMilli);
request.setAttribute(PageResource.TM_DATE, epochMilli);
}
}

Logger.debug(this, ()-> "Fetching page for URL: " + url);

final PageContext pageContext = PageContextBuilder.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,27 +356,27 @@ private Optional<Date> timeMachineDate(final HttpServletRequest request) {
return Optional.empty();
}

Optional<String> millis = Optional.empty();
Optional<Object> millis = Optional.empty();
final HttpSession session = request.getSession(false);
if (session != null) {
millis = Optional.ofNullable ((String)session.getAttribute(PageResource.TM_DATE));
millis = Optional.ofNullable (session.getAttribute(PageResource.TM_DATE));
}

if (millis.isEmpty()) {
millis = Optional.ofNullable((String)request.getAttribute(PageResource.TM_DATE));
millis = Optional.ofNullable(request.getAttribute(PageResource.TM_DATE));
}

if (millis.isEmpty()) {
return Optional.empty();
}

final Object object = millis.get();
try {
final long milliseconds = Long.parseLong(millis.get());
final long milliseconds = object instanceof Number ? (Long) object : Long.parseLong(object.toString());
return milliseconds > 0
? Optional.of(Date.from(Instant.ofEpochMilli(milliseconds)))
: Optional.empty();
} catch (NumberFormatException e) {
Logger.error(this, "Invalid timestamp format: " + millis.get(), e);
Logger.error(this, "Invalid timestamp format: " + object, e);
return Optional.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ private HTMLPageUrl getHtmlPageAsset(final PageContext context, final Host host,
throws DotDataException, DotSecurityException {
Logger.debug(this, "--HTMLPageAssetRenderedAPIImpl_getHtmlPageAsset--");

Optional<HTMLPageUrl> htmlPageUrlOptional = findPageByContext(host, context);
Optional<HTMLPageUrl> htmlPageUrlOptional = findPageByContext(host, context, request);

if (htmlPageUrlOptional.isEmpty()) {
Logger.debug(this, "HTMLPageAssetRenderedAPIImpl_getHtmlPageAsset htmlPageUrlOptional is Empty trying to find by URL Map");
Expand Down Expand Up @@ -428,17 +428,18 @@ private void checkPagePermission(final PageContext context, final IHTMLPage html
* @throws DotSecurityException The User accessing the APIs does not have the required permissions to perform
* this action.
*/
private Optional<HTMLPageUrl> findPageByContext(final Host host, final PageContext context)
private Optional<HTMLPageUrl> findPageByContext(final Host host, final PageContext context, final HttpServletRequest request)
throws DotDataException, DotSecurityException {

final User user = context.getUser();
final String uri = context.getPageUri();
final PageMode mode = context.getPageMode();
final String pageUri = (UUIDUtil.isUUID(uri) ||( uri.length()>0 && '/' == uri.charAt(0))) ? uri : ("/" + uri);
Logger.debug(this, "HTMLPageAssetRenderedAPIImpl_findPageByContext user: " + user + " uri: " + uri + " mode: " + mode + " host: " + host + " pageUri: " + pageUri);
final HTMLPageAsset htmlPageAsset = (HTMLPageAsset) (UUIDUtil.isUUID(pageUri) ?
this.htmlPageAssetAPI.findPage(pageUri, user, mode.respectAnonPerms) :
getPageByUri(mode, host, pageUri));
String uri = context.getPageUri();
uri = uri == null ? StringPool.BLANK : uri;
final String pageUriOrInode = (UUIDUtil.isUUID(uri) ||(!uri.isEmpty() && '/' == uri.charAt(0))) ? uri : ("/" + uri);
Logger.debug(this, "HTMLPageAssetRenderedAPIImpl_findPageByContext user: " + user + " uri: " + uri + " mode: " + mode + " host: " + host + " pageUriOrInode: " + pageUriOrInode);
final HTMLPageAsset htmlPageAsset = (HTMLPageAsset) (UUIDUtil.isUUID(pageUriOrInode) ?
this.htmlPageAssetAPI.findPage(pageUriOrInode, user, mode.respectAnonPerms) :
getPageByUri(mode, host, pageUriOrInode, request));

Logger.debug(this, "HTMLPageAssetRenderedAPIImpl_findPageByContext htmlPageAsset: " + (htmlPageAsset == null ? "Not Found" : htmlPageAsset.toString()));
return Optional.ofNullable(htmlPageAsset == null ? null : new HTMLPageUrl(htmlPageAsset));
Expand Down Expand Up @@ -494,10 +495,9 @@ private Optional<HTMLPageUrl> findByURLMap(
}
}

private IHTMLPage getPageByUri(final PageMode mode, final Host host, final String pageUri)
private IHTMLPage getPageByUri(final PageMode mode, final Host host, final String pageUri, final HttpServletRequest request)
throws DotDataException, DotSecurityException {

final HttpServletRequest request = HttpServletRequestThreadLocal.INSTANCE.getRequest();
final Language defaultLanguage = this.languageAPI.getDefaultLanguage();
final Language language = this.getCurrentLanguage(request);
Logger.debug(this, "HTMLPageAssetRenderedAPIImpl_getPageByUri pageUri: " + pageUri + " host: " + host + " language: " + language + " mode: " + mode);
Expand Down
5 changes: 4 additions & 1 deletion test-karate/src/test/java/KarateCITests.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ public class KarateCITests {

@Test
void defaults() {
Results results = Runner.path("classpath:tests/defaults").tags("~@ignore")
Results results = Runner.path(
"classpath:tests/defaults",
"classpath:tests/graphql/ftm"
).tags("~@ignore")
.outputHtmlReport(true)
.outputJunitXml(true)
.outputCucumberJson(true)
Expand Down
132 changes: 132 additions & 0 deletions test-karate/src/test/java/graphql/ftm/helpers.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
Feature: Reusable Functions and Helpers

Scenario: Define reusable functions

## General error free validation
* def validateNoErrors =
"""
function (response) {
const errors = response.errors;
if (errors) {
return errors;
}
return [];
}
"""

## Builds a payload for creating a new content version
* def buildContentRequestPayload =
"""
function(contentType, title, publishDate, expiresOn, identifier) {
let payload = {
"contentlets": [
{
"contentType": contentType,
"title": title,
"host":"8a7d5e23-da1e-420a-b4f0-471e7da8ea2d"
}
]
};
if (publishDate) payload.contentlets[0].publishDate = publishDate;
if (expiresOn) payload.contentlets[0].expiresOn = expiresOn;
if (identifier) payload.contentlets[0].identifier = identifier;
return payload;
}
"""
## Extracts all errors from a response
* def extractErrors =
"""
function(response) {
let errors = [];
let results = response.entity.results;
if (results && results.length > 0) {
for (let i = 0; i < results.length; i++) {
let result = results[i];
// Handle both nested error messages and direct error messages
for (let key in result) {
if (result[key] && result[key].errorMessage) {
errors.push(result[key].errorMessage);
}
}
}
}
return errors;
}
"""

## Extracts all contentlets from a response
* def extractContentlets =
"""
function(response) {
let containers = response.entity.containers;
let allContentlets = [];
for (let key in containers) {
if (containers[key].contentlets) {
for (let contentletKey in containers[key].contentlets) {
allContentlets = allContentlets.concat(containers[key].contentlets[contentletKey]);
}
}
}
return allContentlets;
}
"""

## Generates a random suffix for test data
* def testSuffix =
"""
function() {
if (!karate.get('testSuffix')) {
let prefix = '__' + Math.floor(Math.random() * 100000);
karate.set('testSuffix', prefix);
}
return karate.get('testSuffix');
}
"""

## Extracts a specific object from a JSON array by UUID
* def getContentletByUUID =
"""
function(jsonArray, uuid) {
for (let i = 0; i < jsonArray.length; i++) {
let keys = Object.keys(jsonArray[i]);
if (keys.includes(uuid)) {
return jsonArray[i][uuid];
}
}
return null; // Return null if not found
}
"""

## Builds a payload for creating a new GraphQL request
* def buildGraphQLRequestPayload =
"""
function(pageUri, publishDate) {
if (!pageUri.startsWith('/')) {
pageUri = '/' + pageUri;
}
var query = 'query Page { page(url: "' + pageUri + '"';
if (publishDate) {
query += ' publishDate: "' + publishDate + '"';
}
query += ') { containers { containerContentlets { contentlets { title } } } } }';
return { query: query };
}
"""

## Extracts all contentlet titles from a GraphQL response
* def contentletsFromGraphQlResponse =
"""
function(response) {
let containers = response.data.page.containers;
let allTitles = [];
containers.forEach(container => {
container.containerContentlets.forEach(cc => {
cc.contentlets.forEach(contentlet => {
allTitles.push(contentlet.title);
});
});
});
return allTitles;
}
"""
##
24 changes: 24 additions & 0 deletions test-karate/src/test/java/graphql/ftm/newContainer.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
Feature: Create a Container
Background:
* def containerNameVariable = 'MyContainer' + Math.floor(Math.random() * 100000)

Scenario: Create a content type and expect 200 OK
Given url baseUrl + '/api/v1/containers'
And headers commonHeaders
And request
"""
{
"title":"#(containerNameVariable)",
"friendlyName":"My test container.",
"maxContentlets":10,
"notes":"Notes",
"containerStructures":[
{
"structureId":"#(contentTypeId)",
"code":"$!{dotContentMap.title}"
}
]
}
"""
When method POST
Then status 200
21 changes: 21 additions & 0 deletions test-karate/src/test/java/graphql/ftm/newContent.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
Feature: Create an instance of a new Content Type and expect 200 OK
Background:

Scenario: Create an instance of a new Content Type and expect 200 OK

# Params are expected as arguments to the feature file
* def contentTypeId = __arg.contentTypeId
* def title = __arg.title
* def publishDate = __arg.publishDate
* def expiresOn = __arg.expiresOn

Given url baseUrl + '/api/v1/workflow/actions/default/fire/PUBLISH?indexPolicy=WAIT_FOR'
And headers commonHeaders

* def requestPayload = buildContentRequestPayload (contentTypeId, title, publishDate, expiresOn)
And request requestPayload

When method POST
Then status 200
* def errors = call extractErrors response
* match errors == []
Loading

0 comments on commit 82ddbb0

Please sign in to comment.