Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qute - improve content type detection #11050

Merged
merged 1 commit into from
Jul 30, 2020
Merged
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 @@ -92,7 +92,7 @@ public CompletionStage<Void> send() {
@SuppressWarnings("unchecked")
List<Variant> variants = (List<Variant>) variantsAttr;
for (Variant variant : variants) {
if (variant.mediaType.equals(Variant.TEXT_HTML) || variant.mediaType.equals(Variant.TEXT_PLAIN)) {
if (variant.getContentType().equals(Variant.TEXT_HTML) || variant.getContentType().equals(Variant.TEXT_PLAIN)) {
results.add(new Result(variant,
Uni.createFrom().completionStage(
new Supplier<CompletionStage<? extends String>>() {
Expand Down Expand Up @@ -130,9 +130,9 @@ public Mail apply(List<?> ignored) {
for (Result res : results) {
// We can safely access the content here: 1. it has been resolved, 2. it's cached.
String content = res.value.await().indefinitely();
if (res.variant.mediaType.equals(Variant.TEXT_HTML)) {
if (res.variant.getContentType().equals(Variant.TEXT_HTML)) {
mail.setHtml(content);
} else if (res.variant.mediaType.equals(Variant.TEXT_PLAIN)) {
} else if (res.variant.getContentType().equals(Variant.TEXT_PLAIN)) {
mail.setText(content);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
import io.quarkus.qute.deployment.TypeInfos.Info;
import io.quarkus.qute.generator.ExtensionMethodGenerator;
import io.quarkus.qute.generator.ValueResolverGenerator;
import io.quarkus.qute.runtime.ContentTypes;
import io.quarkus.qute.runtime.EngineProducer;
import io.quarkus.qute.runtime.QuteConfig;
import io.quarkus.qute.runtime.QuteRecorder;
Expand Down Expand Up @@ -172,7 +173,7 @@ void processTemplateErrors(TemplatesAnalysisBuildItem analysis, List<IncorrectEx
AdditionalBeanBuildItem additionalBeans() {
return AdditionalBeanBuildItem.builder()
.setUnremovable()
.addBeanClasses(EngineProducer.class, TemplateProducer.class, ResourcePath.class,
.addBeanClasses(EngineProducer.class, TemplateProducer.class, ContentTypes.class, ResourcePath.class,
Template.class, TemplateInstance.class, CollectionTemplateExtensions.class,
MapTemplateExtensions.class, NumberTemplateExtensions.class)
.build();
Expand Down Expand Up @@ -834,6 +835,7 @@ void validateTemplateInjectionPoints(QuteConfig config, List<TemplatePathBuildIt
TemplateVariantsBuildItem collectTemplateVariants(List<TemplatePathBuildItem> templatePaths) throws IOException {
Set<String> allPaths = templatePaths.stream().map(TemplatePathBuildItem::getPath).collect(Collectors.toSet());
// item -> [item.html, item.txt]
// ItemResource/item -> -> [ItemResource/item.html, ItemResource/item.xml]
Map<String, List<String>> baseToVariants = new HashMap<>();
for (String path : allPaths) {
int idx = path.lastIndexOf('.');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.qute.deployment;

import static io.quarkus.qute.TemplateInstance.SELECTED_VARIANT;
import static org.junit.jupiter.api.Assertions.assertEquals;

import javax.enterprise.context.Dependent;
Expand Down Expand Up @@ -30,11 +31,11 @@ public class VariantTemplateTest {

@Test
public void testRendering() {
TemplateInstance rendering = simpleBean.foo.instance().data("bar");
rendering.setAttribute(TemplateInstance.SELECTED_VARIANT, new Variant(null, "text/plain", null));
assertEquals("bar", rendering.render());
rendering.setAttribute(TemplateInstance.SELECTED_VARIANT, new Variant(null, "text/html", null));
assertEquals("<strong>bar</strong>", rendering.render());
TemplateInstance instance = simpleBean.foo.instance().data("bar");
instance.setAttribute(SELECTED_VARIANT, Variant.forContentType("text/plain"));
assertEquals("bar", instance.render());
instance.setAttribute(SELECTED_VARIANT, Variant.forContentType("text/html"));
assertEquals("<strong>bar</strong>", instance.render());
}

@Dependent
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package io.quarkus.qute.deployment.contenttypes;

import static io.quarkus.qute.TemplateInstance.SELECTED_VARIANT;
import static org.junit.jupiter.api.Assertions.assertEquals;

import javax.inject.Inject;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.qute.Template;
import io.quarkus.qute.Variant;
import io.quarkus.test.QuarkusUnitTest;

public class AdditionalContentTypeTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addAsResource(new StringAsset("Empty txt"), "templates/foo.txt")
.addAsResource(new StringAsset("Empty graphql"), "templates/foo.graphql")
.addAsResource(new StringAsset("quarkus.qute.content-types.graphql=application/graphql\n"
+ "quarkus.qute.suffixes=txt,graphql"), "application.properties"));

@Inject
Template foo;

@Test
public void testVariant() {
assertEquals("Empty graphql",
foo.instance().setAttribute(SELECTED_VARIANT, Variant.forContentType("application/graphql")).render());
assertEquals("Empty txt",
foo.instance().setAttribute(SELECTED_VARIANT, Variant.forContentType("text/plain")).render());
// foo.txt is used by default because it's first in the quarkus.qute.suffixes list
assertEquals("Empty txt",
foo.instance().render());
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package io.quarkus.qute.runtime;

import java.net.URLConnection;

import javax.inject.Inject;
import javax.inject.Singleton;

import org.jboss.logging.Logger;

import io.quarkus.qute.Variant;

@Singleton
public class ContentTypes {

private static final Logger LOGGER = Logger.getLogger(ContentTypes.class);

@Inject
QuteConfig config;

/**
*
* @param templatePath The path relative to the template root, uses the {@code /} path separator.
* @return the content type
*/
public String getContentType(String templatePath) {
String fileName = templatePath;
int slashIdx = fileName.lastIndexOf('/');
if (slashIdx != -1) {
fileName = fileName.substring(slashIdx, fileName.length());
}
int dotIdx = fileName.lastIndexOf('.');
if (dotIdx != -1) {
String suffix = fileName.substring(dotIdx + 1, fileName.length());
String additionalContentType = config.contentTypes.get(suffix);
if (additionalContentType != null) {
return additionalContentType;
}
if (suffix.equalsIgnoreCase("json")) {
return Variant.APPLICATION_JSON;
}
String contentType = URLConnection.getFileNameMap().getContentTypeFor(fileName);
Copy link
Contributor

Choose a reason for hiding this comment

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

We could use Files.probeContentType too (or instead?) to take advantage of libraries that register their own java.nio.file.spi.FileTypeDetector through the ServiceLoader mechanism

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about Files.probeContentType() too but given the fact that the result may differ, based on the OS or presence of other libraries, I decided to choose the simple and straightforward solution for now. Also it seems that the default FileTypeDetector impl is makes use of URLConnection.getFileNameMap() anyway...

if (contentType != null) {
return contentType;
}
}
LOGGER.warn("Unable to detect the content type for " + templatePath + "; using application/octet-stream");
return "application/octet-stream";
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,15 @@ public class EngineProducer {
private static final Logger LOGGER = Logger.getLogger(EngineProducer.class);

private final Engine engine;
private final ContentTypes contentTypes;
private final List<String> tags;
private final List<String> suffixes;
private final String basePath;
private final String tagPath;

public EngineProducer(QuteContext context, Event<EngineBuilder> builderReady, Event<Engine> engineReady) {
public EngineProducer(QuteContext context, Event<EngineBuilder> builderReady, Event<Engine> engineReady,
ContentTypes contentTypes) {
this.contentTypes = contentTypes;
this.suffixes = context.getConfig().suffixes;
this.basePath = "templates/";
this.tagPath = basePath + TAGS;
Expand Down Expand Up @@ -172,14 +175,9 @@ private URL locatePath(String path) {
return cl.getResource(path);
}

static Variant guessVariant(String path) {
// TODO we need a proper way to detect the variant
int suffixIdx = path.lastIndexOf('.');
if (suffixIdx != -1) {
String suffix = path.substring(suffixIdx);
return new Variant(null, TemplateProducer.parseMediaType(suffix), null);
}
return null;
Variant guessVariant(String path) {
// TODO detect locale and encoding
return Variant.forContentType(contentTypes.getContentType(path));
}

static class ResourceTemplateLocation implements TemplateLocation {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ String escape(CharSequence value) {
}

static boolean requiresDefaultEscaping(Variant variant) {
return variant.mediaType != null
? (Variant.TEXT_HTML.equals(variant.mediaType) || Variant.TEXT_XML.equals(variant.mediaType))
return variant.getContentType() != null
? (Variant.TEXT_HTML.equals(variant.getContentType()) || Variant.TEXT_XML.equals(variant.getContentType()))
: false;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.quarkus.qute.runtime;

import java.util.List;
import java.util.Map;

import io.quarkus.runtime.annotations.ConfigItem;
import io.quarkus.runtime.annotations.ConfigPhase;
Expand All @@ -26,4 +27,11 @@ public class QuteConfig {
@ConfigItem(defaultValue = "true")
public boolean removeStandaloneLines;

/**
* The additional map of suffixes to content types. This map is used when working with template variants. By default, the
* {@link java.net.URLConnection#getFileNameMap()} is used to determine the content type of a template file.
*/
@ConfigItem
public Map<String, String> contentTypes;

}
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ public class TemplateProducer {

private final Map<String, TemplateVariants> templateVariants;

TemplateProducer(Engine engine, QuteContext context) {
TemplateProducer(Engine engine, QuteContext context, ContentTypes contentTypes) {
this.engine = engine;
Map<String, TemplateVariants> templateVariants = new HashMap<>();
for (Entry<String, List<String>> entry : context.getVariants().entrySet()) {
TemplateVariants var = new TemplateVariants(initVariants(entry.getKey(), entry.getValue()), entry.getKey());
TemplateVariants var = new TemplateVariants(initVariants(entry.getKey(), entry.getValue(), contentTypes),
entry.getKey());
templateVariants.put(entry.getKey(), var);
}
this.templateVariants = Collections.unmodifiableMap(templateVariants);
Expand Down Expand Up @@ -206,32 +207,11 @@ public String toString() {
}
}

static String parseMediaType(String suffix) {
// TODO we need a proper way to parse the media type
if (suffix.equalsIgnoreCase(".html") || suffix.equalsIgnoreCase(".htm")) {
return Variant.TEXT_HTML;
} else if (suffix.equalsIgnoreCase(".xml")) {
return Variant.TEXT_XML;
} else if (suffix.equalsIgnoreCase(".txt")) {
return Variant.TEXT_PLAIN;
} else if (suffix.equalsIgnoreCase(".json")) {
return Variant.APPLICATION_JSON;
}
LOGGER.warn("Unknown media type for suffix: " + suffix);
return "application/octet-stream";
}

static String parseMediaType(String base, String variant) {
String suffix = variant.substring(base.length());
return parseMediaType(suffix);
}

private static Map<Variant, String> initVariants(String base, List<String> availableVariants) {
private static Map<Variant, String> initVariants(String base, List<String> availableVariants, ContentTypes contentTypes) {
Map<Variant, String> map = new HashMap<>();
for (String path : availableVariants) {
if (!base.equals(path)) {
String mediaType = parseMediaType(base, path);
map.put(new Variant(null, mediaType, null), path);
map.put(new Variant(null, contentTypes.getContentType(path), null), path);
}
}
return map;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public void filter(ContainerRequestContext requestContext, ContainerResponseCont
List<Variant> variants = (List<Variant>) variantsAttr;
javax.ws.rs.core.Variant selected = requestContext.getRequest()
.selectVariant(variants.stream()
.map(v -> new javax.ws.rs.core.Variant(MediaType.valueOf(v.mediaType), v.locale, v.encoding))
.map(v -> new javax.ws.rs.core.Variant(MediaType.valueOf(v.getMediaType()), v.getLocale(),
v.getEncoding()))
.collect(Collectors.toList()));
if (selected != null) {
instance.setAttribute(TemplateInstance.SELECTED_VARIANT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,44 @@
*/
public final class Variant {

public static Variant forContentType(String contentType) {
return new Variant(null, contentType, null);
}

public final static String TEXT_HTML = "text/html";
public final static String TEXT_PLAIN = "text/plain";
public final static String TEXT_XML = "text/xml";
public final static String APPLICATION_JSON = "application/json";

public final Locale locale;
public final String mediaType;
public final String encoding;
private final Locale locale;
private final String contentType;
private final String encoding;

public Variant(Locale locale, String mediaType, String encoding) {
public Variant(Locale locale, String contentType, String encoding) {
this.locale = locale;
this.mediaType = mediaType;
this.contentType = contentType;
this.encoding = encoding;
}

public Locale getLocale() {
return locale;
}

public String getMediaType() {
return getContentType();
}

public String getContentType() {
return contentType;
}

public String getEncoding() {
return encoding;
}

@Override
public int hashCode() {
return Objects.hash(encoding, locale, mediaType);
return Objects.hash(encoding, locale, contentType);
}

@Override
Expand All @@ -41,7 +61,15 @@ public boolean equals(Object obj) {
}
Variant other = (Variant) obj;
return Objects.equals(encoding, other.encoding) && Objects.equals(locale, other.locale)
&& Objects.equals(mediaType, other.mediaType);
&& Objects.equals(contentType, other.contentType);
}

@Override
public String toString() {
StringBuilder builder = new StringBuilder();
builder.append("Variant [locale=").append(locale).append(", contentType=").append(contentType).append(", encoding=")
.append(encoding).append("]");
return builder.toString();
}

}