Polish [CssLinkResource|AppCacheManifest]Transformer

This commit updates the two transformers to make them more
consistent with updates of their spring-web-reactive equivalents.

Issue: SPR-14521

spring-webmvc/src/main/java/org/springframework/web/servlet/resource/AppCacheManifestTransformer.java

@@ -21,9 +21,9 @@ import java.io.IOException;
 import java.io.StringWriter;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
 import java.util.Scanner;
 import javax.servlet.http.HttpServletRequest;
 
@@ -59,15 +59,18 @@ import org.springframework.util.StringUtils;
  */
 public class AppCacheManifestTransformer extends ResourceTransformerSupport {
 
+	private static final Collection<String> MANIFEST_SECTION_HEADERS =
+			Arrays.asList("CACHE MANIFEST", "NETWORK:", "FALLBACK:", "CACHE:");
+
 	private static final String MANIFEST_HEADER = "CACHE MANIFEST";
 
+	private static final String CACHE_HEADER = "CACHE:";
+
 	private static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8;
 
 	private static final Log logger = LogFactory.getLog(AppCacheManifestTransformer.class);
 
 
-	private final Map<String, SectionTransformer> sectionTransformers = new HashMap<>();
-
 	private final String fileExtension;
 
 
@@ -84,20 +87,14 @@ public class AppCacheManifestTransformer extends ResourceTransformerSupport {
 	 */
 	public AppCacheManifestTransformer(String fileExtension) {
 		this.fileExtension = fileExtension;
-
-		SectionTransformer noOpSection = new NoOpSection();
-		this.sectionTransformers.put(MANIFEST_HEADER, noOpSection);
-		this.sectionTransformers.put("NETWORK:", noOpSection);
-		this.sectionTransformers.put("FALLBACK:", noOpSection);
-		this.sectionTransformers.put("CACHE:", new CacheSection());
 	}
 
 
 	@Override
-	public Resource transform(HttpServletRequest request, Resource resource, ResourceTransformerChain transformerChain)
-			throws IOException {
+	public Resource transform(HttpServletRequest request, Resource resource,
+			ResourceTransformerChain chain) throws IOException {
 
-		resource = transformerChain.transform(request, resource);
+		resource = chain.transform(request, resource);
 		if (!this.fileExtension.equals(StringUtils.getFilenameExtension(resource.getFilename()))) {
 			return resource;
 		}
@@ -107,7 +104,7 @@ public class AppCacheManifestTransformer extends ResourceTransformerSupport {
 
 		if (!content.startsWith(MANIFEST_HEADER)) {
 			if (logger.isTraceEnabled()) {
-				logger.trace("AppCache manifest does not start with 'CACHE MANIFEST', skipping: " + resource);
+				logger.trace("Manifest should start with 'CACHE MANIFEST', skip: " + resource);
 			}
 			return resource;
 		}
@@ -116,111 +113,145 @@ public class AppCacheManifestTransformer extends ResourceTransformerSupport {
 			logger.trace("Transforming resource: " + resource);
 		}
 
-		StringWriter contentWriter = new StringWriter();
-		HashBuilder hashBuilder = new HashBuilder(content.length());
-
 		Scanner scanner = new Scanner(content);
-		SectionTransformer currentTransformer = this.sectionTransformers.get(MANIFEST_HEADER);
-		while (scanner.hasNextLine()) {
+		LineInfo previous = null;
+		LineAggregator aggregator = new LineAggregator(resource, content);
+
+		while (scanner.hasNext()) {
 			String line = scanner.nextLine();
-			if (this.sectionTransformers.containsKey(line.trim())) {
-				currentTransformer = this.sectionTransformers.get(line.trim());
-				contentWriter.write(line + "\n");
-				hashBuilder.appendString(line);
-			}
-			else {
-				contentWriter.write(
-						currentTransformer.transform(line, hashBuilder, resource, transformerChain, request)  + "\n");
-			}
+			LineInfo current = new LineInfo(line, previous);
+			LineOutput lineOutput = processLine(current, request, resource, chain);
+			aggregator.add(lineOutput);
+			previous = current;
 		}
 
-		String hash = hashBuilder.build();
-		contentWriter.write("\n" + "# Hash: " + hash);
+		return aggregator.createResource();
+	}
+
+	private static byte[] getResourceBytes(Resource resource) throws IOException {
+		return FileCopyUtils.copyToByteArray(resource.getInputStream());
+	}
+
+	private LineOutput processLine(LineInfo info, HttpServletRequest request,
+			Resource resource, ResourceTransformerChain transformerChain) {
+
+		if (!info.isLink()) {
+			return new LineOutput(info.getLine(), null);
+		}
+
+		Resource appCacheResource = transformerChain.getResolverChain()
+				.resolveResource(null, info.getLine(), Collections.singletonList(resource));
+
+		String path = resolveUrlPath(info.getLine(), request, resource, transformerChain);
 		if (logger.isTraceEnabled()) {
-			logger.trace("AppCache file: [" + resource.getFilename()+ "] hash: [" + hash + "]");
+			logger.trace("Link modified: " + path + " (original: " + info.getLine() + ")");
 		}
 
-		return new TransformedResource(resource, contentWriter.toString().getBytes(DEFAULT_CHARSET));
+		return new LineOutput(path, appCacheResource);
 	}
 
 
-	@FunctionalInterface
-	private interface SectionTransformer {
+	private static class LineInfo {
 
-		/**
-		 * Transforms a line in a section of the manifest.
-		 * <p>The actual transformation depends on the chosen transformation strategy
-		 * for the current manifest section (CACHE, NETWORK, FALLBACK, etc).
-		 */
-		String transform(String line, HashBuilder builder, Resource resource,
-				ResourceTransformerChain transformerChain, HttpServletRequest request) throws IOException;
-	}
+		private final String line;
 
+		private final boolean cacheSection;
 
-	private static class NoOpSection implements SectionTransformer {
+		private final boolean link;
 
-		public String transform(String line, HashBuilder builder, Resource resource,
-				ResourceTransformerChain transformerChain, HttpServletRequest request) throws IOException {
 
-			builder.appendString(line);
-			return line;
+		public LineInfo(String line, LineInfo previous) {
+			this.line = line;
+			this.cacheSection = initCacheSectionFlag(line, previous);
+			this.link = iniLinkFlag(line, this.cacheSection);
+		}
+
+		private static boolean initCacheSectionFlag(String line, LineInfo previousLine) {
+			if (MANIFEST_SECTION_HEADERS.contains(line.trim())) {
+				return line.trim().equals(CACHE_HEADER);
+			}
+			else if (previousLine != null) {
+				return previousLine.isCacheSection();
+			}
+			throw new IllegalStateException(
+					"Manifest does not start with " + MANIFEST_HEADER + ": " + line);
+		}
+
+		private static boolean iniLinkFlag(String line, boolean isCacheSection) {
+			return (isCacheSection && StringUtils.hasText(line) && !line.startsWith("#")
+					&& !line.startsWith("//") && !hasScheme(line));
+		}
+
+		private static boolean hasScheme(String line) {
+			int index = line.indexOf(":");
+			return (line.startsWith("//") || (index > 0 && !line.substring(0, index).contains("/")));
+		}
+
+
+		public String getLine() {
+			return this.line;
+		}
+
+		public boolean isCacheSection() {
+			return this.cacheSection;
+		}
+
+		public boolean isLink() {
+			return this.link;
 		}
 	}
 
+	private static class LineOutput {
 
-	private class CacheSection implements SectionTransformer {
+		private final String line;
 
-		private static final String COMMENT_DIRECTIVE = "#";
+		private final Resource resource;
 
-		@Override
-		public String transform(String line, HashBuilder builder, Resource resource,
-				ResourceTransformerChain transformerChain, HttpServletRequest request) throws IOException {
 
-			if (isLink(line) && !hasScheme(line)) {
-				ResourceResolverChain resolverChain = transformerChain.getResolverChain();
-				Resource appCacheResource =
-						resolverChain.resolveResource(null, line, Collections.singletonList(resource));
-				String path = resolveUrlPath(line, request, resource, transformerChain);
-				builder.appendResource(appCacheResource);
-				if (logger.isTraceEnabled()) {
-					logger.trace("Link modified: " + path + " (original: " + line + ")");
-				}
-				return path;
-			}
-			builder.appendString(line);
-			return line;
+		public LineOutput(String line, Resource resource) {
+			this.line = line;
+			this.resource = resource;
 		}
 
-		private boolean hasScheme(String link) {
-			int schemeIndex = link.indexOf(":");
-			return (link.startsWith("//") || (schemeIndex > 0 && !link.substring(0, schemeIndex).contains("/")));
+		public String getLine() {
+			return this.line;
 		}
 
-		private boolean isLink(String line) {
-			return (StringUtils.hasText(line) && !line.startsWith(COMMENT_DIRECTIVE));
+		public Resource getResource() {
+			return this.resource;
 		}
 	}
 
+	private static class LineAggregator {
 
-	private static class HashBuilder {
+		private final StringWriter writer = new StringWriter();
 
 		private final ByteArrayOutputStream baos;
 
-		public HashBuilder(int initialSize) {
-			this.baos = new ByteArrayOutputStream(initialSize);
-		}
+		private final Resource resource;
+
 
-		public void appendResource(Resource resource) throws IOException {
-			byte[] content = FileCopyUtils.copyToByteArray(resource.getInputStream());
-			this.baos.write(DigestUtils.md5Digest(content));
+		public LineAggregator(Resource resource, String content) {
+			this.resource = resource;
+			this.baos = new ByteArrayOutputStream(content.length());
 		}
 
-		public void appendString(String content) throws IOException {
-			this.baos.write(content.getBytes(DEFAULT_CHARSET));
+		public void add(LineOutput lineOutput) throws IOException {
+			this.writer.write(lineOutput.getLine() + "\n");
+			byte[] bytes = (lineOutput.getResource() != null ?
+					DigestUtils.md5Digest(getResourceBytes(lineOutput.getResource())) :
+					lineOutput.getLine().getBytes(DEFAULT_CHARSET));
+			this.baos.write(bytes);
 		}
 
-		public String build() {
-			return DigestUtils.md5DigestAsHex(this.baos.toByteArray());
+		public TransformedResource createResource() {
+			String hash = DigestUtils.md5DigestAsHex(this.baos.toByteArray());
+			this.writer.write("\n" + "# Hash: " + hash);
+			if (logger.isTraceEnabled()) {
+				logger.trace("AppCache file: [" + resource.getFilename()+ "] hash: [" + hash + "]");
+			}
+			byte[] bytes = this.writer.toString().getBytes(DEFAULT_CHARSET);
+			return new TransformedResource(this.resource, bytes);
 		}
 	}
 

spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CssLinkResourceTransformer.java

@@ -54,12 +54,12 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 
 	private static final Log logger = LogFactory.getLog(CssLinkResourceTransformer.class);
 
-	private final List<CssLinkParser> linkParsers = new ArrayList<>(2);
+	private final List<LinkParser> linkParsers = new ArrayList<>(2);
 
 
 	public CssLinkResourceTransformer() {
-		this.linkParsers.add(new ImportStatementCssLinkParser());
-		this.linkParsers.add(new UrlFunctionCssLinkParser());
+		this.linkParsers.add(new ImportStatementLinkParser());
+		this.linkParsers.add(new UrlFunctionLinkParser());
 	}
 
 
@@ -81,26 +81,25 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 		byte[] bytes = FileCopyUtils.copyToByteArray(resource.getInputStream());
 		String content = new String(bytes, DEFAULT_CHARSET);
 
-		Set<CssLinkInfo> infos = new HashSet<>(8);
-		for (CssLinkParser parser : this.linkParsers) {
-			parser.parseLink(content, infos);
+		List<Segment> linkSegments = new ArrayList<>(8);
+		for (LinkParser parser : this.linkParsers) {
+			linkSegments.addAll(parser.parseLink(content));
 		}
 
-		if (infos.isEmpty()) {
+		if (linkSegments.isEmpty()) {
 			if (logger.isTraceEnabled()) {
 				logger.trace("No links found.");
 			}
 			return resource;
 		}
 
-		List<CssLinkInfo> sortedInfos = new ArrayList<>(infos);
-		Collections.sort(sortedInfos);
+		Collections.sort(linkSegments);
 
 		int index = 0;
 		StringWriter writer = new StringWriter();
-		for (CssLinkInfo info : sortedInfos) {
-			writer.write(content.substring(index, info.getStart()));
-			String link = content.substring(info.getStart(), info.getEnd());
+		for (Segment linkSegment : linkSegments) {
+			writer.write(content.substring(index, linkSegment.getStart()));
+			String link = content.substring(linkSegment.getStart(), linkSegment.getEnd());
 			String newLink = null;
 			if (!hasScheme(link)) {
 				newLink = resolveUrlPath(link, request, resource, transformerChain);
@@ -114,7 +113,7 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 				}
 			}
 			writer.write(newLink != null ? newLink : link);
-			index = info.getEnd();
+			index = linkSegment.getEnd();
 		}
 		writer.write(content.substring(index));
 
@@ -128,13 +127,14 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 
 
 	@FunctionalInterface
-	protected interface CssLinkParser {
+	protected interface LinkParser {
+
+		Set<Segment> parseLink(String content);
 
-		void parseLink(String content, Set<CssLinkInfo> linkInfos);
 	}
 
 
-	protected static abstract class AbstractCssLinkParser implements CssLinkParser {
+	protected static abstract class AbstractLinkParser implements LinkParser {
 
 		/**
 		 * Return the keyword to use to search for links.
@@ -142,7 +142,8 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 		protected abstract String getKeyword();
 
 		@Override
-		public void parseLink(String content, Set<CssLinkInfo> linkInfos) {
+		public Set<Segment> parseLink(String content) {
+			Set<Segment> linksToAdd = new HashSet<>(8);
 			int index = 0;
 			do {
 				index = content.indexOf(getKeyword(), index);
@@ -151,17 +152,18 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 				}
 				index = skipWhitespace(content, index + getKeyword().length());
 				if (content.charAt(index) == '\'') {
-					index = addLink(index, "'", content, linkInfos);
+					index = addLink(index, "'", content, linksToAdd);
 				}
 				else if (content.charAt(index) == '"') {
-					index = addLink(index, "\"", content, linkInfos);
+					index = addLink(index, "\"", content, linksToAdd);
 				}
 				else {
-					index = extractLink(index, content, linkInfos);
+					index = extractLink(index, content, linksToAdd);
 
 				}
 			}
 			while (true);
+			return linksToAdd;
 		}
 
 		private int skipWhitespace(String content, int index) {
@@ -174,10 +176,10 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 			}
 		}
 
-		protected int addLink(int index, String endKey, String content, Set<CssLinkInfo> linkInfos) {
+		protected int addLink(int index, String endKey, String content, Set<Segment> linksToAdd) {
 			int start = index + 1;
 			int end = content.indexOf(endKey, start);
-			linkInfos.add(new CssLinkInfo(start, end));
+			linksToAdd.add(new Segment(start, end));
 			return end + endKey.length();
 		}
 
@@ -185,12 +187,12 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 		 * Invoked after a keyword match, after whitespaces removed, and when
 		 * the next char is neither a single nor double quote.
 		 */
-		protected abstract int extractLink(int index, String content, Set<CssLinkInfo> linkInfos);
+		protected abstract int extractLink(int index, String content, Set<Segment> linksToAdd);
 
 	}
 
 
-	private static class ImportStatementCssLinkParser extends AbstractCssLinkParser {
+	private static class ImportStatementLinkParser extends AbstractLinkParser {
 
 		@Override
 		protected String getKeyword() {
@@ -198,7 +200,7 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 		}
 
 		@Override
-		protected int extractLink(int index, String content, Set<CssLinkInfo> linkInfos) {
+		protected int extractLink(int index, String content, Set<Segment> linksToAdd) {
 			if (content.substring(index, index + 4).equals("url(")) {
 				// Ignore, UrlLinkParser will take care
 			}
@@ -210,7 +212,7 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 	}
 
 
-	private static class UrlFunctionCssLinkParser extends AbstractCssLinkParser {
+	private static class UrlFunctionLinkParser extends AbstractLinkParser {
 
 		@Override
 		protected String getKeyword() {
@@ -218,20 +220,20 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 		}
 
 		@Override
-		protected int extractLink(int index, String content, Set<CssLinkInfo> linkInfos) {
+		protected int extractLink(int index, String content, Set<Segment> linksToAdd) {
 			// A url() function without unquoted
-			return addLink(index - 1, ")", content, linkInfos);
+			return addLink(index - 1, ")", content, linksToAdd);
 		}
 	}
 
 
-	private static class CssLinkInfo implements Comparable<CssLinkInfo> {
+	private static class Segment implements Comparable<Segment> {
 
 		private final int start;
 
 		private final int end;
 
-		public CssLinkInfo(int start, int end) {
+		public Segment(int start, int end) {
 			this.start = start;
 			this.end = end;
 		}
@@ -245,7 +247,7 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 		}
 
 		@Override
-		public int compareTo(CssLinkInfo other) {
+		public int compareTo(Segment other) {
 			return (this.start < other.start ? -1 : (this.start == other.start ? 0 : 1));
 		}
 
@@ -254,8 +256,8 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 			if (this == obj) {
 				return true;
 			}
-			if (obj != null && obj instanceof CssLinkInfo) {
-				CssLinkInfo other = (CssLinkInfo) obj;
+			if (obj != null && obj instanceof Segment) {
+				Segment other = (Segment) obj;
 				return (this.start == other.start && this.end == other.end);
 			}
 			return false;
@@ -267,4 +269,4 @@ public class CssLinkResourceTransformer extends ResourceTransformerSupport {
 		}
 	}
 
-}
+}