From e8b73a87c6ffc767b9417b3daee97ab892673a54 Mon Sep 17 00:00:00 2001 From: Thomas Vidic Date: Sat, 10 Mar 2018 20:58:23 +0100 Subject: [PATCH] Handle script tags (#27) * parse script tags in html head for composition * introduced asset class for parsed asset links, added handling of js assets * filter options attribute when rendering asset links into response * Improved equals methods, added missing test, added code comment --- readme.md | 2 +- .../rewedigital/composer/composing/Asset.java | 117 ++++++++++++++++++ .../composer/composing/Composition.java | 39 +++--- .../composer/composing/CompositionStep.java | 5 + .../composing/ContentMarkupHandler.java | 83 +++++++------ .../composer/composing/ContentRange.java | 2 +- .../composing/IncludeMarkupHandler.java | 7 +- .../composer/composing/IncludeProcessor.java | 8 +- .../composer/composing/IncludedService.java | 4 +- .../rewedigital/composer/routing/Match.java | 9 +- .../composing/ContentMarkupHandlerTest.java | 17 +-- .../composing/TemplateComposerTest.java | 20 ++- .../LocalSessionIdInterceptorTest.java | 10 ++ .../composer/session/Sessions.java | 6 +- 14 files changed, 238 insertions(+), 91 deletions(-) create mode 100644 src/main/java/com/rewedigital/composer/composing/Asset.java diff --git a/readme.md b/readme.md index 1d27213..9edcff4 100644 --- a/readme.md +++ b/readme.md @@ -54,7 +54,7 @@ Using content tags to mark the content to be included allows the response of som **Specify assets required for included content** -Content fragments usually need some assets like css and/or javascript to work. For this. content responses can mark asset links using a special data attribute. The name of this attribute is *data-rd-options* and can be changed via configuration. If an asset link in the html head of the content response contains the `data-rd-options` attribute with value `include`, this link is included in the head of the document returned to the caller. +Content fragments usually need some assets like css and/or javascript to work. For this. content responses can mark asset links resp. script includes in their head section using a special data attribute. The name of this attribute is *data-rd-options* and can be changed via configuration. If an asset link/script in the html head of the content response contains the `data-rd-options` attribute with value `include`, this link/script is included in the head of the document returned to the caller. *Example* ``` diff --git a/src/main/java/com/rewedigital/composer/composing/Asset.java b/src/main/java/com/rewedigital/composer/composing/Asset.java new file mode 100644 index 0000000..f682060 --- /dev/null +++ b/src/main/java/com/rewedigital/composer/composing/Asset.java @@ -0,0 +1,117 @@ +package com.rewedigital.composer.composing; + +import java.util.HashMap; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Objects; +import java.util.function.Predicate; + +/** + * Describes a link or script tag found in an html head section during parsing of + * a template or content fragment. + */ +class Asset { + public static class Builder { + + private final String optionsAttributeName; + private String type = "link"; + private boolean selfClosing = false; + private final Map attributes = new HashMap<>(); + + public Builder(final String optionsAttributeName) { + this.optionsAttributeName = optionsAttributeName; + } + + public Asset.Builder type(final String type) { + this.type = type; + return this; + } + + public Asset.Builder attribute(final String name, final String value) { + this.attributes.put(name, value); + return this; + } + + public Asset.Builder selfClosing(final boolean selfClosing) { + this.selfClosing = selfClosing; + return this; + } + + public boolean isInclude() { + return optionsContain("include"); + } + + private boolean optionsContain(final String option) { + return attributes.getOrDefault(optionsAttributeName, "").contains(option); + } + + public Asset build() { + return new Asset(this); + } + } + + private final String optionsAttributeName; + private final String type; + private final Map attributes; + private final boolean selfClosing; + + private Asset(final Asset.Builder builder) { + this.optionsAttributeName = builder.optionsAttributeName; + this.type = builder.type; + this.selfClosing = builder.selfClosing; + this.attributes = new HashMap<>(builder.attributes); + } + + public String render() { + return attributes + .entrySet() + .stream() + .filter(notOptionsAttribute()) + .reduce(new StringBuilder().append(renderOpen()), + (builder, e) -> builder.append(e.getKey()) + .append("=\"") + .append(e.getValue()) + .append("\" "), + (a, b) -> a.append(b)) + .append(renderClosing()).toString(); + } + + private Predicate> notOptionsAttribute() { + return e -> !e.getKey().equals(optionsAttributeName); + } + + @Override + public int hashCode() { + return Objects.hash(attributes, selfClosing, type); + } + + @Override + public boolean equals(final Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + final Asset other = (Asset) obj; + return selfClosing == other.selfClosing && + Objects.equals(attributes, other.attributes) && + Objects.equals(type, other.type); + } + + @Override + public String toString() { + return "Asset [type=" + type + ", attributes=" + attributes + ", selfClosing=" + selfClosing + "]"; + } + + private String renderOpen() { + return "<" + type + " "; + } + + private String renderClosing() { + if (selfClosing) { + return "/>"; + } + return ">"; + } +} diff --git a/src/main/java/com/rewedigital/composer/composing/Composition.java b/src/main/java/com/rewedigital/composer/composing/Composition.java index 6f54732..b6dcede 100755 --- a/src/main/java/com/rewedigital/composer/composing/Composition.java +++ b/src/main/java/com/rewedigital/composer/composing/Composition.java @@ -36,50 +36,47 @@ public interface Extractor { } private final List children; - private final List assetLinks; + private final List assets; private final int startOffset; private final int endOffset; private final String template; private final ContentRange contentRange; private final SessionFragment session; - private Composition(final String template, final ContentRange contentRange, final List assetLinks, + private Composition(final String template, final ContentRange contentRange, final List assets, final List children) { - this(0, template.length(), template, contentRange, assetLinks, SessionFragment.empty(), children); + this(0, template.length(), template, contentRange, assets, SessionFragment.empty(), children); } private Composition(final int startOffset, final int endOffset, final String template, - final ContentRange contentRange, final List assetLinks, final SessionFragment session, + final ContentRange contentRange, final List assets, + final SessionFragment session, final List children) { this.startOffset = startOffset; this.endOffset = endOffset; this.template = template; this.contentRange = contentRange; - this.assetLinks = assetLinks; + this.assets = assets; this.children = children; this.session = session; } public Composition forRange(final int startOffset, final int endOffset) { - return new Composition(startOffset, endOffset, template, contentRange, assetLinks, session, + return new Composition(startOffset, endOffset, template, contentRange, assets, session, children); } public Composition withSession(final SessionFragment session) { - return new Composition(startOffset, endOffset, template, contentRange, assetLinks, + return new Composition(startOffset, endOffset, template, contentRange, assets, this.session.mergedWith(session), children); } - public static Composition of(final String template, final ContentRange contentRange, - final List assetLinks, final List children) { - return new Composition(template, contentRange, assetLinks, children); - } - - public R map(final BiFunction mapping) { - return mapping.apply(withAssetLinks(body()), mergedSession()); + public static Composition of(final String template, final ContentRange contentRange, final List assets, + final List children) { + return new Composition(template, contentRange, assets, children); } - public R extract(Extractor extractor) { + public R extract(final Extractor extractor) { return extractor.extract(withAssetLinks(body()), mergedSession()); } @@ -90,7 +87,7 @@ private String body() { writer.write(template, currentIndex, c.startOffset - currentIndex); writer.write(c.body()); currentIndex = c.endOffset; - assetLinks.addAll(c.assetLinks); + assets.addAll(c.assets); } writer.write(template, currentIndex, contentRange.end() - currentIndex); return writer.toString(); @@ -104,9 +101,11 @@ private SessionFragment mergedSession() { } private String withAssetLinks(final String body) { - final String assets = assetLinks.stream() - .distinct() - .collect(Collectors.joining("\n")); - return body.replaceFirst("", assets + "\n"); + final String renderedAssets = + assets.stream().distinct() + .map(Asset::render) + .collect(Collectors.joining("\n")); + + return body.replaceFirst("", renderedAssets + "\n"); } } diff --git a/src/main/java/com/rewedigital/composer/composing/CompositionStep.java b/src/main/java/com/rewedigital/composer/composing/CompositionStep.java index a28299c..5090ada 100644 --- a/src/main/java/com/rewedigital/composer/composing/CompositionStep.java +++ b/src/main/java/com/rewedigital/composer/composing/CompositionStep.java @@ -38,4 +38,9 @@ public String callStack() { return "[" + path + "] included via " + parent.callStack(); } + + @Override + public String toString() { + return callStack(); + } } diff --git a/src/main/java/com/rewedigital/composer/composing/ContentMarkupHandler.java b/src/main/java/com/rewedigital/composer/composing/ContentMarkupHandler.java index 5f84c62..6700292 100755 --- a/src/main/java/com/rewedigital/composer/composing/ContentMarkupHandler.java +++ b/src/main/java/com/rewedigital/composer/composing/ContentMarkupHandler.java @@ -1,9 +1,7 @@ package com.rewedigital.composer.composing; -import java.util.HashMap; import java.util.LinkedList; import java.util.List; -import java.util.Map; import org.attoparser.AbstractMarkupHandler; import org.attoparser.ParseException; @@ -14,15 +12,15 @@ class ContentMarkupHandler extends AbstractMarkupHandler { private final char[] contentTag; private final String assetOptionsAttribute; - private final List links = new LinkedList<>(); - private final Map attributes = new HashMap<>(); + private final List assets = new LinkedList<>(); + + private Asset.Builder current = null; private final ContentRange defaultContentRange; private int contentStart = 0; private int contentEnd = 0; private boolean parsingHead = false; - private boolean parsingLink = false; public ContentMarkupHandler(final ContentRange defaultContentRange, final ComposerHtmlConfiguration configuration) { @@ -35,25 +33,26 @@ public ContentRange contentRange() { return contentEnd <= 0 ? defaultContentRange : new ContentRange(contentStart, contentEnd); } - public List assetLinks() { - return links; + public List assets() { + return assets; } - + @Override public void handleStandaloneElementStart(final char[] buffer, final int nameOffset, final int nameLen, final boolean minimized, final int line, final int col) throws ParseException { super.handleStandaloneElementStart(buffer, nameOffset, nameLen, minimized, line, col); - if (parsingHead && isLinkElement(buffer, nameOffset, nameLen)) { - startLink(); + if (parsingHead && isAssetElement(buffer, nameOffset, nameLen)) { + startAsset(buffer, nameOffset, nameLen, true); } } @Override - public void handleStandaloneElementEnd(char[] buffer, int nameOffset, int nameLen, boolean minimized, int line, - int col) throws ParseException { + public void handleStandaloneElementEnd(final char[] buffer, final int nameOffset, final int nameLen, + final boolean minimized, final int line, + final int col) throws ParseException { super.handleStandaloneElementEnd(buffer, nameOffset, nameLen, minimized, line, col); - if (parsingLink) { - pushLink(); + if (parsingAsset()) { + pushAsset(); } } @@ -66,6 +65,8 @@ public void handleOpenElementStart(final char[] buffer, final int nameOffset, fi parsingHead = true; } else if (isContentElement(buffer, nameOffset, nameLen)) { contentStart = nameOffset + nameLen + 1; + } else if (parsingHead && isAssetElement(buffer, nameOffset, nameLen)) { + startAsset(buffer, nameOffset, nameLen, false); } } @@ -77,29 +78,42 @@ public void handleCloseElementEnd(final char[] buffer, final int nameOffset, fin parsingHead = false; } else if (isContentElement(buffer, nameOffset, nameLen) && contentStart >= 0) { contentEnd = nameOffset - 2; + } else if (parsingAsset()) { + pushAsset(); } } @Override - public void handleAttribute(char[] buffer, int nameOffset, int nameLen, int nameLine, int nameCol, - int operatorOffset, int operatorLen, int operatorLine, int operatorCol, int valueContentOffset, - int valueContentLen, int valueOuterOffset, int valueOuterLen, int valueLine, int valueCol) + public void handleAttribute(final char[] buffer, final int nameOffset, final int nameLen, final int nameLine, + final int nameCol, + final int operatorOffset, final int operatorLen, final int operatorLine, final int operatorCol, + final int valueContentOffset, + final int valueContentLen, final int valueOuterOffset, final int valueOuterLen, final int valueLine, + final int valueCol) throws ParseException { super.handleAttribute(buffer, nameOffset, nameLen, nameLine, nameCol, operatorOffset, operatorLen, operatorLine, operatorCol, valueContentOffset, valueContentLen, valueOuterOffset, valueOuterLen, valueLine, valueCol); - if (parsingLink) { - attributes.put(new String(buffer, nameOffset, nameLen), + if (parsingAsset()) { + current = current.attribute(new String(buffer, nameOffset, nameLen), new String(buffer, valueContentOffset, valueContentLen)); } } - private boolean isLinkElement(final char[] buffer, final int nameOffset, final int nameLen) { - return TextUtil.contains(true, buffer, nameOffset, nameLen, "link", 0, "link".length()); + private boolean parsingAsset() { + return current != null; + } + + private boolean isAssetElement(final char[] buffer, final int nameOffset, final int nameLen) { + return textContains(buffer, nameOffset, nameLen, "link") || textContains(buffer, nameOffset, nameLen, "script"); } private boolean isHeadElement(final char[] buffer, final int nameOffset, final int nameLen) { - return TextUtil.contains(true, buffer, nameOffset, nameLen, "head", 0, "head".length()); + return textContains(buffer, nameOffset, nameLen, "head"); + } + + private boolean textContains(final char[] buffer, final int nameOffset, final int nameLen, final String item) { + return TextUtil.contains(true, buffer, nameOffset, nameLen, item, 0, item.length()); } private boolean isContentElement(final char[] buffer, final int nameOffset, final int nameLen) { @@ -108,26 +122,17 @@ private boolean isContentElement(final char[] buffer, final int nameOffset, fina } - private void startLink() { - parsingLink = true; + private void startAsset(final char[] buffer, final int nameOffset, final int nameLen, final boolean selfClosing) { + current = new Asset.Builder(assetOptionsAttribute) + .type(new String(buffer, nameOffset, nameLen)) + .selfClosing(selfClosing); } - private void pushLink() { - if (attributes.getOrDefault(assetOptionsAttribute, "").contains("include")) { - links.add( - attributes - .entrySet() - .stream() - .reduce(new StringBuilder(" builder.append(e.getKey()) - .append("=\"") - .append(e.getValue()) - .append("\" "), - (a, b) -> a.append(b)) - .append("/>").toString()); + private void pushAsset() { + if (current.isInclude()) { + assets.add(current.build()); } - parsingLink = false; - attributes.clear(); + current = null; } diff --git a/src/main/java/com/rewedigital/composer/composing/ContentRange.java b/src/main/java/com/rewedigital/composer/composing/ContentRange.java index ed241a5..6bb39a8 100755 --- a/src/main/java/com/rewedigital/composer/composing/ContentRange.java +++ b/src/main/java/com/rewedigital/composer/composing/ContentRange.java @@ -46,7 +46,7 @@ public boolean equals(Object obj) { if (getClass() != obj.getClass()) return false; ContentRange other = (ContentRange) obj; - return Objects.equals(this.end, other.end) && Objects.equals(this.start, other.start); + return this.end == other.end && this.start == other.start; } @Override diff --git a/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java b/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java index eedf7c0..ee0b427 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludeMarkupHandler.java @@ -30,16 +30,15 @@ public IncludeMarkupHandler(final ContentRange defaultContentRange, final Compos } public IncludeProcessor buildProcessor(final String template) { - return new IncludeProcessor(template, includedServices, contentRange(), - assetLinks()); + return new IncludeProcessor(template, includedServices, contentRange(), assets()); } private ContentRange contentRange() { return contentMarkupHandler.contentRange(); } - private List assetLinks() { - return contentMarkupHandler.assetLinks(); + private List assets() { + return contentMarkupHandler.assets(); } @Override diff --git a/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java b/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java index 319222f..45ba720 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludeProcessor.java @@ -15,15 +15,15 @@ class IncludeProcessor { private final List includedServices; private final ContentRange contentRange; - private final List assetLinks; private final String template; + private final List assets; public IncludeProcessor(final String template, final List includedServices, - final ContentRange contentRange, final List assetLinks) { + final ContentRange contentRange, final List assets) { this.template = template; this.includedServices = includedServices; this.contentRange = contentRange; - this.assetLinks = assetLinks; + this.assets = assets; } public CompletableFuture composeIncludes(final ContentFetcher contentFetcher, @@ -33,7 +33,7 @@ public CompletableFuture composeIncludes(final ContentFetcher conte .map(s -> s.fetch(contentFetcher, step) .thenCompose(r -> r.compose(composer))); return flatten(composedIncludes) - .thenApply(c -> Composition.of(template, contentRange, assetLinks, c.collect(toList()))); + .thenApply(c -> Composition.of(template, contentRange, assets, c.collect(toList()))); } private static CompletableFuture> flatten(final Stream> com) { diff --git a/src/main/java/com/rewedigital/composer/composing/IncludedService.java b/src/main/java/com/rewedigital/composer/composing/IncludedService.java index e3987a7..dd33ee9 100755 --- a/src/main/java/com/rewedigital/composer/composing/IncludedService.java +++ b/src/main/java/com/rewedigital/composer/composing/IncludedService.java @@ -64,9 +64,7 @@ private WithResponse(final CompositionStep step, final int startOffset, final in this.response = response; this.step = step; - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("included service response: {} received via {}", response, step.callStack()); - } + LOGGER.debug("included service response: {} received via {}", response, step); } public CompletableFuture compose(final ContentComposer contentComposer) { diff --git a/src/main/java/com/rewedigital/composer/routing/Match.java b/src/main/java/com/rewedigital/composer/routing/Match.java index e424c8a..4048d7d 100644 --- a/src/main/java/com/rewedigital/composer/routing/Match.java +++ b/src/main/java/com/rewedigital/composer/routing/Match.java @@ -38,14 +38,7 @@ public boolean equals(final Object obj) { if (getClass() != obj.getClass()) return false; final Match other = (Match) obj; - if (backend == null) { - if (other.backend != null) - return false; - } else if (!backend.equals(other.backend)) - return false; - if (routeType != other.routeType) - return false; - return true; + return Objects.equals(backend, other.backend) && routeType == other.routeType; } diff --git a/src/test/java/com/rewedigital/composer/composing/ContentMarkupHandlerTest.java b/src/test/java/com/rewedigital/composer/composing/ContentMarkupHandlerTest.java index 832aeef..6e0872e 100644 --- a/src/test/java/com/rewedigital/composer/composing/ContentMarkupHandlerTest.java +++ b/src/test/java/com/rewedigital/composer/composing/ContentMarkupHandlerTest.java @@ -14,21 +14,24 @@ public class ContentMarkupHandlerTest { private static final ContentRange defaultContentRange = new ContentRange(123, 456); @Test - public void parsesStylesheetLinksFromHead() { - final List result = parse("\n" + + public void parsesAssetsFromHead() { + final List result = parse("\n" + "" - + "" - + "" + + "" + + "" + "" + "\n" + "" + "" - + "").assetLinks(); + + "").assets(); assertEquals( Arrays.asList( - "", - ""), + new Asset.Builder("data-rd-options").type("link").attribute("rel", "stylesheet") + .attribute("data-rd-options", "include").attribute("href", "../static/css/core.css") + .attribute("media", "screen").selfClosing(true).build(), + new Asset.Builder("data-rd-options").type("script").attribute("data-rd-options", "include") + .attribute("href", "../static/js/other.js").selfClosing(false).build()), result); } diff --git a/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java b/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java index a71ccdf..5fd9b4c 100644 --- a/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java +++ b/src/test/java/com/rewedigital/composer/composing/TemplateComposerTest.java @@ -59,7 +59,21 @@ public void appendsCSSLinksToHead() throws Exception { "template-path") .get().response(); assertThat(result.payload()).contains( - "\n" + + "\n" + + ""); + } + + @Test + public void appendsScriptLinksToHead() throws Exception { + final TemplateComposer composer = makeComposer(aClientWithSimpleContent("", + "")); + final Response result = composer + .composeTemplate(r(""), + "template-path") + .get().response(); + assertThat(result.payload()).contains( + "\n" + + ""); } @@ -121,9 +135,9 @@ public void limitsRecursionDepth() throws Exception { .get().response(); assertThat(result.payload()).contains(fallbackContent); } - + @Test - public void setsNoStoreCacheHeaderIfNoCacheHeaderFromComposition() throws Exception{ + public void setsNoStoreCacheHeaderIfNoCacheHeaderFromComposition() throws Exception { final TemplateComposer composer = makeComposer(aClientWithSimpleContent("content")); final Response result = composer diff --git a/src/test/java/com/rewedigital/composer/session/LocalSessionIdInterceptorTest.java b/src/test/java/com/rewedigital/composer/session/LocalSessionIdInterceptorTest.java index 4548f88..7574a0b 100644 --- a/src/test/java/com/rewedigital/composer/session/LocalSessionIdInterceptorTest.java +++ b/src/test/java/com/rewedigital/composer/session/LocalSessionIdInterceptorTest.java @@ -84,6 +84,16 @@ public void doesNotUpdateExpirationTimeBeforeGracePeriod() throws Exception { assertThat(session.isDirty()).isFalse(); } + @Test + public void exiresSessionIfExiresAtNotParseable() throws Exception { + final SessionRoot initialSession = sessionRootExpiringAt("not-a-long", "x-rd-key", "value"); + final LocalSessionIdInterceptor interceptor = new LocalSessionIdInterceptor(config(ttl, renewAfter)); + final SessionRoot session = + interceptor.afterCreation(initialSession, contextForArrivalTime(0)) + .toCompletableFuture().get(); + assertThat(session.get("key")).isNotPresent(); + } + private RequestContext context() { return contextForArrivalTime(0); } diff --git a/src/test/java/com/rewedigital/composer/session/Sessions.java b/src/test/java/com/rewedigital/composer/session/Sessions.java index e3f5182..e42d726 100644 --- a/src/test/java/com/rewedigital/composer/session/Sessions.java +++ b/src/test/java/com/rewedigital/composer/session/Sessions.java @@ -28,9 +28,13 @@ public static SessionRoot sessionRoot(final String key, final String value, fina } public static SessionRoot sessionRootExpiringAt(final long epochSeconds, final String key, final String value) { + return sessionRootExpiringAt(Long.toString(epochSeconds), key, value); + } + + public static SessionRoot sessionRootExpiringAt(final String expireAt, final String key, final String value) { final Map data = new HashMap<>(); data.put("x-rd-session-id", "1234"); - data.put("expires-at", Long.toString(epochSeconds)); + data.put("expires-at", expireAt); data.put(key, value); return SessionRoot.of(data); }