Skip to content

Commit

Permalink
fix(Page) Broken Assets Loading LIVE Mode Refs: #31402 (#31455)
Browse files Browse the repository at this point in the history
### Proposed Changes
* The Old TimeMachineFilter is blocking requests in Live Mode when the
tm_date is set in session
* I am deprecating this Filter in the hope it can be removed soon
* As for now I am disconnecting its execution with another session
variable to avoid breaking UX
* Also in this PR goes the fix that homologates the response in the
**urlContentMap** attribute
  • Loading branch information
fabrizzio-dotCMS authored Feb 25, 2025
1 parent 701c614 commit 9785ade
Show file tree
Hide file tree
Showing 10 changed files with 257 additions and 97 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,54 +112,69 @@ private static Contentlet fixRecurringDates(Contentlet contentlet, String[] recD
*
* @return The requested {@link Contentlet} object.
*/
public static Contentlet find(final String inodeOrIdentifierIn, final User user, final String tmDate, final boolean EDIT_OR_PREVIEW_MODE,
final long sessionLang) {
final String inodeOrIdentifier = RecurrenceUtil.getBaseEventIdentifier(inodeOrIdentifierIn);
final String[] recDates = RecurrenceUtil.getRecurrenceDates(inodeOrIdentifier);
try {
// by inode
Contentlet contentlet = conAPI.find(inodeOrIdentifier, user, true);
if (contentlet != null) {
return fixRecurringDates(contentlet, recDates);
}

// time-machine
if (tmDate != null) {
// This should take care of the rendering bits for the time machine
final Date ffdate = new Date(Long.parseLong(tmDate));
final PageMode pageMode = PageMode.get();
public static Contentlet find(final String inodeOrIdentifierIn, final User user,
final String tmDate, final boolean EDIT_OR_PREVIEW_MODE,
final long sessionLang) {
final String inodeOrIdentifier = RecurrenceUtil.getBaseEventIdentifier(inodeOrIdentifierIn);
final String[] recDates = RecurrenceUtil.getRecurrenceDates(inodeOrIdentifier);
try {
final PageMode pageMode = PageMode.get();
// Test by inode
Contentlet contentlet = conAPI.find(inodeOrIdentifier, user, true);
if (contentlet != null) { // Time machine by the identifier extracted from the contentlet we found through the inode
if (null != tmDate) {
final Date ffdate = new Date(Long.parseLong(tmDate));
final Optional<Contentlet> futureContent = conAPI.findContentletByIdentifierOrFallback(
inodeOrIdentifier, sessionLang, VariantAPI.DEFAULT_VARIANT.name(),
ffdate, user, pageMode.respectAnonPerms);
if (futureContent.isPresent()) {
return futureContent.get();
}
// If the content is not found or has expired
// No need to return null we continue to the next step to try to find the content in the live or working version
}
contentlet.getIdentifier(), sessionLang,
VariantAPI.DEFAULT_VARIANT.name(),
ffdate, user, pageMode.respectAnonPerms);
if (futureContent.isPresent()) {
return fixRecurringDates(futureContent.get(), recDates);
}
}
}

final ContentletVersionInfo contentletVersionInfoByFallback = WebAPILocator.getVariantWebAPI()
.getContentletVersionInfoByFallback(sessionLang, inodeOrIdentifier,
EDIT_OR_PREVIEW_MODE ? PageMode.PREVIEW_MODE : PageMode.LIVE, user);
// If content is being viewed in EDIT_OR_PREVIEW_MODE, we need to get the working version. Otherwise, we
// need the live version. That's why we're negating it when calling the API
final String contentletInode =
EDIT_OR_PREVIEW_MODE ? contentletVersionInfoByFallback.getWorkingInode()
: contentletVersionInfoByFallback.getLiveInode();

contentlet = conAPI.find(contentletInode, user, true);
return fixRecurringDates(contentlet, recDates);
} catch (final Exception e) {
String msg = e.getMessage();
msg = (msg.contains("\n")) ? msg.substring(0, msg.indexOf("\n")) : msg;
final String errorMsg = String.format("An error occurred when User '%s' attempted to find Contentlet " +
"with Inode/ID '%s' [lang=%s, tmDate=%s]: %s",
user.getUserId(), inodeOrIdentifier, sessionLang, tmDate, msg);
Logger.warn(ContentUtils.class, errorMsg);
Logger.debug(ContentUtils.class, errorMsg, e);
return null;
}
}
// okay if we failed retrieving the contentlet using inode we still need to test by identifier Cuz this method actually takes an identifier
if (null != tmDate) {
final Date ffdate = new Date(Long.parseLong(tmDate));
final Optional<Contentlet> futureContent = conAPI.findContentletByIdentifierOrFallback(
inodeOrIdentifier, sessionLang, VariantAPI.DEFAULT_VARIANT.name(),
ffdate, user, pageMode.respectAnonPerms);
if (futureContent.isPresent()) {
return fixRecurringDates(futureContent.get(), recDates);
}
}

// if We ran out of possibilities retrieving the contentlet via time machine we fall back on the current or present live contentlet
// if we already had one found by inode this is the one We should return
if (null != contentlet) {
return fixRecurringDates(contentlet, recDates);
}
// Fallbacks from here on...
//If not we try to get the contentlet by the identifier
final ContentletVersionInfo contentletVersionInfoByFallback = WebAPILocator.getVariantWebAPI()
.getContentletVersionInfoByFallback(sessionLang, inodeOrIdentifier,
EDIT_OR_PREVIEW_MODE ? PageMode.PREVIEW_MODE : PageMode.LIVE, user);
// If content is being viewed in EDIT_OR_PREVIEW_MODE, we need to get the working version. Otherwise, we
// need the live version. That's why we're negating it when calling the API
final String contentletInode =
EDIT_OR_PREVIEW_MODE ? contentletVersionInfoByFallback.getWorkingInode()
: contentletVersionInfoByFallback.getLiveInode();

contentlet = conAPI.find(contentletInode, user, true);
return fixRecurringDates(contentlet, recDates);
} catch (final Exception e) {
String msg = e.getMessage();
msg = (msg.contains("\n")) ? msg.substring(0, msg.indexOf("\n")) : msg;
final String errorMsg = String.format(
"An error occurred when User '%s' attempted to find Contentlet " +
"with Inode/ID '%s' [lang=%s, tmDate=%s]: %s",
user.getUserId(), inodeOrIdentifier, sessionLang, tmDate, msg);
Logger.warn(ContentUtils.class, errorMsg);
Logger.debug(ContentUtils.class, errorMsg, e);
return null;
}
}

/**
* Returns empty List if no results are found
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ public class PageResource {
public static final String TM_LANG = "tm_lang";
public static final String TM_HOST = "tm_host";
public static final String DOT_CACHE = "dotcache";
public static final String IS_PAGE_RESOURCE = "pageResource";

private final PageResourceHelper pageResourceHelper;
private final WebResource webResource;
Expand Down Expand Up @@ -481,12 +482,13 @@ private void setUpTimeMachineIfPresent(final PageRenderParams renderParams) {
session.setAttribute(TM_LANG, renderParams.languageId());
session.setAttribute(DOT_CACHE, "refresh");
session.setAttribute(TM_HOST, host.get());
} else {
session.setAttribute(IS_PAGE_RESOURCE, true);
}
request.setAttribute(TM_DATE, timeMachineEpochMillis);
request.setAttribute(TM_LANG, renderParams.languageId());
request.setAttribute(DOT_CACHE, "refresh");
request.setAttribute(TM_HOST, host.get());
}
request.setAttribute(IS_PAGE_RESOURCE, true);
}
}

Expand All @@ -501,6 +503,8 @@ private void resetTimeMachineIfPresent(final HttpServletRequest request) {
session.removeAttribute(TM_LANG);
session.removeAttribute(TM_HOST);
session.removeAttribute(DOT_CACHE);
// we do not remove the IS_PAGE_RESOURCE attribute
//It'll get removed from the old from the time machine portal
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.dotcms.notifications.business.NotificationAPI;
import com.dotcms.repackage.com.google.common.annotations.VisibleForTesting;
import com.dotcms.rest.WebResource;
import com.dotcms.rest.api.v1.page.PageResource;
import com.dotcms.util.I18NMessage;
import com.dotmarketing.beans.Host;
import com.dotmarketing.business.APILocator;
Expand Down Expand Up @@ -42,6 +43,7 @@
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;

public class TimeMachineAjaxAction extends IndexAjaxAction {

Expand All @@ -66,6 +68,12 @@ public TimeMachineAjaxAction(final NotificationAPI notificationAPI, ESIndexHelpe

@Override
public void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
final HttpSession session = request.getSession(false);
if(null != session){
//Clean up attribute so TimeMachineFilter can operate
session.removeAttribute(PageResource.IS_PAGE_RESOURCE);
}

Map<String, String> map = getURIParams();
String cmd = map.get("cmd");
java.lang.reflect.Method meth = null;
Expand Down
19 changes: 15 additions & 4 deletions dotCMS/src/main/java/com/dotcms/util/TimeMachineUtil.java
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
package com.dotcms.util;

import com.dotcms.api.web.HttpServletRequestThreadLocal;

import com.dotcms.rest.api.v1.page.PageResource;
import com.dotmarketing.util.Config;
import com.dotmarketing.util.DateUtil;
import io.vavr.Lazy;
import io.vavr.control.Try;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;
import java.time.Duration;
import java.time.Instant;
import java.util.Date;
import java.util.Objects;
import java.util.Optional;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;

public final class TimeMachineUtil {

Expand All @@ -40,6 +38,19 @@ public static Optional<String> getTimeMachineDate() {
return Optional.ofNullable(timeMachineObject != null ? timeMachineObject.toString() : null);
}

/**
* If Time Machine is running return the timestamp of the Time Machine date as a Date
* @return Optional<Date>
*/
public static Optional<Date> getTimeMachineDateAsDate() {
final Optional<String> timeMachine = getTimeMachineDate();
if (timeMachine.isPresent()) {
final String tmDate = timeMachine.get();
return Optional.of(new Date(Long.parseLong(tmDate)));
}
return Optional.empty();
}

/**
* Return true if Time Machine is running, otherwise return false
* @return boolean
Expand Down
10 changes: 10 additions & 0 deletions dotCMS/src/main/java/com/dotmarketing/filters/CMSUrlUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -655,4 +655,14 @@ public String getInodeFromUrlPath(final String urlPath) {
return urlPath.substring(urlPath.indexOf(FORWARD_SLASH) + 1, urlPath.indexOf(UNDERLINE));
}

/**
* Test the request to see if it is a dotAdmin request
* @param request the request to test
* @return true if the request is a dotAdmin request
*/
public static boolean isDotAdminRequest(HttpServletRequest request) {
final String referer = request.getHeader("referer");
return referer != null && referer.contains("/dotAdmin");
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package com.dotmarketing.filters;


import static com.dotmarketing.filters.CMSUrlUtil.isDotAdminRequest;

import com.dotcms.rest.api.v1.page.PageResource;
import javax.ws.rs.core.MediaType;
import org.apache.commons.io.IOUtils;
import com.dotmarketing.beans.Host;
Expand Down Expand Up @@ -45,6 +48,7 @@
* @since May 31, 2012
*
*/
@Deprecated(since = "250221", forRemoval = true)
public class TimeMachineFilter implements Filter {

ServletContext ctx;
Expand All @@ -67,10 +71,18 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
chain.doFilter(request, response);
return;
}

// If there's a session attribute that indicates that the request is a page resource, then skip the filter
// This is a temporary fix to avoid the filter to be executed when the request is a page resource until we retire the old
if(null != req.getSession().getAttribute(PageResource.IS_PAGE_RESOURCE)){
chain.doFilter(request, response);
return;
}

if(!uri.startsWith("/")) {
uri="/"+uri;
}
if(uri != null && uri.startsWith("/admin") && req.getSession().getAttribute("tm_date")!=null){
if(uri.startsWith("/admin") && req.getSession().getAttribute("tm_date") != null){
req.getSession().removeAttribute(TM_DATE_VAR);
req.getSession().removeAttribute(TM_LANG_VAR);
req.getSession().removeAttribute(TM_HOST_VAR);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.dotmarketing.servlets;

import static com.dotmarketing.filters.CMSUrlUtil.isDotAdminRequest;
import static com.dotmarketing.image.focalpoint.FocalPointAPIImpl.TMP;
import static com.liferay.util.HttpHeaders.CACHE_CONTROL;
import static com.liferay.util.HttpHeaders.EXPIRES;
Expand Down Expand Up @@ -684,16 +685,6 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl

}

/**
* Test the request to see if it is a dotAdmin request
* @param request the request to test
* @return true if the request is a dotAdmin request
*/
public static boolean isDotAdminRequest(HttpServletRequest request) {
final String referer = request.getHeader("referer");
return referer != null && referer.contains("/dotAdmin");
}

private Contentlet getContentletByIdentifier(final PageMode pageMode,
final String identifier, final long languageId, final User user)
throws DotDataException, DotSecurityException {
Expand Down
36 changes: 36 additions & 0 deletions dotCMS/src/test/java/com/dotmarketing/filters/CMSUrlUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

import javax.servlet.http.HttpServletRequest;

import static com.dotmarketing.filters.CMSUrlUtil.isDotAdminRequest;
import static com.dotmarketing.filters.Constants.CMS_FILTER_URI_OVERRIDE;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -75,4 +78,37 @@ public void test_getIdentifierFromUrlPath() {

}

/**
* Given scenario: Test the request comes from dotAdmin
* Expected result: Should return true if the referer is a valid dotAdmin referer
*/
@Test
public void testDotAdminRequestValidReferer() {
HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getHeader("referer")).thenReturn("http://localhost:8080/dotAdmin/somepage");
assertTrue( "Should be true for valid dotAdmin referer", isDotAdminRequest(request));
}

/**
* Given scenario: Test the request comes from dotAdmin
* Expected result: Should return true if the referer is a valid dotAdmin referer
*/
@Test
public void testDotAdminRequestWithDifferentDomain() {
HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getHeader("referer")).thenReturn("http://otherdomain.com/dotAdmin/somepage");
assertTrue( "Should be true for valid dotAdmin referer", isDotAdminRequest(request));
}

/**
* Given scenario: Test the request comes from dotAdmin
* Expected result: Should return true if the referer is a valid dotAdmin referer
*/
@Test
public void testDotAdminRequestWithoutDotAdmin() {
HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getHeader("referer")).thenReturn("http://localhost:8080/anotherPath/somepage");
assertFalse("Should be false if /dotAdmin is not present", isDotAdminRequest(request));
}

}
Loading

0 comments on commit 9785ade

Please sign in to comment.