From c6305ff36ca498013e3fda5e03d0138b087fd860 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 26 Sep 2022 16:33:50 +1000 Subject: [PATCH 1/9] Improvements to CachingContentFactory for Jetty 12 Signed-off-by: Lachlan Roberts --- .../jetty/http/CachingContentFactory.java | 39 +- .../jetty/server/CachedContentFactory.java | 621 ------------------ .../jetty/server/ResourceContentFactory.java | 1 - .../jetty/server/handler/ResourceHandler.java | 2 - .../jetty/server/ResourceCacheTest.java | 259 ++------ .../server/handler/ResourceHandlerTest.java | 2 +- 6 files changed, 84 insertions(+), 840 deletions(-) delete mode 100644 jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java index b9bf99c1eb7a..d584eaf52825 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java @@ -43,6 +43,7 @@ */ public class CachingContentFactory implements HttpContent.ContentFactory { + private final CachingContentFactory _parent; private final HttpContent.ContentFactory _authority; private final boolean _useFileMappedBuffer; private final ConcurrentMap _cache = new ConcurrentHashMap<>(); @@ -58,6 +59,12 @@ public CachingContentFactory(HttpContent.ContentFactory authority) public CachingContentFactory(HttpContent.ContentFactory authority, boolean useFileMappedBuffer) { + this (null, authority, useFileMappedBuffer); + } + + public CachingContentFactory(CachingContentFactory parent, HttpContent.ContentFactory authority, boolean useFileMappedBuffer) + { + _parent = parent; _authority = authority; _useFileMappedBuffer = useFileMappedBuffer; } @@ -163,6 +170,30 @@ public void flushCache() } } + /** + * @param httpContent the resource to test + * @return whether the httpContent is cacheable. The default implementation tests the cache sizes. + */ + protected boolean isCacheable(HttpContent httpContent) + { + if (httpContent == null) + return false; + + if (httpContent.getResource().isDirectory()) + return false; + + if (_maxCachedFiles <= 0) + return false; + + // Will it fit in the cache? + long len = httpContent.getContentLengthValue(); + if (len <= 0) + return false; + if (isUseFileMappedBuffer()) + return true; + return ((len <= _maxCachedFileSize) && (len + getCachedSize() <= _maxCacheSize)); + } + @Override public HttpContent getContent(String path) throws IOException { @@ -178,14 +209,18 @@ public HttpContent getContent(String path) throws IOException } HttpContent httpContent = _authority.getContent(path); - // Do not cache directories or files that are too big - if (httpContent != null && !httpContent.getResource().isDirectory() && httpContent.getContentLengthValue() <= _maxCachedFileSize) + if (isCacheable(httpContent)) { httpContent = cachingHttpContent = new CachingHttpContent(path, httpContent); _cache.put(path, cachingHttpContent); _cachedSize.addAndGet(cachingHttpContent.calculateSize()); shrinkCache(); } + + // Is the content in the parent cache? + if (httpContent == null && _parent != null) + httpContent = _parent.getContent(path); + return httpContent; } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java deleted file mode 100644 index 390bcb407e38..000000000000 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java +++ /dev/null @@ -1,621 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.server; - -import java.io.IOException; -import java.nio.ByteBuffer; -import java.nio.file.Files; -import java.time.Instant; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.Map; -import java.util.SortedSet; -import java.util.TreeSet; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; - -import org.eclipse.jetty.http.CompressedContentFormat; -import org.eclipse.jetty.http.DateGenerator; -import org.eclipse.jetty.http.EtagUtils; -import org.eclipse.jetty.http.HttpContent; -import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.MimeTypes; -import org.eclipse.jetty.http.MimeTypes.Type; -import org.eclipse.jetty.http.PreEncodedHttpField; -import org.eclipse.jetty.http.PrecompressedHttpContent; -import org.eclipse.jetty.http.ResourceHttpContent; -import org.eclipse.jetty.util.BufferUtil; -import org.eclipse.jetty.util.resource.Resource; -import org.eclipse.jetty.util.resource.ResourceFactory; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -//TODO rework -public class CachedContentFactory implements HttpContent.ContentFactory -{ - private static final Logger LOG = LoggerFactory.getLogger(CachedContentFactory.class); - private static final Map NO_PRECOMPRESSED = Collections.unmodifiableMap(Collections.emptyMap()); - - private final ConcurrentMap _cache; - private final AtomicInteger _cachedSize; - private final AtomicInteger _cachedFiles; - private final ResourceFactory _factory; - private final CachedContentFactory _parent; - private final MimeTypes _mimeTypes; - private final boolean _etags; - private final CompressedContentFormat[] _precompressedFormats; - private final boolean _useFileMappedBuffer; - - private int _maxCachedFileSize = 128 * 1024 * 1024; - private int _maxCachedFiles = 2048; - private int _maxCacheSize = 256 * 1024 * 1024; - - /** - * Constructor. - * - * @param parent the parent resource cache - * @param factory the resource factory - * @param mimeTypes Mimetype to use for meta data - * @param useFileMappedBuffer true to file memory mapped buffers - * @param etags true to support etags - * @param precompressedFormats array of precompression formats to support - */ - public CachedContentFactory(CachedContentFactory parent, ResourceFactory factory, MimeTypes mimeTypes, boolean useFileMappedBuffer, boolean etags, CompressedContentFormat[] precompressedFormats) - { - _factory = factory; - _cache = new ConcurrentHashMap<>(); - _cachedSize = new AtomicInteger(); - _cachedFiles = new AtomicInteger(); - _mimeTypes = mimeTypes; - _parent = parent; - _useFileMappedBuffer = useFileMappedBuffer; - _etags = etags; - _precompressedFormats = precompressedFormats; - } - - public int getCachedSize() - { - return _cachedSize.get(); - } - - public int getCachedFiles() - { - return _cachedFiles.get(); - } - - public int getMaxCachedFileSize() - { - return _maxCachedFileSize; - } - - public void setMaxCachedFileSize(int maxCachedFileSize) - { - _maxCachedFileSize = maxCachedFileSize; - shrinkCache(); - } - - public int getMaxCacheSize() - { - return _maxCacheSize; - } - - public void setMaxCacheSize(int maxCacheSize) - { - _maxCacheSize = maxCacheSize; - shrinkCache(); - } - - /** - * @return the max number of cached files. - */ - public int getMaxCachedFiles() - { - return _maxCachedFiles; - } - - /** - * @param maxCachedFiles the max number of cached files. - */ - public void setMaxCachedFiles(int maxCachedFiles) - { - _maxCachedFiles = maxCachedFiles; - shrinkCache(); - } - - public boolean isUseFileMappedBuffer() - { - return _useFileMappedBuffer; - } - - public void flushCache() - { - while (_cache.size() > 0) - { - for (String path : _cache.keySet()) - { - CachedHttpContent content = _cache.remove(path); - if (content != null) - content.invalidate(); - } - } - } - - /** - *

Returns an entry from the cache, or creates a new one.

- * - * @param pathInContext The key into the cache - * @return The entry matching {@code pathInContext}, or a new entry - * if no matching entry was found. If the content exists but is not cacheable, - * then a {@link ResourceHttpContent} instance is returned. If - * the resource does not exist, then null is returned. - * @throws IOException if the resource cannot be retrieved - */ - @Override - public HttpContent getContent(String pathInContext) throws IOException - { - // Is the content in this cache? - CachedHttpContent content = _cache.get(pathInContext); - if (content != null && (content).isValid()) - return content; - - // try loading the content from our factory. - Resource resource = _factory.newResource(pathInContext); - HttpContent loaded = load(pathInContext, resource); - if (loaded != null) - return loaded; - - // Is the content in the parent cache? - if (_parent != null) - { - HttpContent httpContent = _parent.getContent(pathInContext); - if (httpContent != null) - return httpContent; - } - - return null; - } - - /** - * @param resource the resource to test - * @return whether the resource is cacheable. The default implementation tests the cache sizes. - */ - protected boolean isCacheable(Resource resource) - { - if (_maxCachedFiles <= 0) - return false; - - long len = resource.length(); - - // Will it fit in the cache? - return (len > 0 && (_useFileMappedBuffer || (len < _maxCachedFileSize && len < _maxCacheSize))); - } - - private HttpContent load(String pathInContext, Resource resource) throws IOException - { - if (resource == null || !resource.exists()) - return null; - - if (resource.isDirectory()) - return new ResourceHttpContent(resource, _mimeTypes.getMimeByExtension(resource.toString())); - - // Will it fit in the cache? - if (isCacheable(resource)) - { - CachedHttpContent content; - - // Look for precompressed resources - if (_precompressedFormats.length > 0) - { - Map precompresssedContents = new HashMap<>(_precompressedFormats.length); - for (CompressedContentFormat format : _precompressedFormats) - { - String compressedPathInContext = pathInContext + format.getExtension(); - CachedHttpContent compressedContent = _cache.get(compressedPathInContext); - if (compressedContent == null || compressedContent.isValid()) - { - compressedContent = null; - Resource compressedResource = _factory.newResource(compressedPathInContext); - if (compressedResource.exists() && compressedResource.lastModified().isAfter(resource.lastModified()) && - compressedResource.length() < resource.length()) - { - compressedContent = new CachedHttpContent(compressedPathInContext, compressedResource, null); - CachedHttpContent added = _cache.putIfAbsent(compressedPathInContext, compressedContent); - if (added != null) - { - compressedContent.invalidate(); - compressedContent = added; - } - } - } - if (compressedContent != null) - precompresssedContents.put(format, compressedContent); - } - content = new CachedHttpContent(pathInContext, resource, precompresssedContents); - } - else - content = new CachedHttpContent(pathInContext, resource, null); - - // Add it to the cache. - CachedHttpContent added = _cache.putIfAbsent(pathInContext, content); - if (added != null) - { - content.invalidate(); - content = added; - } - - return content; - } - - // Look for non Cacheable precompressed resource or content - String mt = _mimeTypes.getMimeByExtension(pathInContext); - if (_precompressedFormats.length > 0) - { - // Is the precompressed content cached? - Map compressedContents = new HashMap<>(); - for (CompressedContentFormat format : _precompressedFormats) - { - String compressedPathInContext = pathInContext + format.getExtension(); - CachedHttpContent compressedContent = _cache.get(compressedPathInContext); - if (compressedContent != null && compressedContent.isValid() && Files.getLastModifiedTime(compressedContent.getResource().getPath()).toInstant().isAfter(resource.lastModified())) - compressedContents.put(format, compressedContent); - - // Is there a precompressed resource? - Resource compressedResource = _factory.newResource(compressedPathInContext); - if (compressedResource.exists() && compressedResource.lastModified().isAfter(resource.lastModified()) && - compressedResource.length() < resource.length()) - compressedContents.put(format, - new ResourceHttpContent(compressedResource, _mimeTypes.getMimeByExtension(compressedPathInContext))); - } - if (!compressedContents.isEmpty()) - return new ResourceHttpContent(resource, mt, compressedContents); - } - - return new ResourceHttpContent(resource, mt); - } - - private void shrinkCache() - { - // While we need to shrink - while (_cache.size() > 0 && (_cachedFiles.get() > _maxCachedFiles || _cachedSize.get() > _maxCacheSize)) - { - // Scan the entire cache and generate an ordered list by last accessed time. - SortedSet sorted = new TreeSet<>( - Comparator.comparing((CachedHttpContent c) -> c._lastAccessed) - .thenComparingLong(c -> c._contentLengthValue) - .thenComparing(c -> c._key)); - sorted.addAll(_cache.values()); - - // Invalidate least recently used first - for (CachedHttpContent content : sorted) - { - if (_cachedFiles.get() <= _maxCachedFiles && _cachedSize.get() <= _maxCacheSize) - break; - if (content == _cache.remove(content.getKey())) - content.invalidate(); - } - } - } - - protected ByteBuffer getIndirectBuffer(Resource resource) - { - try - { - return BufferUtil.toBuffer(resource, false); - } - catch (IOException | IllegalArgumentException e) - { - if (LOG.isDebugEnabled()) - LOG.debug("Unable to get Indirect Buffer for {}", resource, e); - } - return null; - } - - protected ByteBuffer getMappedBuffer(Resource resource) - { - // Only use file mapped buffers for cached resources, otherwise too much virtual memory commitment for - // a non shared resource. Also ignore max buffer size - try - { - if (_useFileMappedBuffer && resource.getPath() != null && resource.length() <= Integer.MAX_VALUE) - return BufferUtil.toMappedBuffer(resource); - } - catch (IOException | IllegalArgumentException e) - { - if (LOG.isDebugEnabled()) - LOG.debug("Unable to get Mapped Buffer for {}", resource, e); - } - return null; - } - - protected ByteBuffer getDirectBuffer(Resource resource) - { - try - { - return BufferUtil.toBuffer(resource, true); - } - catch (IOException | IllegalArgumentException e) - { - if (LOG.isDebugEnabled()) - LOG.debug("Unable to get Direct Buffer for {}", resource, e); - } - return null; - } - - @Override - public String toString() - { - return "ResourceCache[" + _parent + "," + _factory + "]@" + hashCode(); - } - - /** - * MetaData associated with a context Resource. - */ - public class CachedHttpContent implements HttpContent - { - private final String _key; - private final Resource _resource; - private final long _contentLengthValue; - private final HttpField _contentType; - private final String _characterEncoding; - private final MimeTypes.Type _mimeType; - private final HttpField _contentLength; - private final HttpField _lastModified; - private final Instant _lastModifiedValue; - private final HttpField _etag; - private final Map _precompressed; - private final AtomicReference _indirectBuffer = new AtomicReference<>(); - private final AtomicReference _directBuffer = new AtomicReference<>(); - private final AtomicReference _mappedBuffer = new AtomicReference<>(); - private volatile Instant _lastAccessed; - - CachedHttpContent(String pathInContext, Resource resource, Map precompressedResources) - { - _key = pathInContext; - _resource = resource; - - String contentType = _mimeTypes.getMimeByExtension(_resource.toString()); - _contentType = contentType == null ? null : new PreEncodedHttpField(HttpHeader.CONTENT_TYPE, contentType); - _characterEncoding = _contentType == null ? null : MimeTypes.getCharsetFromContentType(contentType); - _mimeType = _contentType == null ? null : MimeTypes.CACHE.get(MimeTypes.getContentTypeWithoutCharset(contentType)); - - boolean exists = resource.exists(); - _lastModifiedValue = exists ? resource.lastModified() : null; - _lastModified = _lastModifiedValue == null ? null - : new PreEncodedHttpField(HttpHeader.LAST_MODIFIED, DateGenerator.formatDate(_lastModifiedValue)); - - _contentLengthValue = exists ? resource.length() : 0; - _contentLength = new PreEncodedHttpField(HttpHeader.CONTENT_LENGTH, Long.toString(_contentLengthValue)); - - if (_cachedFiles.incrementAndGet() > _maxCachedFiles) - shrinkCache(); - - _lastAccessed = Instant.now(); - - _etag = CachedContentFactory.this._etags ? new PreEncodedHttpField(HttpHeader.ETAG, EtagUtils.computeWeakEtag(resource.getPath())) : null; - - if (precompressedResources != null) - { - _precompressed = new HashMap<>(precompressedResources.size()); - for (Map.Entry entry : precompressedResources.entrySet()) - { - _precompressed.put(entry.getKey(), new CachedPrecompressedHttpContent(this, entry.getValue(), entry.getKey())); - } - } - else - { - _precompressed = NO_PRECOMPRESSED; - } - } - - public String getKey() - { - return _key; - } - - public boolean isCached() - { - return _key != null; - } - - @Override - public Resource getResource() - { - return _resource; - } - - @Override - public HttpField getETag() - { - return _etag; - } - - @Override - public String getETagValue() - { - return _etag.getValue(); - } - - boolean isValid() - { - if (_lastModifiedValue == _resource.lastModified() && _contentLengthValue == _resource.length()) - { - _lastAccessed = Instant.now(); - return true; - } - - if (this == _cache.remove(_key)) - invalidate(); - return false; - } - - protected void invalidate() - { - ByteBuffer indirect = _indirectBuffer.getAndSet(null); - if (indirect != null) - _cachedSize.addAndGet(-BufferUtil.length(indirect)); - - ByteBuffer direct = _directBuffer.getAndSet(null); - if (direct != null) - _cachedSize.addAndGet(-BufferUtil.length(direct)); - - _mappedBuffer.getAndSet(null); - - _cachedFiles.decrementAndGet(); - } - - @Override - public HttpField getLastModified() - { - return _lastModified; - } - - @Override - public String getLastModifiedValue() - { - return _lastModified == null ? null : _lastModified.getValue(); - } - - @Override - public HttpField getContentType() - { - return _contentType; - } - - @Override - public String getContentTypeValue() - { - return _contentType == null ? null : _contentType.getValue(); - } - - @Override - public HttpField getContentEncoding() - { - return null; - } - - @Override - public String getContentEncodingValue() - { - return null; - } - - @Override - public String getCharacterEncoding() - { - return _characterEncoding; - } - - @Override - public Type getMimeType() - { - return _mimeType; - } - - @Override - public HttpField getContentLength() - { - return _contentLength; - } - - @Override - public long getContentLengthValue() - { - return _contentLengthValue; - } - - @Override - public String toString() - { - return String.format("CachedContent@%x{r=%s,e=%b,lm=%s,ct=%s,c=%d}", hashCode(), _resource, _resource.exists(), _lastModified, _contentType, _precompressed.size()); - } - - @Override - public Map getPrecompressedContents() - { - if (_precompressed.size() == 0) - return null; - Map ret = _precompressed; - for (Map.Entry entry : _precompressed.entrySet()) - { - if (!entry.getValue().isValid()) - { - if (ret == _precompressed) - ret = new HashMap<>(_precompressed); - ret.remove(entry.getKey()); - } - } - return ret; - } - - @Override - public ByteBuffer getBuffer() - { - return _indirectBuffer.get(); - } - - @Override - public void release() - { - invalidate(); - } - } - - public class CachedPrecompressedHttpContent extends PrecompressedHttpContent - { - private final CachedHttpContent _content; - private final CachedHttpContent _precompressedContent; - private final HttpField _etag; - - CachedPrecompressedHttpContent(CachedHttpContent content, CachedHttpContent precompressedContent, CompressedContentFormat format) - { - super(content, precompressedContent, format); - _content = content; - _precompressedContent = precompressedContent; - - // _etag = (CachedContentFactory.this._etags) ? new PreEncodedHttpField(HttpHeader.ETAG, _content.getResource().getWeakETag(format.getEtagSuffix())) : null; - _etag = null; - } - - public boolean isValid() - { - return _precompressedContent.isValid() && _content.isValid(); // && _content.getResource().lastModified() <= _precompressedContent.getResource().lastModified(); - } - - @Override - public HttpField getETag() - { - if (_etag != null) - return _etag; - return super.getETag(); - } - - @Override - public String getETagValue() - { - if (_etag != null) - return _etag.getValue(); - return super.getETagValue(); - } - - @Override - public String toString() - { - return "Cached" + super.toString(); - } - } -} diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java index e4110315e0c8..21102f77e5db 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java @@ -33,7 +33,6 @@ * this factory are not intended to be cached, so memory limits for individual * HttpOutput streams are enforced. */ -//TODO remove public class ResourceContentFactory implements ContentFactory { private final ResourceFactory _factory; diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java index 476096962743..fe40d8e81ad0 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java @@ -272,7 +272,6 @@ public void setPrecompressedFormats(CompressedContentFormat... precompressedForm public void setPrecompressedFormats(List precompressedFormats) { _resourceService.setPrecompressedFormats(precompressedFormats); - setupContentFactory(); } public void setEncodingCacheSize(int encodingCacheSize) @@ -288,7 +287,6 @@ public int getEncodingCacheSize() public void setMimeTypes(MimeTypes mimeTypes) { _mimeTypes = mimeTypes; - setupContentFactory(); } /** diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResourceCacheTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResourceCacheTest.java index 32b2d42e8f51..a0ec6fa94cd0 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResourceCacheTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResourceCacheTest.java @@ -14,47 +14,38 @@ package org.eclipse.jetty.server; import java.io.BufferedWriter; -import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; -import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.List; +import java.util.function.Predicate; import java.util.stream.Stream; +import org.eclipse.jetty.http.CachingContentFactory; import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.http.MimeTypes; -import org.eclipse.jetty.http.ResourceHttpContent; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; -import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.resource.FileSystemPool; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceCollection; import org.eclipse.jetty.util.resource.ResourceFactory; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.not; -import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; @ExtendWith(WorkDirExtension.class) -@Disabled // TODO public class ResourceCacheTest { public WorkDir workDir; @@ -126,6 +117,23 @@ private void makeFile(Path file, String contents) throws IOException } } + private static CachingContentFactory newCachingContentFactory(CachingContentFactory parent, ResourceFactory factory, MimeTypes mimeTypes, boolean useFileMappedBuffer) + { + return newCachingContentFactory(parent, factory, mimeTypes, useFileMappedBuffer, c -> true); + } + + private static CachingContentFactory newCachingContentFactory(CachingContentFactory parent, ResourceFactory factory, MimeTypes mimeTypes, boolean useFileMappedBuffer, Predicate predicate) + { + return new CachingContentFactory(parent, new ResourceContentFactory(factory, mimeTypes, List.of(CompressedContentFormat.NONE)), useFileMappedBuffer) + { + @Override + protected boolean isCacheable(HttpContent httpContent) + { + return super.isCacheable(httpContent) && predicate.test(httpContent); + } + }; + } + @Test public void testMultipleSources1() throws Exception { @@ -141,21 +149,21 @@ public void testMultipleSources1() throws Exception List r = rc.getResources(); MimeTypes mime = new MimeTypes(); - CachedContentFactory rc3 = new CachedContentFactory(null, ResourceFactory.of(r.get(2)), mime, false, false, CompressedContentFormat.NONE); - CachedContentFactory rc2 = new CachedContentFactory(rc3, ResourceFactory.of(r.get(1)), mime, false, false, CompressedContentFormat.NONE); - CachedContentFactory rc1 = new CachedContentFactory(rc2, ResourceFactory.of(r.get(0)), mime, false, false, CompressedContentFormat.NONE); + CachingContentFactory rc3 = newCachingContentFactory(null, ResourceFactory.of(r.get(2)), mime, false); + CachingContentFactory rc2 = newCachingContentFactory(rc3, ResourceFactory.of(r.get(1)), mime, false); + CachingContentFactory rc1 = newCachingContentFactory(rc2, ResourceFactory.of(r.get(0)), mime, false); - assertEquals(getContent(rc1, "1.txt"), "1 - one"); - assertEquals(getContent(rc1, "2.txt"), "2 - two"); - assertEquals(getContent(rc1, "3.txt"), "3 - three"); + assertEquals("1 - one", getContent(rc1, "1.txt")); + assertEquals("2 - two", getContent(rc1, "2.txt")); + assertEquals("3 - three", getContent(rc1, "3.txt")); - assertEquals(getContent(rc2, "1.txt"), "1 - two"); - assertEquals(getContent(rc2, "2.txt"), "2 - two"); - assertEquals(getContent(rc2, "3.txt"), "3 - three"); + assertEquals("1 - two", getContent(rc2, "1.txt")); + assertEquals("2 - two", getContent(rc2, "2.txt")); + assertEquals("3 - three", getContent(rc2, "3.txt")); assertNull(getContent(rc3, "1.txt")); - assertEquals(getContent(rc3, "2.txt"), "2 - three"); - assertEquals(getContent(rc3, "3.txt"), "3 - three"); + assertEquals("2 - three", getContent(rc3, "2.txt")); + assertEquals("3 - three", getContent(rc3, "3.txt")); } @Test @@ -173,198 +181,23 @@ public void testUncacheable() throws Exception List r = rc.getResources(); MimeTypes mime = new MimeTypes(); - CachedContentFactory rc3 = new CachedContentFactory(null, ResourceFactory.of(r.get(2)), mime, false, false, CompressedContentFormat.NONE); - CachedContentFactory rc2 = new CachedContentFactory(rc3, ResourceFactory.of(r.get(1)), mime, false, false, CompressedContentFormat.NONE) - { - @Override - public boolean isCacheable(Resource resource) - { - return super.isCacheable(resource) && !resource.getFileName().equals("2.txt"); - } - }; + CachingContentFactory rc3 = newCachingContentFactory(null, ResourceFactory.of(r.get(2)), mime, false); + CachingContentFactory rc2 = newCachingContentFactory(rc3, ResourceFactory.of(r.get(1)), mime, false, + httpContent -> !httpContent.getResource().getFileName().equals("2.txt")); - CachedContentFactory rc1 = new CachedContentFactory(rc2, ResourceFactory.of(r.get(0)), mime, false, false, CompressedContentFormat.NONE); + CachingContentFactory rc1 = newCachingContentFactory(rc2, ResourceFactory.of(r.get(0)), mime, false); - assertEquals(getContent(rc1, "1.txt"), "1 - one"); - assertEquals(getContent(rc1, "2.txt"), "2 - two"); - assertEquals(getContent(rc1, "3.txt"), "3 - three"); + assertEquals("1 - one", getContent(rc1, "1.txt")); + assertEquals("2 - two", getContent(rc1, "2.txt")); + assertEquals("3 - three", getContent(rc1, "3.txt")); - assertEquals(getContent(rc2, "1.txt"), "1 - two"); - assertEquals(getContent(rc2, "2.txt"), "2 - two"); - assertEquals(getContent(rc2, "3.txt"), "3 - three"); + assertEquals("1 - two", getContent(rc2, "1.txt")); + assertEquals("2 - two", getContent(rc2, "2.txt")); + assertEquals("3 - three", getContent(rc2, "3.txt")); assertNull(getContent(rc3, "1.txt")); - assertEquals(getContent(rc3, "2.txt"), "2 - three"); - assertEquals(getContent(rc3, "3.txt"), "3 - three"); - } - - @Test - public void testResourceCache() throws Exception - { - final Resource directory; - File[] files = new File[10]; - String[] names = new String[files.length]; - CachedContentFactory cache; - - Path basePath = workDir.getEmptyPathDir(); - - for (int i = 0; i < files.length; i++) - { - Path tmpFile = basePath.resolve("R-" + i + ".txt"); - try (BufferedWriter writer = Files.newBufferedWriter(tmpFile, UTF_8, StandardOpenOption.CREATE_NEW)) - { - for (int j = 0; j < (i * 10 - 1); j++) - { - writer.write(' '); - } - writer.write('\n'); - } - files[i] = tmpFile.toFile(); - names[i] = tmpFile.getFileName().toString(); - } - - directory = ResourceFactory.root().newResource(files[0].getParentFile().getAbsolutePath()); - - cache = new CachedContentFactory(null, ResourceFactory.of(directory), new MimeTypes(), false, false, CompressedContentFormat.NONE); - - cache.setMaxCacheSize(95); - cache.setMaxCachedFileSize(85); - cache.setMaxCachedFiles(4); - - assertNull(cache.getContent("does not exist")); - assertTrue(cache.getContent(names[9]) instanceof ResourceHttpContent); - assertNotNull(cache.getContent(names[9]).getBuffer()); - - HttpContent content; - content = cache.getContent(names[8]); - assertThat(content, is(not(nullValue()))); - assertEquals(80, content.getContentLengthValue()); - assertEquals(0, cache.getCachedSize()); - - if (org.junit.jupiter.api.condition.OS.LINUX.isCurrentOs()) - { - // Initially not using memory mapped files - content.getBuffer(); - assertEquals(80, cache.getCachedSize()); - - // with both types of buffer loaded, this is too large for cache - content.getBuffer(); - assertEquals(0, cache.getCachedSize()); - assertEquals(0, cache.getCachedFiles()); - - cache = new CachedContentFactory(null, ResourceFactory.of(directory), new MimeTypes(), true, false, CompressedContentFormat.NONE); - cache.setMaxCacheSize(95); - cache.setMaxCachedFileSize(85); - cache.setMaxCachedFiles(4); - - content = cache.getContent(names[8]); - content.getBuffer(); - assertEquals(cache.isUseFileMappedBuffer() ? 0 : 80, cache.getCachedSize()); - - // with both types of buffer loaded, this is not too large for cache because - // mapped buffers don't count, so we can continue - } - - content.getBuffer(); - assertEquals(80, cache.getCachedSize()); - assertEquals(1, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[1]); - assertEquals(80, cache.getCachedSize()); - content.getBuffer(); - assertEquals(90, cache.getCachedSize()); - assertEquals(2, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[2]); - content.getBuffer(); - assertEquals(30, cache.getCachedSize()); - assertEquals(2, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[3]); - content.getBuffer(); - assertEquals(60, cache.getCachedSize()); - assertEquals(3, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[4]); - content.getBuffer(); - assertEquals(90, cache.getCachedSize()); - assertEquals(3, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[5]); - content.getBuffer(); - assertEquals(90, cache.getCachedSize()); - assertEquals(2, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[6]); - content.getBuffer(); - assertEquals(60, cache.getCachedSize()); - assertEquals(1, cache.getCachedFiles()); - - Thread.sleep(200); - - try (OutputStream out = new FileOutputStream(files[6])) - { - out.write(' '); - } - content = cache.getContent(names[7]); - content.getBuffer(); - assertEquals(70, cache.getCachedSize()); - assertEquals(1, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[6]); - content.getBuffer(); - assertEquals(71, cache.getCachedSize()); - assertEquals(2, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[0]); - content.getBuffer(); - assertEquals(72, cache.getCachedSize()); - assertEquals(3, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[1]); - content.getBuffer(); - assertEquals(82, cache.getCachedSize()); - assertEquals(4, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[2]); - content.getBuffer(); - assertEquals(32, cache.getCachedSize()); - assertEquals(4, cache.getCachedFiles()); - - Thread.sleep(200); - - content = cache.getContent(names[3]); - content.getBuffer(); - assertEquals(61, cache.getCachedSize()); - assertEquals(4, cache.getCachedFiles()); - - Thread.sleep(200); - - cache.flushCache(); - assertEquals(0, cache.getCachedSize()); - assertEquals(0, cache.getCachedFiles()); - - cache.flushCache(); + assertEquals("2 - three", getContent(rc3, "2.txt")); + assertEquals("3 - three", getContent(rc3, "3.txt")); } @Test @@ -375,18 +208,18 @@ public void testNoextension() throws Exception Resource resource = ResourceFactory.root().newResource(basePath.resolve("four")); MimeTypes mime = new MimeTypes(); - CachedContentFactory cache = new CachedContentFactory(null, ResourceFactory.of(resource), mime, false, false, CompressedContentFormat.NONE); + CachingContentFactory cache = newCachingContentFactory(null, ResourceFactory.of(resource), mime, false); assertEquals(getContent(cache, "four.txt"), "4 - four"); assertEquals(getContent(cache, "four"), "4 - four (no extension)"); } - static String getContent(CachedContentFactory rc, String path) throws Exception + static String getContent(CachingContentFactory rc, String path) throws Exception { HttpContent content = rc.getContent(path); if (content == null) return null; - return BufferUtil.toString(content.getBuffer()); + return IO.toString(content.getResource().newInputStream()); } } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java index 32ab5ef68a25..e2d194789bcd 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java @@ -1211,7 +1211,6 @@ public void testCachingFilesCached() throws Exception public void testCachingMaxCacheSizeRespected() throws Exception { copySimpleTestResource(docRoot); - // TODO explicitly turn on caching long expectedSize = Files.size(docRoot.resolve("simple.txt")); CachingContentFactory contentFactory = (CachingContentFactory)_rootResourceHandler.getContentFactory(); contentFactory.setMaxCacheSize((int)expectedSize); @@ -1362,6 +1361,7 @@ public void testCachingNotFoundNotCached() throws Exception public void testCachingPrecompressedFilesCached() throws Exception { setupBigFiles(docRoot); + long expectedSize = Files.size(docRoot.resolve("big.txt")) + Files.size(docRoot.resolve("big.txt.gz")); From bed034c22dd335bac6de0d2ce28cf8134320c9af Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 27 Sep 2022 11:01:56 +1000 Subject: [PATCH 2/9] more fixes for test cases in DefaultServletTest Signed-off-by: Lachlan Roberts --- .../eclipse/jetty/http/PrecompressedHttpContent.java | 2 +- .../jetty/ee9/nested/CachedContentFactory.java | 12 +++++++----- .../jetty/ee9/nested/ResourceContentFactory.java | 2 +- .../jetty/ee9/servlet/DefaultServletTest.java | 9 --------- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java index ce4b19f08004..80fe4f60ab24 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java @@ -139,7 +139,7 @@ public Map getPrecompressedContents() @Override public ByteBuffer getBuffer() { - return _content.getBuffer(); + return _precompressedContent.getBuffer(); } @Override diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java index b65a2c2173e8..3d413f9c73e0 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java @@ -21,6 +21,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.SortedSet; import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; @@ -230,7 +231,7 @@ private HttpContent load(String pathInContext, Resource resource) throws IOExcep { compressedContent = null; Resource compressedResource = _factory.newResource(compressedPathInContext); - if (compressedResource.exists() && compressedResource.lastModified().isAfter(resource.lastModified()) && + if (compressedResource.exists() && compressedResource.lastModified().compareTo(resource.lastModified()) >= 0 && compressedResource.length() < resource.length()) { compressedContent = new CachedHttpContent(compressedPathInContext, compressedResource, null); @@ -271,12 +272,12 @@ private HttpContent load(String pathInContext, Resource resource) throws IOExcep { String compressedPathInContext = pathInContext + format.getExtension(); CachedHttpContent compressedContent = _cache.get(compressedPathInContext); - if (compressedContent != null && compressedContent.isValid() && compressedContent.getResource().lastModified().isAfter(resource.lastModified())) + if (compressedContent != null && compressedContent.isValid() && compressedContent.getResource().lastModified().compareTo(resource.lastModified()) >= 0) compressedContents.put(format, compressedContent); // Is there a precompressed resource? Resource compressedResource = _factory.newResource(compressedPathInContext); - if (compressedResource.exists() && compressedResource.lastModified().isAfter(resource.lastModified()) && + if (compressedResource.exists() && compressedResource.lastModified().compareTo(resource.lastModified()) >= 0 && compressedResource.length() < resource.length()) compressedContents.put(format, new ResourceHttpContent(compressedResource, _mimeTypes.getMimeByExtension(compressedPathInContext))); @@ -452,7 +453,7 @@ public String getETagValue() boolean isValid() { - if (_lastModifiedValue == _resource.lastModified() && _contentLengthValue == _resource.length()) + if (Objects.equals(_lastModifiedValue, _resource.lastModified()) && _contentLengthValue == _resource.length()) { _lastAccessed = Instant.now(); return true; @@ -611,7 +612,8 @@ public class CachedPrecompressedHttpContent extends PrecompressedHttpContent public boolean isValid() { - return _precompressedContent.isValid() && _content.isValid() && _content.getResource().lastModified().isBefore(_precompressedContent.getResource().lastModified()); + return _precompressedContent.isValid() && _content.isValid() && + _content.getResource().lastModified().compareTo(_precompressedContent.getResource().lastModified()) >= 0; } @Override diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceContentFactory.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceContentFactory.java index 571825debf27..199661393057 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceContentFactory.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceContentFactory.java @@ -87,7 +87,7 @@ private HttpContent load(String pathInContext, Resource resource) { String compressedPathInContext = pathInContext + format.getExtension(); Resource compressedResource = _factory.newResource(compressedPathInContext); - if (compressedResource != null && compressedResource.exists() && compressedResource.lastModified().isAfter(resource.lastModified()) && + if (compressedResource != null && compressedResource.exists() && compressedResource.lastModified().compareTo(resource.lastModified()) >= 0 && compressedResource.length() < resource.length()) compressedContents.put(format, new ResourceHttpContent(compressedResource, _mimeTypes.getMimeByExtension(compressedPathInContext))); diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java index 0e80ef41771f..8cbd2137bebe 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java @@ -358,7 +358,6 @@ public void testListingFilenamesOnlyUrlResource() throws Exception } @Test - @Disabled // TODO public void testListingProperUrlEncoding() throws Exception { ServletHolder defholder = context.addServlet(DefaultServlet.class, "/*"); @@ -962,7 +961,6 @@ public void testRelativeRedirect() throws Exception * Ensure that oddball directory names are served with proper escaping */ @Test - @Disabled // TODO public void testWelcomeRedirectDirWithQuestion() throws Exception { FS.ensureDirExists(docRoot); @@ -995,7 +993,6 @@ public void testWelcomeRedirectDirWithQuestion() throws Exception * Ensure that oddball directory names are served with proper escaping */ @Test - @Disabled // TODO public void testWelcomeRedirectDirWithSemicolon() throws Exception { FS.ensureDirExists(docRoot); @@ -1553,7 +1550,6 @@ public void testFiltered() throws Exception } @Test - @Disabled // TODO public void testGzip() throws Exception { FS.ensureDirExists(docRoot); @@ -1648,7 +1644,6 @@ public void testGzip() throws Exception } @Test - @Disabled // TODO public void testCachedGzip() throws Exception { FS.ensureDirExists(docRoot); @@ -1731,7 +1726,6 @@ public void testCachedGzip() throws Exception } @Test - @Disabled // TODO public void testBrotli() throws Exception { createFile(docRoot.resolve("data0.txt"), "Hello Text 0"); @@ -1819,7 +1813,6 @@ public void testBrotli() throws Exception } @Test - @Disabled // TODO public void testCachedBrotli() throws Exception { createFile(docRoot.resolve("data0.txt"), "Hello Text 0"); @@ -1899,7 +1892,6 @@ public void testCachedBrotli() throws Exception } @Test - @Disabled // TODO public void testDefaultBrotliOverGzip() throws Exception { createFile(docRoot.resolve("data0.txt"), "Hello Text 0"); @@ -1936,7 +1928,6 @@ public void testDefaultBrotliOverGzip() throws Exception } @Test - @Disabled // TODO public void testCustomCompressionFormats() throws Exception { createFile(docRoot.resolve("data0.txt"), "Hello Text 0"); From c5b4ddee873c160ac88c9bdcaf04d4b527909afa Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 27 Sep 2022 12:40:23 +1000 Subject: [PATCH 3/9] Fix ETag header in 304 responses for ResourceService, provide precompressedFormats to DefaultServlet Signed-off-by: Lachlan Roberts --- .../main/java/org/eclipse/jetty/server/ResourceService.java | 4 +++- .../java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java | 5 ++--- .../org/eclipse/jetty/ee10/servlet/DefaultServletTest.java | 6 ------ 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 9d00c68859bc..975e0a270456 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -365,8 +365,10 @@ protected boolean passConditionalHeaders(Request request, Response response, Htt String matched = matchesEtag(etag, ifnm); if (matched != null) { + // If we do sendError it clears the ETag header. + response.setStatus(HttpStatus.NOT_MODIFIED_304); response.getHeaders().put(HttpHeader.ETAG, matched); - writeHttpError(request, response, callback, HttpStatus.NOT_MODIFIED_304); + response.write(true, null, callback); return true; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java index 529a18c81a2e..00297e683608 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java @@ -117,8 +117,7 @@ public void init() throws ServletException // TODO: should this come from context? MimeTypes mimeTypes = new MimeTypes(); - // TODO: this is configured further down below - see _resourceService.setPrecompressedFormats - List precompressedFormats = List.of(); + List precompressedFormats = parsePrecompressedFormats(getInitParameter("precompressed"), getInitBoolean("gzip"), _resourceService.getPrecompressedFormats()); _useFileMappedBuffer = getInitBoolean("useFileMappedBuffer", _useFileMappedBuffer); ResourceContentFactory resourceContentFactory = new ResourceContentFactory(ResourceFactory.of(_baseResource), mimeTypes, precompressedFormats); @@ -156,7 +155,7 @@ public void init() throws ServletException _resourceService.setAcceptRanges(getInitBoolean("acceptRanges", _resourceService.isAcceptRanges())); _resourceService.setDirAllowed(getInitBoolean("dirAllowed", _resourceService.isDirAllowed())); _resourceService.setRedirectWelcome(getInitBoolean("redirectWelcome", _resourceService.isRedirectWelcome())); - _resourceService.setPrecompressedFormats(parsePrecompressedFormats(getInitParameter("precompressed"), getInitBoolean("gzip"), _resourceService.getPrecompressedFormats())); + _resourceService.setPrecompressedFormats(precompressedFormats); _resourceService.setEtags(getInitBoolean("etags", _resourceService.isEtags())); _isPathInfoOnly = getInitBoolean("pathInfoOnly", _isPathInfoOnly); diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java index efefe2cce08e..78ae28d4f905 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java @@ -2090,7 +2090,6 @@ public void testWriterAndCharsetFiltered() throws Exception } @Test - @Disabled public void testGzip() throws Exception { FS.ensureDirExists(docRoot); @@ -2243,7 +2242,6 @@ public void testGzip() throws Exception } @Test - @Disabled public void testCachedGzip() throws Exception { FS.ensureDirExists(docRoot); @@ -2371,7 +2369,6 @@ public void testCachedGzip() throws Exception } @Test - @Disabled public void testBrotli() throws Exception { Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); @@ -2511,7 +2508,6 @@ public void testBrotli() throws Exception } @Test - @Disabled public void testCachedBrotli() throws Exception { Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); @@ -2636,7 +2632,6 @@ public void testCachedBrotli() throws Exception } @Test - @Disabled public void testDefaultBrotliOverGzip() throws Exception { Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); @@ -2685,7 +2680,6 @@ public void testDefaultBrotliOverGzip() throws Exception } @Test - @Disabled public void testCustomCompressionFormats() throws Exception { Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); From b503d93360d75d63816696c3e22569353237d95d Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 27 Sep 2022 13:40:16 +1000 Subject: [PATCH 4/9] fix some more tests in DefaultServletTest for EE10 Signed-off-by: Lachlan Roberts --- .../jetty/ee10/servlet/DefaultServlet.java | 5 +++ .../ee10/servlet/DefaultServletTest.java | 31 ++++++++++++------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java index 00297e683608..88d8a0c76ecc 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java @@ -92,6 +92,11 @@ public class DefaultServlet extends HttpServlet private boolean _isPathInfoOnly = false; + public ResourceService getResourceService() + { + return _resourceService; + } + @Override public void init() throws ServletException { diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java index 78ae28d4f905..40676d4c8108 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java @@ -2730,7 +2730,6 @@ public void testCustomCompressionFormats() throws Exception } @Test - @Disabled public void testProgrammaticCustomCompressionFormats() throws Exception { Files.writeString(docRoot.resolve("data0.txt"), "Hello Text 0", UTF_8); @@ -2738,13 +2737,22 @@ public void testProgrammaticCustomCompressionFormats() throws Exception Files.writeString(docRoot.resolve("data0.txt.gz"), "fake gzip", UTF_8); Files.writeString(docRoot.resolve("data0.txt.bz2"), "fake bzip2", UTF_8); - ResourceService resourceService = new ResourceService(); - resourceService.setPrecompressedFormats(List.of( - new CompressedContentFormat("bzip2", ".bz2"), - new CompressedContentFormat("gzip", ".gz"), - new CompressedContentFormat("br", ".br") - )); - ServletHolder defholder = new ServletHolder(new DefaultServlet()); // TODO: how to integrate resource service / precompressed format + DefaultServlet defaultServlet = new DefaultServlet(); + ServletHolder defholder = new ServletHolder(defaultServlet) + { + @Override + public void initialize() throws Exception + { + super.initialize(); + ResourceService resourceService = defaultServlet.getResourceService(); + resourceService.setPrecompressedFormats(List.of( + new CompressedContentFormat("bzip2", ".bz2"), + new CompressedContentFormat("gzip", ".gz"), + new CompressedContentFormat("br", ".br") + )); + } + }; + context.addServlet(defholder, "/"); defholder.setInitParameter("resourceBase", docRoot.toString()); @@ -2904,7 +2912,6 @@ public void testIfModified(String content) throws Exception "Hello World", "Now is the time for all good men to come to the aid of the party" }) - @Disabled public void testIfETag(String content) throws Exception { Files.writeString(docRoot.resolve("file.txt"), content, UTF_8); @@ -2947,7 +2954,7 @@ public void testIfETag(String content) throws Exception Connection:close\r If-None-Match: wibble,@ETAG@,wobble\r \r - """.replace("@ETAG", etag)); + """.replace("@ETAG@", etag)); response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.NOT_MODIFIED_304)); @@ -2977,7 +2984,7 @@ public void testIfETag(String content) throws Exception Connection:close\r If-Match: @ETAG@\r \r - """.replace("@ETAG", etag)); + """.replace("@ETAG@", etag)); response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); @@ -2987,7 +2994,7 @@ public void testIfETag(String content) throws Exception Connection:close\r If-Match: wibble,@ETAG@,wobble\r \r - """.replace("@ETAG", etag)); + """.replace("@ETAG@", etag)); response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); From c58f76911787ee09bc0995e532f5ddd297211011 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 27 Sep 2022 14:15:12 +1000 Subject: [PATCH 5/9] refactor test for resource lastModified comparisons Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/util/resource/Resource.java | 2 +- .../jetty/ee9/nested/CachedContentFactory.java | 8 ++++---- .../jetty/ee9/nested/ResourceContentFactory.java | 13 ++++++++++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index 8db2c97142bd..d2b62368f352 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -84,7 +84,7 @@ public static String dump(Resource resource) { if (resource == null) return "null exists=false directory=false lm=-1"; - return "%s exists=%b directory=%b lm=%d" + return "%s exists=%b directory=%b lm=%s" .formatted(resource.toString(), resource.exists(), resource.isDirectory(), resource.lastModified()); } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java index 3d413f9c73e0..8942264fc2fa 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/CachedContentFactory.java @@ -231,7 +231,7 @@ private HttpContent load(String pathInContext, Resource resource) throws IOExcep { compressedContent = null; Resource compressedResource = _factory.newResource(compressedPathInContext); - if (compressedResource.exists() && compressedResource.lastModified().compareTo(resource.lastModified()) >= 0 && + if (compressedResource.exists() && ResourceContentFactory.newerThanOrEqual(compressedResource, resource) && compressedResource.length() < resource.length()) { compressedContent = new CachedHttpContent(compressedPathInContext, compressedResource, null); @@ -272,12 +272,12 @@ private HttpContent load(String pathInContext, Resource resource) throws IOExcep { String compressedPathInContext = pathInContext + format.getExtension(); CachedHttpContent compressedContent = _cache.get(compressedPathInContext); - if (compressedContent != null && compressedContent.isValid() && compressedContent.getResource().lastModified().compareTo(resource.lastModified()) >= 0) + if (compressedContent != null && compressedContent.isValid() && ResourceContentFactory.newerThanOrEqual(compressedContent.getResource(), resource)) compressedContents.put(format, compressedContent); // Is there a precompressed resource? Resource compressedResource = _factory.newResource(compressedPathInContext); - if (compressedResource.exists() && compressedResource.lastModified().compareTo(resource.lastModified()) >= 0 && + if (compressedResource.exists() && ResourceContentFactory.newerThanOrEqual(compressedResource, resource) && compressedResource.length() < resource.length()) compressedContents.put(format, new ResourceHttpContent(compressedResource, _mimeTypes.getMimeByExtension(compressedPathInContext))); @@ -613,7 +613,7 @@ public class CachedPrecompressedHttpContent extends PrecompressedHttpContent public boolean isValid() { return _precompressedContent.isValid() && _content.isValid() && - _content.getResource().lastModified().compareTo(_precompressedContent.getResource().lastModified()) >= 0; + ResourceContentFactory.newerThanOrEqual(_precompressedContent.getResource(), _content.getResource()); } @Override diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceContentFactory.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceContentFactory.java index 199661393057..b6464db9c6a1 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceContentFactory.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceContentFactory.java @@ -87,7 +87,7 @@ private HttpContent load(String pathInContext, Resource resource) { String compressedPathInContext = pathInContext + format.getExtension(); Resource compressedResource = _factory.newResource(compressedPathInContext); - if (compressedResource != null && compressedResource.exists() && compressedResource.lastModified().compareTo(resource.lastModified()) >= 0 && + if (compressedResource != null && compressedResource.exists() && ResourceContentFactory.newerThanOrEqual(compressedResource, resource) && compressedResource.length() < resource.length()) compressedContents.put(format, new ResourceHttpContent(compressedResource, _mimeTypes.getMimeByExtension(compressedPathInContext))); @@ -98,6 +98,17 @@ private HttpContent load(String pathInContext, Resource resource) return new ResourceHttpContent(resource, mt); } + /** + *

Utility to compare {@link Resource#lastModified()} of two resources.

+ * @param resource1 the first resource to test. + * @param resource2 the second resource to test. + * @return true if modified time of resource1 is newer or equal to that of resource2. + */ + static boolean newerThanOrEqual(Resource resource1, Resource resource2) + { + return !resource2.lastModified().isAfter(resource1.lastModified()); + } + @Override public String toString() { From ae071c5a9915a2eaf271091f8424c52f0b3e39a6 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 27 Sep 2022 22:18:54 +1000 Subject: [PATCH 6/9] release precompressed content buffer in PrecompressedHttpContent & use HttpContent.ContentFactory for parent in CachingContentFactory Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/http/CachingContentFactory.java | 4 ++-- .../java/org/eclipse/jetty/http/PrecompressedHttpContent.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java index d584eaf52825..1e3a6763fb68 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java @@ -43,7 +43,7 @@ */ public class CachingContentFactory implements HttpContent.ContentFactory { - private final CachingContentFactory _parent; + private final HttpContent.ContentFactory _parent; private final HttpContent.ContentFactory _authority; private final boolean _useFileMappedBuffer; private final ConcurrentMap _cache = new ConcurrentHashMap<>(); @@ -62,7 +62,7 @@ public CachingContentFactory(HttpContent.ContentFactory authority, boolean useFi this (null, authority, useFileMappedBuffer); } - public CachingContentFactory(CachingContentFactory parent, HttpContent.ContentFactory authority, boolean useFileMappedBuffer) + public CachingContentFactory(HttpContent.ContentFactory parent, HttpContent.ContentFactory authority, boolean useFileMappedBuffer) { _parent = parent; _authority = authority; diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java index 80fe4f60ab24..31155ec28115 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java @@ -145,6 +145,6 @@ public ByteBuffer getBuffer() @Override public void release() { - _content.release(); + _precompressedContent.release(); } } From 1508eb61e7ed35b937ef5ac8c295ddecc6b1ee42 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 28 Sep 2022 10:50:03 +1000 Subject: [PATCH 7/9] keep the ETag header in ServletContextResponse.resetContent if Response is 304 Signed-off-by: Lachlan Roberts --- .../main/java/org/eclipse/jetty/server/ResourceService.java | 4 +--- .../eclipse/jetty/ee10/servlet/ServletContextResponse.java | 5 ++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 975e0a270456..9d00c68859bc 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -365,10 +365,8 @@ protected boolean passConditionalHeaders(Request request, Response response, Htt String matched = matchesEtag(etag, ifnm); if (matched != null) { - // If we do sendError it clears the ETag header. - response.setStatus(HttpStatus.NOT_MODIFIED_304); response.getHeaders().put(HttpHeader.ETAG, matched); - response.write(true, null, callback); + writeHttpError(request, response, callback, HttpStatus.NOT_MODIFIED_304); return true; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java index eee0d2efc130..63488f7c4af6 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java @@ -310,11 +310,14 @@ public void resetContent() case CACHE_CONTROL: case LAST_MODIFIED: case EXPIRES: - case ETAG: case DATE: case VARY: i.remove(); continue; + case ETAG: + if (getStatus() != HttpStatus.NOT_MODIFIED_304) + i.remove(); + continue; default: } } From 98f9deed430b9e7f21119a055d1661d773e2e628 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 29 Sep 2022 22:20:00 +1000 Subject: [PATCH 8/9] remove parent from CachingContentFactory Signed-off-by: Lachlan Roberts --- .../jetty/http/CachingContentFactory.java | 11 - .../jetty/server/ResourceCacheTest.java | 225 ------------------ 2 files changed, 236 deletions(-) delete mode 100644 jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResourceCacheTest.java diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java index 1e3a6763fb68..3de8a5928e32 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java @@ -43,7 +43,6 @@ */ public class CachingContentFactory implements HttpContent.ContentFactory { - private final HttpContent.ContentFactory _parent; private final HttpContent.ContentFactory _authority; private final boolean _useFileMappedBuffer; private final ConcurrentMap _cache = new ConcurrentHashMap<>(); @@ -59,12 +58,6 @@ public CachingContentFactory(HttpContent.ContentFactory authority) public CachingContentFactory(HttpContent.ContentFactory authority, boolean useFileMappedBuffer) { - this (null, authority, useFileMappedBuffer); - } - - public CachingContentFactory(HttpContent.ContentFactory parent, HttpContent.ContentFactory authority, boolean useFileMappedBuffer) - { - _parent = parent; _authority = authority; _useFileMappedBuffer = useFileMappedBuffer; } @@ -217,10 +210,6 @@ public HttpContent getContent(String path) throws IOException shrinkCache(); } - // Is the content in the parent cache? - if (httpContent == null && _parent != null) - httpContent = _parent.getContent(path); - return httpContent; } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResourceCacheTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResourceCacheTest.java deleted file mode 100644 index a0ec6fa94cd0..000000000000 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResourceCacheTest.java +++ /dev/null @@ -1,225 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.server; - -import java.io.BufferedWriter; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.StandardOpenOption; -import java.util.List; -import java.util.function.Predicate; -import java.util.stream.Stream; - -import org.eclipse.jetty.http.CachingContentFactory; -import org.eclipse.jetty.http.CompressedContentFormat; -import org.eclipse.jetty.http.HttpContent; -import org.eclipse.jetty.http.MimeTypes; -import org.eclipse.jetty.toolchain.test.FS; -import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; -import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; -import org.eclipse.jetty.util.IO; -import org.eclipse.jetty.util.resource.FileSystemPool; -import org.eclipse.jetty.util.resource.Resource; -import org.eclipse.jetty.util.resource.ResourceCollection; -import org.eclipse.jetty.util.resource.ResourceFactory; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; - -import static java.nio.charset.StandardCharsets.UTF_8; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.empty; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; - -@ExtendWith(WorkDirExtension.class) -public class ResourceCacheTest -{ - public WorkDir workDir; - - @BeforeEach - public void beforeEach() - { - assertThat(FileSystemPool.INSTANCE.mounts(), empty()); - } - - @AfterEach - public void afterEach() - { - assertThat(FileSystemPool.INSTANCE.mounts(), empty()); - } - - public Path createUtilTestResources(Path basePath) throws IOException - { - // root - makeFile(basePath.resolve("resource.txt"), "this is test data"); - - // - one/ - Path one = basePath.resolve("one"); - FS.ensureDirExists(one); - makeFile(one.resolve("1.txt"), "1 - one"); - - // - one/dir/ - Path oneDir = one.resolve("dir"); - FS.ensureDirExists(oneDir); - makeFile(oneDir.resolve("1.txt"), "1 - one"); - - // - two/ - Path two = basePath.resolve("two"); - FS.ensureDirExists(two); - makeFile(two.resolve("1.txt"), "1 - two"); - makeFile(two.resolve("2.txt"), "2 - two"); - - // - two/dir/ - Path twoDir = two.resolve("dir"); - FS.ensureDirExists(twoDir); - makeFile(twoDir.resolve("2.txt"), "2 - two"); - - // - three/ - Path three = basePath.resolve("three"); - FS.ensureDirExists(three); - makeFile(three.resolve("2.txt"), "2 - three"); - makeFile(three.resolve("3.txt"), "3 - three"); - - // - three/dir/ - Path threeDir = three.resolve("dir"); - FS.ensureDirExists(threeDir); - makeFile(threeDir.resolve("3.txt"), "3 - three"); - - // - four/ - Path four = basePath.resolve("four"); - FS.ensureDirExists(four); - makeFile(four.resolve("four"), "4 - four (no extension)"); - makeFile(four.resolve("four.txt"), "4 - four"); - - return basePath; - } - - private void makeFile(Path file, String contents) throws IOException - { - try (BufferedWriter writer = Files.newBufferedWriter(file, UTF_8, StandardOpenOption.CREATE_NEW)) - { - writer.write(contents); - writer.flush(); - } - } - - private static CachingContentFactory newCachingContentFactory(CachingContentFactory parent, ResourceFactory factory, MimeTypes mimeTypes, boolean useFileMappedBuffer) - { - return newCachingContentFactory(parent, factory, mimeTypes, useFileMappedBuffer, c -> true); - } - - private static CachingContentFactory newCachingContentFactory(CachingContentFactory parent, ResourceFactory factory, MimeTypes mimeTypes, boolean useFileMappedBuffer, Predicate predicate) - { - return new CachingContentFactory(parent, new ResourceContentFactory(factory, mimeTypes, List.of(CompressedContentFormat.NONE)), useFileMappedBuffer) - { - @Override - protected boolean isCacheable(HttpContent httpContent) - { - return super.isCacheable(httpContent) && predicate.test(httpContent); - } - }; - } - - @Test - public void testMultipleSources1() throws Exception - { - Path basePath = createUtilTestResources(workDir.getEmptyPathDir()); - - List resourceList = Stream.of("one", "two", "three") - .map(basePath::resolve) - .map(ResourceFactory.root()::newResource) - .toList(); - - ResourceCollection rc = Resource.combine(resourceList); - - List r = rc.getResources(); - MimeTypes mime = new MimeTypes(); - - CachingContentFactory rc3 = newCachingContentFactory(null, ResourceFactory.of(r.get(2)), mime, false); - CachingContentFactory rc2 = newCachingContentFactory(rc3, ResourceFactory.of(r.get(1)), mime, false); - CachingContentFactory rc1 = newCachingContentFactory(rc2, ResourceFactory.of(r.get(0)), mime, false); - - assertEquals("1 - one", getContent(rc1, "1.txt")); - assertEquals("2 - two", getContent(rc1, "2.txt")); - assertEquals("3 - three", getContent(rc1, "3.txt")); - - assertEquals("1 - two", getContent(rc2, "1.txt")); - assertEquals("2 - two", getContent(rc2, "2.txt")); - assertEquals("3 - three", getContent(rc2, "3.txt")); - - assertNull(getContent(rc3, "1.txt")); - assertEquals("2 - three", getContent(rc3, "2.txt")); - assertEquals("3 - three", getContent(rc3, "3.txt")); - } - - @Test - public void testUncacheable() throws Exception - { - Path basePath = createUtilTestResources(workDir.getEmptyPathDir()); - - List resourceList = Stream.of("one", "two", "three") - .map(basePath::resolve) - .map(ResourceFactory.root()::newResource) - .toList(); - - ResourceCollection rc = Resource.combine(resourceList); - - List r = rc.getResources(); - MimeTypes mime = new MimeTypes(); - - CachingContentFactory rc3 = newCachingContentFactory(null, ResourceFactory.of(r.get(2)), mime, false); - CachingContentFactory rc2 = newCachingContentFactory(rc3, ResourceFactory.of(r.get(1)), mime, false, - httpContent -> !httpContent.getResource().getFileName().equals("2.txt")); - - CachingContentFactory rc1 = newCachingContentFactory(rc2, ResourceFactory.of(r.get(0)), mime, false); - - assertEquals("1 - one", getContent(rc1, "1.txt")); - assertEquals("2 - two", getContent(rc1, "2.txt")); - assertEquals("3 - three", getContent(rc1, "3.txt")); - - assertEquals("1 - two", getContent(rc2, "1.txt")); - assertEquals("2 - two", getContent(rc2, "2.txt")); - assertEquals("3 - three", getContent(rc2, "3.txt")); - - assertNull(getContent(rc3, "1.txt")); - assertEquals("2 - three", getContent(rc3, "2.txt")); - assertEquals("3 - three", getContent(rc3, "3.txt")); - } - - @Test - public void testNoextension() throws Exception - { - Path basePath = createUtilTestResources(workDir.getEmptyPathDir()); - - Resource resource = ResourceFactory.root().newResource(basePath.resolve("four")); - MimeTypes mime = new MimeTypes(); - - CachingContentFactory cache = newCachingContentFactory(null, ResourceFactory.of(resource), mime, false); - - assertEquals(getContent(cache, "four.txt"), "4 - four"); - assertEquals(getContent(cache, "four"), "4 - four (no extension)"); - } - - static String getContent(CachingContentFactory rc, String path) throws Exception - { - HttpContent content = rc.getContent(path); - if (content == null) - return null; - - return IO.toString(content.getResource().newInputStream()); - } -} From 1c9ac9c846103b1607d71537dc0d22c64117b3fa Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 30 Sep 2022 10:34:25 +1000 Subject: [PATCH 9/9] Update javadoc for CachingContentFactory.isCacheable() Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/http/CachingContentFactory.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java index 3de8a5928e32..eb9b8eaea71a 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingContentFactory.java @@ -164,8 +164,10 @@ public void flushCache() } /** - * @param httpContent the resource to test - * @return whether the httpContent is cacheable. The default implementation tests the cache sizes. + * Tests whether the given HttpContent is cacheable, and if there is enough room to fit it in the cache. + * + * @param httpContent the HttpContent to test. + * @return whether the HttpContent is cacheable. */ protected boolean isCacheable(HttpContent httpContent) {