Return Optional when parsing HTTP dates

methanol/src/main/java/com/github/mizosoft/methanol/CacheControl.java

@@ -27,7 +27,7 @@
 import static com.github.mizosoft.methanol.internal.Utils.requireValidHeaderValue;
 import static com.github.mizosoft.methanol.internal.Utils.requireValidToken;
 import static com.github.mizosoft.methanol.internal.Validate.requireArgument;
-import static com.github.mizosoft.methanol.internal.cache.HttpDates.toDeltaSeconds;
+import static com.github.mizosoft.methanol.internal.cache.HttpDates.parseDeltaSeconds;
 import static java.lang.String.format;
 
 import com.github.mizosoft.methanol.internal.text.HeaderValueTokenizer;
@@ -561,26 +561,26 @@ private boolean setStandardDirective(String normalizedDirective, String argument
 
       switch (normalizedDirective) {
         case "max-age":
-          maxAge = toDeltaSeconds(argument);
+          maxAge = parseDeltaSeconds(argument);
           break;
         case "min-fresh":
-          minFresh = toDeltaSeconds(argument);
+          minFresh = parseDeltaSeconds(argument);
           break;
         case "s-maxage":
-          sMaxAge = toDeltaSeconds(argument);
+          sMaxAge = parseDeltaSeconds(argument);
           break;
         case "max-stale":
           if (argument.isEmpty()) {
             anyMaxStale = true;
           } else {
-            maxStale = toDeltaSeconds(argument);
+            maxStale = parseDeltaSeconds(argument);
           }
           break;
         case "stale-while-revalidate":
-          staleWhileRevalidate = toDeltaSeconds(argument);
+          staleWhileRevalidate = parseDeltaSeconds(argument);
           break;
         case "stale-if-error":
-          staleIfError = toDeltaSeconds(argument);
+          staleIfError = parseDeltaSeconds(argument);
           break;
         case "no-cache":
           noCache = true;

methanol/src/main/java/com/github/mizosoft/methanol/internal/cache/CacheInterceptor.java

@@ -408,7 +408,7 @@ private static Optional<Boolean> evaluateIfModifiedSince(
     return request
         .headers()
         .firstValue("If-Modified-Since")
-        .map(HttpDates::toHttpDate)
+        .flatMap(HttpDates::tryParseHttpDate)
         .map(value -> isModifiedSince(cacheResponse, value));
   }
 
@@ -480,7 +480,7 @@ private static boolean isModifiedSince(TrackedResponse<?> cacheResponse, LocalDa
         .headers()
         .firstValue("Last-Modified")
         .or(() -> cacheResponse.headers().firstValue("Date"))
-        .map(HttpDates::toHttpDate)
+        .flatMap(HttpDates::tryParseHttpDate)
         .orElseGet(() -> HttpDates.toUtcDateTime(cacheResponse.timeResponseReceived()))
         .isAfter(dateTime);
   }

methanol/src/main/java/com/github/mizosoft/methanol/internal/cache/CacheStrategy.java

@@ -22,12 +22,14 @@
 
 package com.github.mizosoft.methanol.internal.cache;
 
+import static com.github.mizosoft.methanol.internal.cache.HttpDates.formatHttpDate;
+import static com.github.mizosoft.methanol.internal.cache.HttpDates.toUtcDateTime;
+
 import com.github.mizosoft.methanol.CacheControl;
 import com.github.mizosoft.methanol.MutableRequest;
 import com.github.mizosoft.methanol.ResponseBuilder;
 import com.github.mizosoft.methanol.TrackedResponse;
 import com.github.mizosoft.methanol.internal.util.Compare;
-
 import java.net.http.HttpHeaders;
 import java.net.http.HttpRequest;
 import java.time.Duration;
@@ -36,9 +38,6 @@
 import java.time.ZoneOffset;
 import java.util.Optional;
 
-import static com.github.mizosoft.methanol.internal.cache.HttpDates.toHttpDateString;
-import static com.github.mizosoft.methanol.internal.cache.HttpDates.toUtcDateTime;
-
 /**
  * A strategy for determining whether a stored response is fresh enough for the cache to serve
  * without contacting the origin, based on the caching rules imposed by the server & client.
@@ -107,7 +106,7 @@ HttpRequest conditionalize(HttpRequest request) {
     etag.ifPresent(etag -> conditionalizedRequest.setHeader("If-None-Match", etag));
     lastModified.ifPresent(
         lastModified ->
-            conditionalizedRequest.setHeader("If-Modified-Since", toHttpDateString(lastModified)));
+            conditionalizedRequest.setHeader("If-Modified-Since", formatHttpDate(lastModified)));
     return conditionalizedRequest.toImmutableRequest();
   }
 
@@ -133,16 +132,17 @@ static final class Builder {
       requestCacheControl = CacheControl.parse(request.headers());
       responseCacheControl = CacheControl.parse(cacheResponse.headers());
       maxAge = requestCacheControl.maxAge().or(responseCacheControl::maxAge);
-      expires = cacheResponse.headers().firstValue("Expires").map(HttpDates::toHttpDate);
-      lastModified = cacheResponse.headers().firstValue("Last-Modified").map(HttpDates::toHttpDate);
+      expires = cacheResponse.headers().firstValue("Expires").flatMap(HttpDates::tryParseHttpDate);
+      lastModified =
+          cacheResponse.headers().firstValue("Last-Modified").flatMap(HttpDates::tryParseHttpDate);
 
       // As per rfc7231 Section 7.1.1.2, we must use the time the response was received as the value
       // of on absent Date field.
       date =
           cacheResponse
               .headers()
               .firstValue("Date")
-              .map(HttpDates::toHttpDate)
+              .flatMap(HttpDates::tryParseHttpDate)
               .orElseGet(() -> toUtcDateTime(timeResponseReceived));
     }
 
@@ -151,7 +151,7 @@ Duration computeAge(Instant now) {
       var age =
           cacheResponseHeaders
               .firstValue("Age")
-              .map(HttpDates::toDeltaSecondsOrNull)
+              .flatMap(HttpDates::tryParseDeltaSeconds)
               .orElse(Duration.ZERO);
       var apparentAge =
           Compare.max(
@@ -193,9 +193,7 @@ CacheStrategy build(Instant now) {
     }
   }
 
-  /**
-   * A rule for accepting stale responses upto a maximum staleness.
-   */
+  /** A rule for accepting stale responses upto a maximum staleness. */
   enum StalenessRule {
     MAX_STALE {
       @Override

methanol/src/main/java/com/github/mizosoft/methanol/internal/cache/HttpDates.java

@@ -35,7 +35,7 @@
 import java.time.temporal.TemporalAccessor;
 import java.time.temporal.TemporalQueries;
 import java.util.List;
-import org.checkerframework.checker.nullness.qual.Nullable;
+import java.util.Optional;
 
 /** Helpers for parsing/formatting HTTP dates. */
 public class HttpDates {
@@ -69,19 +69,19 @@ public class HttpDates {
 
   private HttpDates() {}
 
-  public static String toHttpDateString(LocalDateTime dateTime) {
+  public static String formatHttpDate(LocalDateTime dateTime) {
     return PREFERRED_FORMATTER.format(dateTime);
   }
 
   public static boolean isHttpDate(String value) {
-    return toHttpDate0(value, false) != null;
+    return tryParseHttpDate0(value, false).isPresent();
   }
 
-  static @Nullable LocalDateTime toHttpDate(String value) {
-    return toHttpDate0(value, true);
+  static Optional<LocalDateTime> tryParseHttpDate(String value) {
+    return tryParseHttpDate0(value, true);
   }
 
-  private static @Nullable LocalDateTime toHttpDate0(String value, boolean logFailure) {
+  private static Optional<LocalDateTime> tryParseHttpDate0(String value, boolean logFailure) {
     TemporalAccessor parsedTemporal = null;
     for (var formatter : FORMATTERS) {
       try {
@@ -97,9 +97,10 @@ public static boolean isHttpDate(String value) {
       try {
         var dateTime = LocalDateTime.from(parsedTemporal);
         var offset = parsedTemporal.query(TemporalQueries.offset());
-        return (offset == null || offset.equals(ZoneOffset.UTC))
-            ? dateTime
-            : toUtcDateTime(dateTime.toInstant(offset));
+        return Optional.of(
+            (offset == null || offset.equals(ZoneOffset.UTC))
+                ? dateTime
+                : toUtcDateTime(dateTime.toInstant(offset)));
       } catch (DateTimeException e) {
         malformedHttpDate = e;
       }
@@ -109,11 +110,11 @@ public static boolean isHttpDate(String value) {
       logger.log(
           Level.WARNING, () -> "Malformed or unrecognized HTTP date: " + value, malformedHttpDate);
     }
-    return null;
+    return Optional.empty();
   }
 
   // TODO tolerate -ve values by truncating to 0?
-  public static Duration toDeltaSeconds(String value) {
+  public static Duration parseDeltaSeconds(String value) {
     long secondsLong = Long.parseLong(value);
     requireArgument(secondsLong >= 0, "delta seconds can't be negative");
 
@@ -122,11 +123,11 @@ public static Duration toDeltaSeconds(String value) {
     return Duration.ofSeconds(secondsInt);
   }
 
-  static @Nullable Duration toDeltaSecondsOrNull(String value) {
+  static Optional<Duration> tryParseDeltaSeconds(String value) {
     try {
-      return toDeltaSeconds(value);
+      return Optional.of(parseDeltaSeconds(value));
     } catch (NumberFormatException ignored) {
-      return null;
+      return Optional.empty();
     }
   }
 

methanol/src/test/java/com/github/mizosoft/methanol/AbstractHttpCacheTest.java

@@ -24,7 +24,7 @@
 
 import static com.github.mizosoft.methanol.MutableRequest.GET;
 import static com.github.mizosoft.methanol.internal.Validate.requireState;
-import static com.github.mizosoft.methanol.internal.cache.HttpDates.toHttpDateString;
+import static com.github.mizosoft.methanol.internal.cache.HttpDates.formatHttpDate;
 import static java.net.HttpURLConnection.HTTP_UNAVAILABLE;
 import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -188,7 +188,7 @@ static LocalDateTime toUtcDateTime(Instant instant) {
   }
 
   static String instantToHttpDateString(Instant instant) {
-    return toHttpDateString(toUtcDateTime(instant));
+    return formatHttpDate(toUtcDateTime(instant));
   }
 
   static class ForwardingStore implements Store {