You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2021/11/26 12:58:39 UTC
[GitHub] [httpcomponents-client] ok2c commented on a change in pull request #330: HTTPCLIENT-2189 - Use Time APIs for date for the remain classes in the HttpClient API.
ok2c commented on a change in pull request #330:
URL: https://github.com/apache/httpcomponents-client/pull/330#discussion_r757473176
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/cookie/BasicCookieStore.java
##########
@@ -155,6 +156,34 @@ public boolean clearExpired(final Date date) {
}
}
+ /**
+ * Removes all of {@link Cookie cookies} in this HTTP state
+ * that have expired by the specified {@link java.util.Date date}.
+ *
+ * @return true if any cookies were purged.
+ *
+ * @see Cookie#isExpired(Instant)
+ */
Review comment:
@arturobernalg Please add `@since` tag.
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/cookie/Cookie.java
##########
@@ -119,14 +133,31 @@
* @param date Current time
*
* @return {@code true} if the cookie has expired.
+ * @deprecated Use {{@link #isExpired(Instant)}}
*/
+ @Deprecated
boolean isExpired(final Date date);
+ /**
+ * Returns true if this cookie has expired.
+ * @param date Current time
+ *
+ * @return {@code true} if the cookie has expired.
+ */
+ default boolean isExpired(final Instant date) { return false; }
+
/**
* Returns creation time of the cookie.
+ * @deprecated Use {@link #getCreationInstant()}
*/
+ @Deprecated
Date getCreationDate();
+ /**
+ * Returns creation time of the cookie.
+ */
+ default Instant getCreationInstant() { return null; }
Review comment:
@arturobernalg Use deprecated method here and suppress deprecation warnings.
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/cookie/SetCookie.java
##########
@@ -48,10 +49,25 @@
* @param expiryDate the {@link Date} after which this cookie is no longer valid.
*
* @see Cookie#getExpiryDate
- *
+ * @deprecated Use {{@link #setExpiryDate(Instant)}}
*/
+ @Deprecated
void setExpiryDate (Date expiryDate);
+ /**
+ * Sets expiration date.
+ * <p><strong>Note:</strong> the object returned by this method is considered
+ * immutable. Changing it (e.g. using setTime()) could result in undefined
+ * behaviour. Do so at your peril.</p>
+ *
+ * @param expiryDate the {@link Instant} after which this cookie is no longer valid.
+ *
+ * @see Cookie#getExpiryInstant()
+ *
+ */
+ default void setExpiryDate (Instant expiryDate) {}
Review comment:
@arturobernalg Use deprecated method here and suppress deprecation warnings.
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/cookie/CookieStore.java
##########
@@ -58,9 +59,19 @@
* the specified {@link java.util.Date}.
*
* @return true if any cookies were purged.
+ * @deprecated Use {@link #clearExpired(Instant)}
*/
+ @Deprecated
boolean clearExpired(Date date);
+ /**
+ * Removes all of {@link Cookie}s in this store that have expired by
+ * the specified {@link Instant}.
+ *
+ * @return true if any cookies were purged.
+ */
+ default boolean clearExpired(Instant date) { return false; }
Review comment:
@arturobernalg Use deprecated method here and suppress deprecation warnings.
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/cookie/Cookie.java
##########
@@ -77,9 +78,22 @@
* considered immutable. Changing it (e.g. using setTime()) could result
* in undefined behaviour. Do so at your peril. </p>
* @return Expiration {@link Date}, or {@code null}.
+ * @deprecated Use {{@link #getExpiryInstant()}}
*/
+ @Deprecated
Date getExpiryDate();
+ /**
+ * Returns the expiration {@link Instant} of the cookie, or {@code null}
+ * if none exists.
+ * <p><strong>Note:</strong> the object returned by this method is
+ * considered immutable. Changing it (e.g. using setTime()) could result
+ * in undefined behaviour. Do so at your peril. </p>
+ * @return Expiration {@link Instant}, or {@code null}.
+ * @since 5.2
+ */
+ default Instant getExpiryInstant() { return null; }
Review comment:
@arturobernalg Please convert `Date` returned by deprecated `getExpiryDate()` to `Instance` here. Suppress deprecation warnings.
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/cookie/Cookie.java
##########
@@ -119,14 +133,31 @@
* @param date Current time
*
* @return {@code true} if the cookie has expired.
+ * @deprecated Use {{@link #isExpired(Instant)}}
*/
+ @Deprecated
boolean isExpired(final Date date);
+ /**
+ * Returns true if this cookie has expired.
+ * @param date Current time
+ *
+ * @return {@code true} if the cookie has expired.
+ */
+ default boolean isExpired(final Instant date) { return false; }
Review comment:
@arturobernalg Use deprecated method here and suppress deprecation warnings.
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/LaxMaxAgeHandler.java
##########
@@ -68,8 +68,8 @@ public void parse(final SetCookie cookie, final String value) throws MalformedCo
} catch (final NumberFormatException e) {
return;
}
- final Date expiryDate = age >= 0 ? new Date(System.currentTimeMillis() + age * 1000L) :
- new Date(Long.MIN_VALUE);
+ final Instant expiryDate = age >= 0 ? Instant.now().plusSeconds(age * 1000L) :
Review comment:
@arturobernalg This looks wrong. Should not it be `Instant.now().plusSeconds(age)` or `Instant.now().plusMillis(age * 1000L)`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org