diff --git a/magnolia-module-cache/src/main/java/info/magnolia/module/cache/executor/Store.java b/magnolia-module-cache/src/main/java/info/magnolia/module/cache/executor/Store.java index b8ab97e..1df5207 100644 --- a/magnolia-module-cache/src/main/java/info/magnolia/module/cache/executor/Store.java +++ b/magnolia-module-cache/src/main/java/info/magnolia/module/cache/executor/Store.java @@ -42,10 +42,10 @@ import info.magnolia.module.cache.CachePolicyResult; import info.magnolia.module.cache.filter.CacheResponseWrapper; import info.magnolia.module.cache.filter.CachedEntry; import info.magnolia.module.cache.filter.CachedError; -import info.magnolia.module.cache.filter.ContentCachedEntry; import info.magnolia.module.cache.filter.CachedRedirect; -import info.magnolia.module.cache.filter.InMemoryCachedEntry; +import info.magnolia.module.cache.filter.ContentCachedEntry; import info.magnolia.module.cache.filter.DelegatingBlobCachedEntry; +import info.magnolia.module.cache.filter.InMemoryCachedEntry; import java.io.IOException; @@ -69,19 +69,29 @@ public class Store extends AbstractExecutor { final Object key = cachePolicyResult.getCacheKey(); final CacheResponseWrapper responseWrapper = new CacheResponseWrapper(response, CacheResponseWrapper.DEFAULT_THRESHOLD, false); - responseWrapper.setResponseExpirationDetectionEnabled(); // setting Last-Modified to when this resource was stored in the cache. This value might get overridden by further filters or servlets. final long cacheStorageDate = System.currentTimeMillis(); responseWrapper.setDateHeader("Last-Modified", cacheStorageDate); - chain.doFilter(request, responseWrapper); - - if (responseWrapper.getStatus() == HttpServletResponse.SC_NOT_MODIFIED) { - response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); - } - else { - responseWrapper.flushBuffer(); - cachedEntry = makeCachedEntry(request, responseWrapper); + try { + chain.doFilter(request, responseWrapper); + + if (responseWrapper.getStatus() == HttpServletResponse.SC_NOT_MODIFIED) { + response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); + } + else { + responseWrapper.flushBuffer(); + cachedEntry = makeCachedEntry(request, responseWrapper); + } + } catch (IOException e) { + responseWrapper.cleanUp(); + throw e; + } catch (ServletException e) { + responseWrapper.cleanUp(); + throw e; + } catch (Throwable e) { + responseWrapper.cleanUp(); + throw new RuntimeException("Failed to process request with: " + e.getMessage(), e); } if (cachedEntry == null) { diff --git a/magnolia-module-cache/src/main/java/info/magnolia/module/cache/filter/CacheResponseWrapper.java b/magnolia-module-cache/src/main/java/info/magnolia/module/cache/filter/CacheResponseWrapper.java index ff938b0..20945d4 100644 --- a/magnolia-module-cache/src/main/java/info/magnolia/module/cache/filter/CacheResponseWrapper.java +++ b/magnolia-module-cache/src/main/java/info/magnolia/module/cache/filter/CacheResponseWrapper.java @@ -59,6 +59,8 @@ import org.apache.commons.httpclient.util.DateUtil; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.io.output.ThresholdingOutputStream; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** @@ -67,12 +69,12 @@ import org.apache.commons.io.output.ThresholdingOutputStream; * {@link #getBufferedContent()}. Once the threshold is reached either a tmp file is created which * can be retrieved with {@link #getContentFile()} or the content/headers are made transparent to * the original response if {@link #serveIfThresholdReached} is true. - * - * @version $Id$ + * @version $Revision: 14052 $ ($Author: gjoseph $) */ public class CacheResponseWrapper extends HttpServletResponseWrapper { public static final int DEFAULT_THRESHOLD = 500 * 1024; + private static final Logger log = LoggerFactory.getLogger(CacheResponseWrapper.class); private final ServletOutputStream wrappedStream; private PrintWriter wrappedWriter = null; @@ -80,14 +82,14 @@ public class CacheResponseWrapper extends HttpServletResponseWrapper { private int status = SC_OK; private boolean isError; private String redirectionLocation; - private HttpServletResponse originalResponse; - private ByteArrayOutputStream inMemoryBuffer; + private final HttpServletResponse originalResponse; + private final ByteArrayOutputStream inMemoryBuffer; private File contentFile; private long contentLength = -1; - private ResponseExpirationCalculator responseExpirationCalculator; + private final ResponseExpirationCalculator responseExpirationCalculator = new ResponseExpirationCalculator(); - private ThresholdingOutputStream thresholdingOutputStream; - private boolean serveIfThresholdReached; + private final ThresholdingOutputStream thresholdingOutputStream; + private final boolean serveIfThresholdReached; private String errorMsg; @@ -113,25 +115,28 @@ public class CacheResponseWrapper extends HttpServletResponseWrapper { } // MAGNOLIA-1996: this can be called multiple times, e.g. by chunk writers, but always from a single thread. + @Override public ServletOutputStream getOutputStream() throws IOException { return wrappedStream; } - + public ThresholdingOutputStream getThresholdingOutputStream() throws IOException { return thresholdingOutputStream; } + @Override public PrintWriter getWriter() throws IOException { if (wrappedWriter == null) { String encoding = getCharacterEncoding(); wrappedWriter = encoding != null ? new PrintWriter(new OutputStreamWriter(getOutputStream(), encoding)) - : new PrintWriter(new OutputStreamWriter(getOutputStream())); + : new PrintWriter(new OutputStreamWriter(getOutputStream())); } return wrappedWriter; } + @Override public void flushBuffer() throws IOException { flush(); } @@ -145,27 +150,23 @@ public class CacheResponseWrapper extends HttpServletResponseWrapper { } + @Override public void reset() { super.reset(); -// if (wrappedStream instanceof ByteArrayOutputStream) { -// ((ByteArrayOutputStream)wrappedStream).reset(); -// } wrappedWriter = null; status = SC_OK; -// cookies.clear(); headers.clear(); -// contentType = null; -// contentLength = 0; - } + // cleanup temp file if any + cleanUp(); + } + @Override public void resetBuffer() { super.resetBuffer(); -// if (wrappedStream != null) { -// ((ByteArrayOutputStream)wrappedStream).reset(); -// } wrappedWriter = null; + cleanUp(); } public int getStatus() { @@ -375,17 +375,28 @@ public class CacheResponseWrapper extends HttpServletResponseWrapper { @Override protected void thresholdReached() throws IOException { - if(serveIfThresholdReached){ + if (serveIfThresholdReached) { replayHeadersAndStatus(originalResponse); out = originalResponse.getOutputStream(); - } - else{ + log.debug("Reached threshold for in-memory caching. Will not cache and stream response directly to user. Cache temp file is {}", contentFile.getAbsolutePath()); + } else { contentFile = File.createTempFile("cacheStream", null, Path.getTempDirectory()); + log.debug("Reached threshold for in-memory caching. Will continue caching in new cache temp file {}", contentFile.getAbsolutePath()); contentFile.deleteOnExit(); out = new FileOutputStream(contentFile); } out.write(getBufferedContent()); + // TODO: flush the buffer afterwards or even set it to null? should not be needed anymore so GC should be allowed to collect it. + } + } + + public void cleanUp() { + if (contentFile != null && contentFile.exists()) { + if (!contentFile.delete()) { + log.error("Can't delete file: " + contentFile); + } } + contentFile = null; } } diff --git a/magnolia-module-cache/src/main/java/info/magnolia/module/cache/filter/DelegatingBlobCachedEntry.java b/magnolia-module-cache/src/main/java/info/magnolia/module/cache/filter/DelegatingBlobCachedEntry.java index 8a9d0f4..244748f 100644 --- a/magnolia-module-cache/src/main/java/info/magnolia/module/cache/filter/DelegatingBlobCachedEntry.java +++ b/magnolia-module-cache/src/main/java/info/magnolia/module/cache/filter/DelegatingBlobCachedEntry.java @@ -62,7 +62,7 @@ public class DelegatingBlobCachedEntry extends ContentCachedEntry { private static final String CONTENT_FILE_ATTIBUTE = DelegatingBlobCachedEntry.class.getName() + ".contentFile"; - private long contentLength; + private final long contentLength; public DelegatingBlobCachedEntry(long contentLength, String contentType, String characterEncoding, int statusCode, MultiMap headers, long modificationDate, String originalUrl, int timeToLiveInSeconds) throws IOException { super(contentType, characterEncoding, statusCode, headers, modificationDate, originalUrl, timeToLiveInSeconds); @@ -74,9 +74,11 @@ public class DelegatingBlobCachedEntry extends ContentCachedEntry { return false; } + @Override public void replay(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { File contentFile = getContentFileBoundToTheRequest(request); if(contentFile != null){ + log.debug("About to serve response from {}", contentFile.getAbsolutePath()); super.replay(request, response, chain); } else{ @@ -89,11 +91,13 @@ public class DelegatingBlobCachedEntry extends ContentCachedEntry { response.setContentLength((int) contentLength); File contentFile = getContentFileBoundToTheRequest(request); if(contentFile != null){ + log.debug("Streaming out output from {}", contentFile.getAbsolutePath()); FileInputStream contentStream = FileUtils.openInputStream(contentFile); try{ IOUtils.copy(contentStream,response.getOutputStream()); }finally{ IOUtils.closeQuietly(contentStream); + log.debug("About to delete temp file {}", contentFile.getAbsolutePath()); if(!contentFile.delete()){ log.error("Can't delete file: " + contentFile); }