diff --git a/commafeed-client/src/components/metrics/Gauge.tsx b/commafeed-client/src/components/metrics/Gauge.tsx index 74b77147..d026671a 100644 --- a/commafeed-client/src/components/metrics/Gauge.tsx +++ b/commafeed-client/src/components/metrics/Gauge.tsx @@ -1,3 +1,4 @@ +import { NumberFormatter } from "@mantine/core" import type { MetricGauge } from "app/types" interface MeterProps { @@ -5,5 +6,5 @@ interface MeterProps { } export function Gauge(props: MeterProps) { - return {props.gauge.value} + return } diff --git a/commafeed-client/src/pages/admin/MetricsPage.tsx b/commafeed-client/src/pages/admin/MetricsPage.tsx index d90e6227..c64260ab 100644 --- a/commafeed-client/src/pages/admin/MetricsPage.tsx +++ b/commafeed-client/src/pages/admin/MetricsPage.tsx @@ -23,6 +23,8 @@ const shownGauges: Record = { "com.commafeed.backend.HttpGetter.pool.size": "HttpGetter current pool size", "com.commafeed.backend.HttpGetter.pool.leased": "HttpGetter active connections", "com.commafeed.backend.HttpGetter.pool.pending": "HttpGetter waiting for a connection", + "com.commafeed.backend.HttpGetter.cache.size": "HttpGetter cached entries", + "com.commafeed.backend.HttpGetter.cache.memoryUsage": "HttpGetter cache memory usage", "com.commafeed.frontend.ws.WebSocketSessions.users": "WebSocket users", "com.commafeed.frontend.ws.WebSocketSessions.sessions": "WebSocket sessions", } diff --git a/commafeed-server/doc/commafeed.adoc b/commafeed-server/doc/commafeed.adoc index 3d334c4d..f576c599 100644 --- a/commafeed-server/doc/commafeed.adoc +++ b/commafeed-server/doc/commafeed.adoc @@ -250,6 +250,62 @@ endif::add-copy-button-to-env-var[] |MemorySize link:#memory-size-note-anchor-{summaryTableId}[icon:question-circle[title=More information about the MemorySize format]] |`5M` +h|[[commafeed-server_section_commafeed-http-client-cache]] [.section-name.section-level1]##HTTP client cache configuration## +h|Type +h|Default + +a| [[commafeed-server_commafeed-http-client-cache-enabled]] [.property-path]##`commafeed.http-client.cache.enabled`## + +[.description] +-- +Whether to enable the cache. This cache is used to avoid spamming feeds too often (e.g. when subscribing to a feed for the first time or when clicking "fetch all my feeds now"). + + +ifdef::add-copy-button-to-env-var[] +Environment variable: env_var_with_copy_button:+++COMMAFEED_HTTP_CLIENT_CACHE_ENABLED+++[] +endif::add-copy-button-to-env-var[] +ifndef::add-copy-button-to-env-var[] +Environment variable: `+++COMMAFEED_HTTP_CLIENT_CACHE_ENABLED+++` +endif::add-copy-button-to-env-var[] +-- +|boolean +|`true` + +a| [[commafeed-server_commafeed-http-client-cache-maximum-memory-size]] [.property-path]##`commafeed.http-client.cache.maximum-memory-size`## + +[.description] +-- +Maximum amount of memory the cache can use. + + +ifdef::add-copy-button-to-env-var[] +Environment variable: env_var_with_copy_button:+++COMMAFEED_HTTP_CLIENT_CACHE_MAXIMUM_MEMORY_SIZE+++[] +endif::add-copy-button-to-env-var[] +ifndef::add-copy-button-to-env-var[] +Environment variable: `+++COMMAFEED_HTTP_CLIENT_CACHE_MAXIMUM_MEMORY_SIZE+++` +endif::add-copy-button-to-env-var[] +-- +|MemorySize link:#memory-size-note-anchor-{summaryTableId}[icon:question-circle[title=More information about the MemorySize format]] +|`10M` + +a| [[commafeed-server_commafeed-http-client-cache-expiration]] [.property-path]##`commafeed.http-client.cache.expiration`## + +[.description] +-- +Duration after which an entry is removed from the cache. + + +ifdef::add-copy-button-to-env-var[] +Environment variable: env_var_with_copy_button:+++COMMAFEED_HTTP_CLIENT_CACHE_EXPIRATION+++[] +endif::add-copy-button-to-env-var[] +ifndef::add-copy-button-to-env-var[] +Environment variable: `+++COMMAFEED_HTTP_CLIENT_CACHE_EXPIRATION+++` +endif::add-copy-button-to-env-var[] +-- +|link:https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/time/Duration.html[Duration] link:#duration-note-anchor-{summaryTableId}[icon:question-circle[title=More information about the Duration format]] +|`1M` + + h|[[commafeed-server_section_commafeed-feed-refresh]] [.section-name.section-level0]##Feed refresh engine settings## h|Type diff --git a/commafeed-server/src/main/java/com/commafeed/CommaFeedConfiguration.java b/commafeed-server/src/main/java/com/commafeed/CommaFeedConfiguration.java index c4f42ae1..b0fed899 100644 --- a/commafeed-server/src/main/java/com/commafeed/CommaFeedConfiguration.java +++ b/commafeed-server/src/main/java/com/commafeed/CommaFeedConfiguration.java @@ -137,6 +137,33 @@ public interface CommaFeedConfiguration { */ @WithDefault("5M") MemorySize maxResponseSize(); + + /** + * HTTP client cache configuration + */ + @ConfigDocSection + HttpClientCache cache(); + } + + interface HttpClientCache { + /** + * Whether to enable the cache. This cache is used to avoid spamming feeds too often (e.g. when subscribing to a feed for the first + * time or when clicking "fetch all my feeds now"). + */ + @WithDefault("true") + boolean enabled(); + + /** + * Maximum amount of memory the cache can use. + */ + @WithDefault("10M") + MemorySize maximumMemorySize(); + + /** + * Duration after which an entry is removed from the cache. + */ + @WithDefault("1m") + Duration expiration(); } interface FeedRefresh { diff --git a/commafeed-server/src/main/java/com/commafeed/backend/HttpGetter.java b/commafeed-server/src/main/java/com/commafeed/backend/HttpGetter.java index 61ac226e..d01a7323 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/HttpGetter.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/HttpGetter.java @@ -7,6 +7,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.concurrent.ExecutionException; import org.apache.commons.lang3.StringUtils; import org.apache.hc.client5.http.config.ConnectionConfig; @@ -31,14 +32,19 @@ import org.apache.hc.core5.util.Timeout; import com.codahale.metrics.MetricRegistry; import com.commafeed.CommaFeedConfiguration; +import com.commafeed.CommaFeedConfiguration.HttpClientCache; import com.commafeed.CommaFeedVersion; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.common.collect.Iterables; import com.google.common.io.ByteStreams; import com.google.common.net.HttpHeaders; import jakarta.inject.Singleton; +import lombok.Builder; +import lombok.EqualsAndHashCode; import lombok.Getter; -import lombok.RequiredArgsConstructor; +import lombok.Value; import lombok.extern.slf4j.Slf4j; import nl.altindag.ssl.SSLFactory; import nl.altindag.ssl.apache5.util.Apache5SslUtils; @@ -52,6 +58,7 @@ public class HttpGetter { private final CommaFeedConfiguration config; private final CloseableHttpClient client; + private final Cache cache; public HttpGetter(CommaFeedConfiguration config, CommaFeedVersion version, MetricRegistry metrics) { this.config = config; @@ -62,42 +69,66 @@ public class HttpGetter { .orElseGet(() -> String.format("CommaFeed/%s (https://github.com/Athou/commafeed)", version.getVersion())); this.client = newClient(connectionManager, userAgent, config.httpClient().idleConnectionsEvictionInterval()); + this.cache = newCache(config); metrics.registerGauge(MetricRegistry.name(getClass(), "pool", "max"), () -> connectionManager.getTotalStats().getMax()); metrics.registerGauge(MetricRegistry.name(getClass(), "pool", "size"), () -> connectionManager.getTotalStats().getAvailable() + connectionManager.getTotalStats().getLeased()); metrics.registerGauge(MetricRegistry.name(getClass(), "pool", "leased"), () -> connectionManager.getTotalStats().getLeased()); metrics.registerGauge(MetricRegistry.name(getClass(), "pool", "pending"), () -> connectionManager.getTotalStats().getPending()); + metrics.registerGauge(MetricRegistry.name(getClass(), "cache", "size"), () -> cache == null ? 0 : cache.size()); + metrics.registerGauge(MetricRegistry.name(getClass(), "cache", "memoryUsage"), + () -> cache == null ? 0 : cache.asMap().values().stream().mapToInt(e -> e.content != null ? e.content.length : 0).sum()); } - public HttpResult getBinary(String url) throws IOException, NotModifiedException { - return getBinary(url, null, null); + public HttpResult get(String url) throws IOException, NotModifiedException { + return get(HttpRequest.builder(url).build()); } - /** - * @param url - * the url to retrive - * @param lastModified - * header we got last time we queried that url, or null - * @param eTag - * header we got last time we queried that url, or null - * @throws NotModifiedException - * if the url hasn't changed since we asked for it last time - */ - public HttpResult getBinary(String url, String lastModified, String eTag) throws IOException, NotModifiedException { - log.debug("fetching {}", url); + public HttpResult get(HttpRequest request) throws IOException, NotModifiedException { + final HttpResponse response; + if (cache == null) { + response = invoke(request); + } else { + try { + response = cache.get(request, () -> invoke(request)); + } catch (ExecutionException e) { + if (e.getCause() instanceof IOException ioe) { + throw ioe; + } else { + throw new RuntimeException(e); + } + } + } - ClassicHttpRequest request = ClassicRequestBuilder.get(url).build(); - if (lastModified != null) { - request.addHeader(HttpHeaders.IF_MODIFIED_SINCE, lastModified); + int code = response.getCode(); + if (code == HttpStatus.SC_NOT_MODIFIED) { + throw new NotModifiedException("'304 - not modified' http code received"); + } else if (code >= 300) { + throw new HttpResponseException(code, "Server returned HTTP error code " + code); } - if (eTag != null) { - request.addHeader(HttpHeaders.IF_NONE_MATCH, eTag); + + String lastModifiedHeader = response.getLastModifiedHeader(); + if (lastModifiedHeader != null && lastModifiedHeader.equals(request.getLastModified())) { + throw new NotModifiedException("lastModifiedHeader is the same"); } + String eTagHeader = response.getETagHeader(); + if (eTagHeader != null && eTagHeader.equals(request.getETag())) { + throw new NotModifiedException("eTagHeader is the same"); + } + + return new HttpResult(response.getContent(), response.getContentType(), lastModifiedHeader, eTagHeader, + response.getUrlAfterRedirect()); + } + + private HttpResponse invoke(HttpRequest request) throws IOException { + log.debug("fetching {}", request.getUrl()); + HttpClientContext context = HttpClientContext.create(); context.setRequestConfig(RequestConfig.custom().setResponseTimeout(Timeout.of(config.httpClient().responseTimeout())).build()); - HttpResponse response = client.execute(request, context, resp -> { + + return client.execute(request.toClassicHttpRequest(), context, resp -> { byte[] content = resp.getEntity() == null ? null : toByteArray(resp.getEntity(), config.httpClient().maxResponseSize().asLongValue()); int code = resp.getCode(); @@ -115,30 +146,10 @@ public class HttpGetter { .map(RedirectLocations::getAll) .map(l -> Iterables.getLast(l, null)) .map(URI::toString) - .orElse(url); + .orElse(request.getUrl()); return new HttpResponse(code, lastModifiedHeader, eTagHeader, content, contentType, urlAfterRedirect); }); - - int code = response.getCode(); - if (code == HttpStatus.SC_NOT_MODIFIED) { - throw new NotModifiedException("'304 - not modified' http code received"); - } else if (code >= 300) { - throw new HttpResponseException(code, "Server returned HTTP error code " + code); - } - - String lastModifiedHeader = response.getLastModifiedHeader(); - if (lastModifiedHeader != null && lastModifiedHeader.equals(lastModified)) { - throw new NotModifiedException("lastModifiedHeader is the same"); - } - - String eTagHeader = response.getETagHeader(); - if (eTagHeader != null && eTagHeader.equals(eTag)) { - throw new NotModifiedException("eTagHeader is the same"); - } - - return new HttpResult(response.getContent(), response.getContentType(), lastModifiedHeader, eTagHeader, - response.getUrlAfterRedirect()); } private static byte[] toByteArray(HttpEntity entity, long maxBytes) throws IOException { @@ -197,6 +208,19 @@ public class HttpGetter { .build(); } + private static Cache newCache(CommaFeedConfiguration config) { + HttpClientCache cacheConfig = config.httpClient().cache(); + if (!cacheConfig.enabled()) { + return null; + } + + return CacheBuilder.newBuilder() + .weigher((HttpRequest key, HttpResponse value) -> value.getContent() != null ? value.getContent().length : 0) + .maximumWeight(cacheConfig.maximumMemorySize().asLongValue()) + .expireAfterWrite(cacheConfig.expiration()) + .build(); + } + @Getter public static class NotModifiedException extends Exception { private static final long serialVersionUID = 1L; @@ -232,28 +256,49 @@ public class HttpGetter { super(message); this.code = code; } - } + @Builder(builderMethodName = "") + @EqualsAndHashCode @Getter - @RequiredArgsConstructor + public static class HttpRequest { + private String url; + private String lastModified; + private String eTag; + + public static HttpRequestBuilder builder(String url) { + return new HttpRequestBuilder().url(url); + } + + public ClassicHttpRequest toClassicHttpRequest() { + ClassicHttpRequest req = ClassicRequestBuilder.get(url).build(); + if (lastModified != null) { + req.addHeader(HttpHeaders.IF_MODIFIED_SINCE, lastModified); + } + if (eTag != null) { + req.addHeader(HttpHeaders.IF_NONE_MATCH, eTag); + } + return req; + } + } + + @Value private static class HttpResponse { - private final int code; - private final String lastModifiedHeader; - private final String eTagHeader; - private final byte[] content; - private final String contentType; - private final String urlAfterRedirect; + int code; + String lastModifiedHeader; + String eTagHeader; + byte[] content; + String contentType; + String urlAfterRedirect; } - @Getter - @RequiredArgsConstructor + @Value public static class HttpResult { - private final byte[] content; - private final String contentType; - private final String lastModifiedSince; - private final String eTag; - private final String urlAfterRedirect; + byte[] content; + String contentType; + String lastModifiedSince; + String eTag; + String urlAfterRedirect; } } diff --git a/commafeed-server/src/main/java/com/commafeed/backend/favicon/DefaultFaviconFetcher.java b/commafeed-server/src/main/java/com/commafeed/backend/favicon/DefaultFaviconFetcher.java index 1dc94a1c..684b3e78 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/favicon/DefaultFaviconFetcher.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/favicon/DefaultFaviconFetcher.java @@ -69,7 +69,7 @@ public class DefaultFaviconFetcher extends AbstractFaviconFetcher { try { url = FeedUtils.removeTrailingSlash(url) + "/favicon.ico"; log.debug("getting root icon at {}", url); - HttpResult result = getter.getBinary(url); + HttpResult result = getter.get(url); bytes = result.getContent(); contentType = result.getContentType(); } catch (Exception e) { @@ -87,7 +87,7 @@ public class DefaultFaviconFetcher extends AbstractFaviconFetcher { Document doc; try { - HttpResult result = getter.getBinary(url); + HttpResult result = getter.get(url); doc = Jsoup.parse(new String(result.getContent()), url); } catch (Exception e) { log.debug("Failed to retrieve page to find icon"); @@ -113,7 +113,7 @@ public class DefaultFaviconFetcher extends AbstractFaviconFetcher { byte[] bytes; String contentType; try { - HttpResult result = getter.getBinary(href); + HttpResult result = getter.get(href); bytes = result.getContent(); contentType = result.getContentType(); } catch (Exception e) { diff --git a/commafeed-server/src/main/java/com/commafeed/backend/favicon/FacebookFaviconFetcher.java b/commafeed-server/src/main/java/com/commafeed/backend/favicon/FacebookFaviconFetcher.java index fad9d1ef..0bf84c13 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/favicon/FacebookFaviconFetcher.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/favicon/FacebookFaviconFetcher.java @@ -43,7 +43,7 @@ public class FacebookFaviconFetcher extends AbstractFaviconFetcher { try { log.debug("Getting Facebook user's icon, {}", url); - HttpResult iconResult = getter.getBinary(iconUrl); + HttpResult iconResult = getter.get(iconUrl); bytes = iconResult.getContent(); contentType = iconResult.getContentType(); } catch (Exception e) { diff --git a/commafeed-server/src/main/java/com/commafeed/backend/favicon/YoutubeFaviconFetcher.java b/commafeed-server/src/main/java/com/commafeed/backend/favicon/YoutubeFaviconFetcher.java index e389856d..86198d04 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/favicon/YoutubeFaviconFetcher.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/favicon/YoutubeFaviconFetcher.java @@ -78,7 +78,7 @@ public class YoutubeFaviconFetcher extends AbstractFaviconFetcher { return null; } - HttpResult iconResult = getter.getBinary(thumbnailUrl.asText()); + HttpResult iconResult = getter.get(thumbnailUrl.asText()); bytes = iconResult.getContent(); contentType = iconResult.getContentType(); } catch (Exception e) { @@ -97,7 +97,7 @@ public class YoutubeFaviconFetcher extends AbstractFaviconFetcher { .queryParam("key", googleAuthKey) .queryParam("forUsername", userId) .build(); - return getter.getBinary(uri.toString()).getContent(); + return getter.get(uri.toString()).getContent(); } private byte[] fetchForChannel(String googleAuthKey, String channelId) throws IOException, NotModifiedException { @@ -106,7 +106,7 @@ public class YoutubeFaviconFetcher extends AbstractFaviconFetcher { .queryParam("key", googleAuthKey) .queryParam("id", channelId) .build(); - return getter.getBinary(uri.toString()).getContent(); + return getter.get(uri.toString()).getContent(); } private byte[] fetchForPlaylist(String googleAuthKey, String playlistId) throws IOException, NotModifiedException { @@ -115,7 +115,7 @@ public class YoutubeFaviconFetcher extends AbstractFaviconFetcher { .queryParam("key", googleAuthKey) .queryParam("id", playlistId) .build(); - byte[] playlistBytes = getter.getBinary(uri.toString()).getContent(); + byte[] playlistBytes = getter.get(uri.toString()).getContent(); JsonNode channelId = objectMapper.readTree(playlistBytes).at(PLAYLIST_CHANNEL_ID); if (channelId.isMissingNode()) { diff --git a/commafeed-server/src/main/java/com/commafeed/backend/feed/FeedFetcher.java b/commafeed-server/src/main/java/com/commafeed/backend/feed/FeedFetcher.java index b40e73d2..6a8b11f9 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/feed/FeedFetcher.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/feed/FeedFetcher.java @@ -9,6 +9,7 @@ import org.apache.commons.lang3.StringUtils; import com.commafeed.backend.Digests; import com.commafeed.backend.HttpGetter; +import com.commafeed.backend.HttpGetter.HttpRequest; import com.commafeed.backend.HttpGetter.HttpResult; import com.commafeed.backend.HttpGetter.NotModifiedException; import com.commafeed.backend.feed.parser.FeedParser; @@ -41,7 +42,7 @@ public class FeedFetcher { Instant lastPublishedDate, String lastContentHash) throws FeedException, IOException, NotModifiedException { log.debug("Fetching feed {}", feedUrl); - HttpResult result = getter.getBinary(feedUrl, lastModified, eTag); + HttpResult result = getter.get(HttpRequest.builder(feedUrl).lastModified(lastModified).eTag(eTag).build()); byte[] content = result.getContent(); FeedParserResult parserResult; @@ -53,7 +54,7 @@ public class FeedFetcher { if (org.apache.commons.lang3.StringUtils.isNotBlank(extractedUrl)) { feedUrl = extractedUrl; - result = getter.getBinary(extractedUrl, lastModified, eTag); + result = getter.get(HttpRequest.builder(extractedUrl).lastModified(lastModified).eTag(eTag).build()); content = result.getContent(); parserResult = parser.parse(result.getUrlAfterRedirect(), content); } else { diff --git a/commafeed-server/src/main/java/com/commafeed/backend/service/FeedSubscriptionService.java b/commafeed-server/src/main/java/com/commafeed/backend/service/FeedSubscriptionService.java index a3d69ba5..d554500c 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/service/FeedSubscriptionService.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/service/FeedSubscriptionService.java @@ -40,9 +40,14 @@ public class FeedSubscriptionService { this.feedRefreshEngine = feedRefreshEngine; this.config = config; - // automatically refresh feeds after they are subscribed to - // we need to use this hook because the feed needs to have been persisted because the queue processing is asynchronous - feedSubscriptionDAO.onPostCommitInsert(sub -> feedRefreshEngine.refreshImmediately(sub.getFeed())); + // automatically refresh new feeds after they are subscribed to + // we need to use this hook because the feed needs to have been persisted before being processed by the feed engine + feedSubscriptionDAO.onPostCommitInsert(sub -> { + Feed feed = sub.getFeed(); + if (feed.getDisabledUntil() == null || feed.getDisabledUntil().isBefore(Instant.now())) { + feedRefreshEngine.refreshImmediately(feed); + } + }); } public long subscribe(User user, String url, String title) { diff --git a/commafeed-server/src/main/java/com/commafeed/frontend/resource/ServerREST.java b/commafeed-server/src/main/java/com/commafeed/frontend/resource/ServerREST.java index 272b79f1..3145ba87 100644 --- a/commafeed-server/src/main/java/com/commafeed/frontend/resource/ServerREST.java +++ b/commafeed-server/src/main/java/com/commafeed/frontend/resource/ServerREST.java @@ -77,7 +77,7 @@ public class ServerREST { url = FeedUtils.imageProxyDecoder(url); try { - HttpResult result = httpGetter.getBinary(url); + HttpResult result = httpGetter.get(url); return Response.ok(result.getContent()).build(); } catch (Exception e) { return Response.status(Status.SERVICE_UNAVAILABLE).entity(e.getMessage()).build(); diff --git a/commafeed-server/src/main/resources/application.properties b/commafeed-server/src/main/resources/application.properties index 4d2bb580..3c424f2e 100644 --- a/commafeed-server/src/main/resources/application.properties +++ b/commafeed-server/src/main/resources/application.properties @@ -39,6 +39,7 @@ quarkus.native.add-all-charsets=true %dev.quarkus.http.auth.session.encryption-key=123456789012345678901234567890 %dev.quarkus.log.category."com.commafeed".level=DEBUG # %dev.quarkus.hibernate-orm.log.sql=true +%dev.commafeed.users.create-demo-account=true # test profile overrides @@ -47,6 +48,7 @@ quarkus.native.add-all-charsets=true %test.commafeed.users.create-demo-account=true %test.commafeed.users.allow-registrations=true %test.commafeed.password-recovery-enabled=true +%test.commafeed.http-client.cache.enabled=false # prod profile overrides diff --git a/commafeed-server/src/test/java/com/commafeed/backend/HttpGetterTest.java b/commafeed-server/src/test/java/com/commafeed/backend/HttpGetterTest.java index dc4c87c2..9ea9b639 100644 --- a/commafeed-server/src/test/java/com/commafeed/backend/HttpGetterTest.java +++ b/commafeed-server/src/test/java/com/commafeed/backend/HttpGetterTest.java @@ -68,6 +68,9 @@ class HttpGetterTest { Mockito.when(config.httpClient().responseTimeout()).thenReturn(Duration.ofSeconds(30)); Mockito.when(config.httpClient().connectionTimeToLive()).thenReturn(Duration.ofSeconds(30)); Mockito.when(config.httpClient().maxResponseSize()).thenReturn(new MemorySize(new BigInteger("10000"))); + Mockito.when(config.httpClient().cache().enabled()).thenReturn(true); + Mockito.when(config.httpClient().cache().maximumMemorySize()).thenReturn(new MemorySize(new BigInteger("100000"))); + Mockito.when(config.httpClient().cache().expiration()).thenReturn(Duration.ofMinutes(1)); Mockito.when(config.feedRefresh().httpThreads()).thenReturn(3); this.getter = new HttpGetter(config, Mockito.mock(CommaFeedVersion.class), Mockito.mock(MetricRegistry.class)); @@ -79,7 +82,7 @@ class HttpGetterTest { void errorCodes(int code) { this.mockServerClient.when(HttpRequest.request().withMethod("GET")).respond(HttpResponse.response().withStatusCode(code)); - HttpResponseException e = Assertions.assertThrows(HttpResponseException.class, () -> getter.getBinary(this.feedUrl)); + HttpResponseException e = Assertions.assertThrows(HttpResponseException.class, () -> getter.get(this.feedUrl)); Assertions.assertEquals(code, e.getCode()); } @@ -92,7 +95,7 @@ class HttpGetterTest { .withHeader(HttpHeaders.LAST_MODIFIED, "123456") .withHeader(HttpHeaders.ETAG, "78910")); - HttpResult result = getter.getBinary(this.feedUrl); + HttpResult result = getter.get(this.feedUrl); Assertions.assertArrayEquals(feedContent, result.getContent()); Assertions.assertEquals(MediaType.APPLICATION_ATOM_XML.toString(), result.getContentType()); Assertions.assertEquals("123456", result.getLastModifiedSince()); @@ -121,7 +124,7 @@ class HttpGetterTest { this.mockServerClient.when(HttpRequest.request().withMethod("GET").withPath("/redirected-2")) .respond(HttpResponse.response().withBody(feedContent).withContentType(MediaType.APPLICATION_ATOM_XML)); - HttpResult result = getter.getBinary(this.feedUrl); + HttpResult result = getter.get(this.feedUrl); Assertions.assertEquals("http://localhost:" + this.mockServerClient.getPort() + "/redirected-2", result.getUrlAfterRedirect()); } @@ -133,7 +136,7 @@ class HttpGetterTest { this.mockServerClient.when(HttpRequest.request().withMethod("GET")) .respond(HttpResponse.response().withDelay(Delay.milliseconds(1000))); - Assertions.assertThrows(SocketTimeoutException.class, () -> getter.getBinary(this.feedUrl)); + Assertions.assertThrows(SocketTimeoutException.class, () -> getter.get(this.feedUrl)); } @Test @@ -142,7 +145,7 @@ class HttpGetterTest { this.getter = new HttpGetter(config, Mockito.mock(CommaFeedVersion.class), Mockito.mock(MetricRegistry.class)); // try to connect to a non-routable address // https://stackoverflow.com/a/904609 - Assertions.assertThrows(ConnectTimeoutException.class, () -> getter.getBinary("http://10.255.255.1")); + Assertions.assertThrows(ConnectTimeoutException.class, () -> getter.get("http://10.255.255.1")); } @Test @@ -150,7 +153,7 @@ class HttpGetterTest { this.mockServerClient.when(HttpRequest.request().withMethod("GET").withHeader(HttpHeaders.USER_AGENT, "http-getter-test")) .respond(HttpResponse.response().withBody("ok")); - HttpResult result = getter.getBinary(this.feedUrl); + HttpResult result = getter.get(this.feedUrl); Assertions.assertEquals("ok", new String(result.getContent())); } @@ -159,7 +162,8 @@ class HttpGetterTest { this.mockServerClient.when(HttpRequest.request().withMethod("GET").withHeader(HttpHeaders.IF_MODIFIED_SINCE, "123456")) .respond(HttpResponse.response().withStatusCode(HttpStatus.SC_NOT_MODIFIED)); - Assertions.assertThrows(NotModifiedException.class, () -> getter.getBinary(this.feedUrl, "123456", null)); + Assertions.assertThrows(NotModifiedException.class, + () -> getter.get(HttpGetter.HttpRequest.builder(this.feedUrl).lastModified("123456").build())); } @Test @@ -167,7 +171,8 @@ class HttpGetterTest { this.mockServerClient.when(HttpRequest.request().withMethod("GET").withHeader(HttpHeaders.IF_NONE_MATCH, "78910")) .respond(HttpResponse.response().withStatusCode(HttpStatus.SC_NOT_MODIFIED)); - Assertions.assertThrows(NotModifiedException.class, () -> getter.getBinary(this.feedUrl, null, "78910")); + Assertions.assertThrows(NotModifiedException.class, + () -> getter.get(HttpGetter.HttpRequest.builder(this.feedUrl).eTag("78910").build())); } @Test @@ -184,18 +189,32 @@ class HttpGetterTest { return HttpResponse.response().withBody("ok").withHeader(HttpHeaders.SET_COOKIE, "foo=bar"); }); - Assertions.assertDoesNotThrow(() -> getter.getBinary(this.feedUrl)); - Assertions.assertDoesNotThrow(() -> getter.getBinary(this.feedUrl)); + Assertions.assertDoesNotThrow(() -> getter.get(this.feedUrl)); + Assertions.assertDoesNotThrow(() -> getter.get(this.feedUrl + "?foo=bar")); Assertions.assertEquals(2, calls.get()); } + @Test + void cacheSubsequentCalls() throws IOException, NotModifiedException { + AtomicInteger calls = new AtomicInteger(); + + this.mockServerClient.when(HttpRequest.request().withMethod("GET")).respond(req -> { + calls.incrementAndGet(); + return HttpResponse.response().withBody("ok"); + }); + + HttpResult result = getter.get(this.feedUrl); + Assertions.assertEquals(result, getter.get(this.feedUrl)); + Assertions.assertEquals(1, calls.get()); + } + @Test void largeFeedWithContentLengthHeader() { byte[] bytes = new byte[100000]; Arrays.fill(bytes, (byte) 1); this.mockServerClient.when(HttpRequest.request().withMethod("GET")).respond(HttpResponse.response().withBody(bytes)); - IOException e = Assertions.assertThrows(IOException.class, () -> getter.getBinary(this.feedUrl)); + IOException e = Assertions.assertThrows(IOException.class, () -> getter.get(this.feedUrl)); Assertions.assertEquals("Response size (100000 bytes) exceeds the maximum allowed size (10000 bytes)", e.getMessage()); } @@ -208,7 +227,7 @@ class HttpGetterTest { .withBody(bytes) .withConnectionOptions(ConnectionOptions.connectionOptions().withSuppressContentLengthHeader(true))); - IOException e = Assertions.assertThrows(IOException.class, () -> getter.getBinary(this.feedUrl)); + IOException e = Assertions.assertThrows(IOException.class, () -> getter.get(this.feedUrl)); Assertions.assertEquals("Response size exceeds the maximum allowed size (10000 bytes)", e.getMessage()); } @@ -216,7 +235,7 @@ class HttpGetterTest { void ignoreInvalidSsl() throws Exception { this.mockServerClient.when(HttpRequest.request().withMethod("GET")).respond(HttpResponse.response().withBody("ok")); - HttpResult result = getter.getBinary("https://localhost:" + this.mockServerClient.getPort()); + HttpResult result = getter.get("https://localhost:" + this.mockServerClient.getPort()); Assertions.assertEquals("ok", new String(result.getContent())); } @@ -251,7 +270,7 @@ class HttpGetterTest { return HttpResponse.response().withBody(output.toByteArray()).withHeader(HttpHeaders.CONTENT_ENCODING, encoding); }); - HttpResult result = getter.getBinary(HttpGetterTest.this.feedUrl); + HttpResult result = getter.get(HttpGetterTest.this.feedUrl); Assertions.assertEquals(body, new String(result.getContent())); } diff --git a/commafeed-server/src/test/java/com/commafeed/backend/feed/FeedFetcherTest.java b/commafeed-server/src/test/java/com/commafeed/backend/feed/FeedFetcherTest.java index 69f4f395..03ad1142 100644 --- a/commafeed-server/src/test/java/com/commafeed/backend/feed/FeedFetcherTest.java +++ b/commafeed-server/src/test/java/com/commafeed/backend/feed/FeedFetcherTest.java @@ -45,7 +45,7 @@ class FeedFetcherTest { byte[] content = "content".getBytes(); String lastContentHash = Digests.sha1Hex(content); - Mockito.when(getter.getBinary(url, lastModified, etag)) + Mockito.when(getter.get(HttpGetter.HttpRequest.builder(url).lastModified(lastModified).eTag(etag).build())) .thenReturn(new HttpResult(content, "content-type", "last-modified-2", "etag-2", null)); NotModifiedException e = Assertions.assertThrows(NotModifiedException.class,