Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ public interface Localization {
*/
boolean isStaticContent(String url);

/**
* Determines if the specified URL refers to static content within the webapp
*
* @return {@code true} if the specified URL refers to static content within the webapp, {@code false} otherwise.
* @param url a {@link java.lang.String} object.
*/
boolean isNonPublishedAsset(String url);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need default implementation that always returns false to not to break compatibility

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would compatibility be broken exactly? As far as I can see there would only be a problem in the situation where the HTML files were published from the CMS and you had put a _version.json file on the web app filesystem or dxa.assets.version property in dxa.properties - which would be a non-sensical situation, but still one which you would resolve by simply removing the file/property

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willprice76 all these comments in all the PRs from are now for internal usage, so don't worry and ignore all of them :) We cannot merge all your PRs to 1.7 release on github, and we also have internal conventions on how to merge PRs from GitHub (happens through stash quality gate).

Regarding the comment itself: we have never defined what is really public API for DXA, so it doesn't hurt to add a default implementation to also support the case if anyone extended localization implementation or implemented their own localization.


/**
* <p>isDefault.</p>
*
Expand Down Expand Up @@ -120,5 +128,4 @@ public interface Localization {
*/
List<String> getDataFormats();


}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.sdl.webapp.common.api.localization.Localization;
import com.sdl.webapp.common.util.MimeUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.joda.time.Hours;
import org.joda.time.Weeks;
import org.slf4j.Logger;
Expand All @@ -27,6 +28,7 @@
import java.io.OutputStream;
import java.lang.management.ManagementFactory;
import java.net.URL;
import java.util.regex.Pattern;

/**
* Static content interceptor. This interceptor checks if the request is for static content, and if it is, it sends
Expand All @@ -38,6 +40,7 @@ public class StaticContentInterceptor extends HandlerInterceptorAdapter {
private static final Logger LOG = LoggerFactory.getLogger(StaticContentInterceptor.class);
private static final String CACHE_CONTROL_WEEK = "public, max-age=" + Weeks.ONE.toStandardSeconds().getSeconds();
private static final String CACHE_CONTROL_HOUR = "public, max-age=" + Hours.ONE.toStandardSeconds().getSeconds();
private static final Pattern SYSTEM_VERSION_PATTERN = Pattern.compile("/system/v\\d+\\.\\d+/");

@Autowired
private ContentProvider contentProvider;
Expand Down Expand Up @@ -89,9 +92,10 @@ private static void fallbackForContentProvider(ServletServerHttpRequest req, Str

/**
* {@inheritDoc}
* @throws IOException
*/
@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws ServletException {
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws ServletException, IOException {
final String requestPath = webRequestContext.getRequestPath();
LOG.trace("preHandle: {}", requestPath);

Expand All @@ -103,12 +107,16 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
final ServletServerHttpResponse res = new ServletServerHttpResponse(response);

try {
if (localization.isNonPublishedAsset(requestPath))
{
fallbackForContentProvider(req, removeVersionNumber(requestPath), res);
}
StaticContentItem staticContentItem = null;

try {
staticContentItem = contentProvider.getStaticContent(requestPath, localization.getId(), localization.getPath());
} catch (StaticContentNotFoundException e) {
fallbackForContentProvider(req, requestPath, res);
fallbackForContentProvider(req, removeVersionNumber(requestPath), res);
}

if (staticContentItem != null) {
Expand All @@ -131,7 +139,10 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
res.close();
return false;
}

return true;
}

protected static String removeVersionNumber(String path) {
return SYSTEM_VERSION_PATTERN.matcher(path).replaceFirst("/system/");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,16 @@ private boolean loadVersionFromProperties(String id, String path, LocalizationIm
return false;
}
builder.setVersion(assetsVersion);
builder.setHtmlDesignPublished(false);
return true;
}
}

private boolean loadVersionFromBroker(String id, String path, LocalizationImpl.Builder builder) throws LocalizationFactoryException {
try {
StaticContentItem item = contentProvider.getStaticContent(VERSION_PATH, id, path);
try (final InputStream in = item.getContent()) {
builder.setVersion(objectMapper.readTree(in).get("version").asText());
builder.setHtmlDesignPublished(true);
return true;
}
} catch (StaticContentNotFoundException e) {
Expand All @@ -153,6 +155,7 @@ private boolean loadVersionFromWebapp(String id, String path, LocalizationImpl.B

try (final InputStream in = new FileInputStream(file)) {
builder.setVersion(objectMapper.readTree(in).get("version").asText());
builder.setHtmlDesignPublished(false);
return true;
} catch (IOException e) {
throw new LocalizationFactoryException("Exception while reading configuration of localization: [" + id +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ public class LocalizationImpl implements Localization {
@Getter
private final String version;

@Getter
private final boolean isHtmlDesignPublished;

@Getter
private final List<SiteLocalization> siteLocalizations;

Expand All @@ -67,7 +70,7 @@ private LocalizationImpl(Builder builder) {
this.default_ = builder.default_;
this.staging = builder.staging;
this.version = builder.version;

this.isHtmlDesignPublished = builder.htmlDesignPublished;
this.siteLocalizations = builder.siteLocalizationsBuilder.build();
this.configuration = builder.configurationBuilder.build();
this.resources = builder.resourcesBuilder.build();
Expand Down Expand Up @@ -96,11 +99,21 @@ public boolean isStaticContent(String url) {
if (url.startsWith(mediaRoot)) {
return true;
}

final String p = path.equals("/") ? url : url.substring(path.length());
return p.equals(FAVICON_PATH) || SYSTEM_ASSETS_PATTERN.matcher(p).matches();
}

/**
* {@inheritDoc}
*/
@Override
public boolean isNonPublishedAsset(String url) {
if (!this.isHtmlDesignPublished && SYSTEM_ASSETS_PATTERN.matcher(url).matches()){
return true;
}
return false;
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -173,6 +186,7 @@ public static final class Builder {
private boolean default_;
private boolean staging;
private String version;
private boolean htmlDesignPublished;

private Builder() {
}
Expand Down Expand Up @@ -207,6 +221,11 @@ public Builder setVersion(String version) {
return this;
}

public Builder setHtmlDesignPublished(boolean htmlDesignPublished) {
this.htmlDesignPublished = htmlDesignPublished;
return this;
}

public Builder addSiteLocalizations(Iterable<? extends SiteLocalization> siteLocalizations) {
this.siteLocalizationsBuilder.addAll(siteLocalizations);
return this;
Expand Down