Fix todos in Cookie

8 files changed, 20 insertions, 168 deletions

diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java
index a43310aff51..d65d2ecb30d 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java
@@ -7,15 +7,11 @@ import org.eclipse.jetty.server.Response;
 import java.net.HttpCookie;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
-import java.util.StringTokenizer;
 import java.util.concurrent.TimeUnit;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 
@@ -24,22 +20,17 @@ import java.util.stream.StreamSupport;
  *
  * Note: RFC 2109 and RFC 2965 is no longer supported. All fields that are not part of RFC 6265 are deprecated.
  *
- * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a>
+ * @author Einar M R Rosenvinge
  * @author bjorncs
  */
 public class Cookie {
 
-    private final static Logger log = Logger.getLogger(Cookie.class.getName());
-
     private final Set<Integer> ports = new HashSet<>();
     private String name;
     private String value;
     private String domain;
     private String path;
-    private String comment;
-    private String commentUrl;
     private long maxAgeSeconds = Integer.MIN_VALUE;
-    private int version;
     private boolean secure;
     private boolean httpOnly;
     private boolean discard;
@@ -53,10 +44,7 @@ public class Cookie {
         value = cookie.value;
         domain = cookie.domain;
         path = cookie.path;
-        comment = cookie.comment;
-        commentUrl = cookie.commentUrl;
         maxAgeSeconds = cookie.maxAgeSeconds;
-        version = cookie.version;
         secure = cookie.secure;
         httpOnly = cookie.httpOnly;
         discard = cookie.discard;
@@ -103,38 +91,6 @@ public class Cookie {
         return this;
     }
 
-    @Deprecated
-    public String getComment() {
-        return comment;
-    }
-
-    @Deprecated
-    public Cookie setComment(String comment) {
-        this.comment = comment;
-        return this;
-    }
-
-    @Deprecated
-    public String getCommentURL() {
-        return getCommentUrl();
-    }
-
-    @Deprecated
-    public Cookie setCommentURL(String commentUrl) {
-        return setCommentUrl(commentUrl);
-    }
-
-    @Deprecated
-    public String getCommentUrl() {
-        return commentUrl;
-    }
-
-    @Deprecated
-    public Cookie setCommentUrl(String commentUrl) {
-        this.commentUrl = commentUrl;
-        return this;
-    }
-
     public int getMaxAge(TimeUnit unit) {
         return (int)unit.convert(maxAgeSeconds, TimeUnit.SECONDS);
     }
@@ -144,17 +100,6 @@ public class Cookie {
         return this;
     }
 
-    @Deprecated
-    public int getVersion() {
-        return version;
-    }
-
-    @Deprecated
-    public Cookie setVersion(int version) {
-        this.version = version;
-        return this;
-    }
-
     public boolean isSecure() {
         return secure;
     }
@@ -173,29 +118,12 @@ public class Cookie {
         return this;
     }
 
-    @Deprecated
-    public boolean isDiscard() {
-        return discard;
-    }
-
-    @Deprecated
-    public Cookie setDiscard(boolean discard) {
-        this.discard = discard;
-        return this;
-    }
-
-    @Deprecated
-    public Set<Integer> ports() {
-        return ports;
-    }
-
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         Cookie cookie = (Cookie) o;
         return maxAgeSeconds == cookie.maxAgeSeconds &&
-                version == cookie.version &&
                 secure == cookie.secure &&
                 httpOnly == cookie.httpOnly &&
                 discard == cookie.discard &&
@@ -203,14 +131,12 @@ public class Cookie {
                 Objects.equals(name, cookie.name) &&
                 Objects.equals(value, cookie.value) &&
                 Objects.equals(domain, cookie.domain) &&
-                Objects.equals(path, cookie.path) &&
-                Objects.equals(comment, cookie.comment) &&
-                Objects.equals(commentUrl, cookie.commentUrl);
+                Objects.equals(path, cookie.path);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(ports, name, value, domain, path, comment, commentUrl, maxAgeSeconds, version, secure, httpOnly, discard);
+        return Objects.hash(ports, name, value, domain, path, maxAgeSeconds, secure, httpOnly, discard);
     }
 
     @Override
@@ -229,19 +155,12 @@ public class Cookie {
         return StreamSupport.stream(cookies.spliterator(), false)
                 .map(cookie -> {
                     HttpCookie httpCookie = new HttpCookie(cookie.getName(), cookie.getValue());
-                    httpCookie.setComment(cookie.getComment());
-                    httpCookie.setCommentURL(cookie.getCommentURL());
-                    httpCookie.setDiscard(cookie.isDiscard());
                     httpCookie.setDomain(cookie.getDomain());
                     httpCookie.setHttpOnly(cookie.isHttpOnly());
                     httpCookie.setMaxAge(cookie.getMaxAge(TimeUnit.SECONDS));
                     httpCookie.setPath(cookie.getPath());
                     httpCookie.setSecure(cookie.isSecure());
-                    httpCookie.setVersion(cookie.getVersion());
-                    String portList = cookie.ports().stream()
-                            .map(Number::toString)
-                            .collect(Collectors.joining(","));
-                    httpCookie.setPortlist(portList);
+                    httpCookie.setVersion(0);
                     return httpCookie.toString();
                 })
                 .collect(Collectors.joining(";"));
@@ -255,29 +174,17 @@ public class Cookie {
                     Cookie cookie = new Cookie();
                     cookie.setName(servletCookie.getName());
                     cookie.setValue(servletCookie.getValue());
-                    cookie.setComment(servletCookie.getComment());
                     cookie.setPath(servletCookie.getPath());
                     cookie.setDomain(servletCookie.getDomain());
                     cookie.setMaxAge(servletCookie.getMaxAge(), TimeUnit.SECONDS);
                     cookie.setSecure(servletCookie.getSecure());
-                    cookie.setVersion(servletCookie.getVersion());
                     cookie.setHttpOnly(servletCookie.isHttpOnly());
                     return cookie;
                 })
                 .collect(Collectors.toList());
     }
 
-    /**
-     * @deprecated Use {@link #toSetCookieHeaderAll(Iterable)} instead.
-     */
-    @Deprecated
-    public static String toSetCookieHeader(Iterable<? extends Cookie> cookies) {
-        List<String> encodedCookies = toSetCookieHeaderAll(cookies);
-        return encodedCookies.isEmpty() ? null : encodedCookies.get(0);
-    }
-
-    // TODO Rename to toSetCookieHeader for Vespa 7
-    public static List<String> toSetCookieHeaderAll(Iterable<? extends Cookie> cookies) {
+    public static List<String> toSetCookieHeaders(Iterable<? extends Cookie> cookies) {
         // Ugly, bot Jetty does not provide a dedicated cookie parser (will be included in Jetty 10)
         Response response = new Response(null, null);
         for (Cookie cookie : cookies) {
@@ -293,45 +200,20 @@ public class Cookie {
         return new ArrayList<>(response.getHeaders("Set-Cookie"));
     }
 
-    // TODO Change return type to Cookie for Vespa 7
-    public static List<Cookie> fromSetCookieHeader(String headerVal) {
+    public static Cookie fromSetCookieHeader(String headerVal) {
         return HttpCookie.parse(headerVal).stream()
                 .map(httpCookie -> {
                     Cookie cookie = new Cookie();
                     cookie.setName(httpCookie.getName());
                     cookie.setValue(httpCookie.getValue());
-                    cookie.setComment(httpCookie.getComment());
-                    cookie.setCommentUrl(httpCookie.getCommentURL());
-                    cookie.setDiscard(httpCookie.getDiscard());
                     cookie.setDomain(httpCookie.getDomain());
                     cookie.setHttpOnly(httpCookie.isHttpOnly());
-                    cookie.setMaxAge((int)httpCookie.getMaxAge(), TimeUnit.SECONDS);
+                    cookie.setMaxAge((int) httpCookie.getMaxAge(), TimeUnit.SECONDS);
                     cookie.setPath(httpCookie.getPath());
                     cookie.setSecure(httpCookie.getSecure());
-                    cookie.setVersion(httpCookie.getVersion());
-                    cookie.ports().addAll(parsePortList(httpCookie.getPortlist()));
                     return cookie;
                 })
-                .collect(Collectors.toList());
-    }
-
-
-    private static List<Integer> parsePortList(String rawPortList) {
-        if (rawPortList == null) return Collections.emptyList();
-
-        List<Integer> ports = new ArrayList<>();
-        StringTokenizer tokenizer = new StringTokenizer(rawPortList, ",");
-        while (tokenizer.hasMoreTokens()) {
-            String rawPort = tokenizer.nextToken().trim();
-            if (!rawPort.isEmpty()) {
-                try {
-                    ports.add(Integer.parseInt(rawPort));
-                } catch (NumberFormatException e) {
-                    log.log(Level.FINE, "Unable to parse port: " + rawPort, e);
-                }
-            }
-        }
-        return ports;
+                .findFirst().get();
     }
 
 }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpResponse.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpResponse.java
index 8905549a002..f7138ba0e2b 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpResponse.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpResponse.java
@@ -69,7 +69,7 @@ public class HttpResponse extends Response implements ServletOrJdiscHttpResponse
         }
         List<Cookie> ret = new LinkedList<>();
         for (String cookie : cookies) {
-            ret.addAll(Cookie.fromSetCookieHeader(cookie));
+            ret.add(Cookie.fromSetCookieHeader(cookie));
         }
         return ret;
     }
@@ -77,7 +77,7 @@ public class HttpResponse extends Response implements ServletOrJdiscHttpResponse
     public void encodeSetCookieHeader(List<Cookie> cookies) {
         headers().remove(HttpHeaders.Names.SET_COOKIE);
         for (Cookie cookie : cookies) {
-            headers().add(HttpHeaders.Names.SET_COOKIE, Cookie.toSetCookieHeaderAll(Arrays.asList(cookie)));
+            headers().add(HttpHeaders.Names.SET_COOKIE, Cookie.toSetCookieHeaders(Arrays.asList(cookie)));
         }
     }
 
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapper.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapper.java
index 8845f233fab..2b9c650d545 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapper.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapper.java
@@ -8,7 +8,7 @@ import java.util.concurrent.TimeUnit;
 /**
  * Wrapper of Cookie.
  *
- * @author tejalk
+ * @author Tejal Knot
  *
  */
 public class JDiscCookieWrapper {
@@ -23,11 +23,6 @@ public class JDiscCookieWrapper {
         return new JDiscCookieWrapper(cookie);
     }
 
-    @Deprecated
-    public String getComment() {
-        return cookie.getComment();
-    }
-
     public String getDomain() {
         return cookie.getDomain();
     }
@@ -52,16 +47,6 @@ public class JDiscCookieWrapper {
         return cookie.getValue();
     }
 
-    @Deprecated
-    public int getVersion() {
-        return cookie.getVersion();
-    }
-
-    @Deprecated
-    public void setComment(String purpose) {
-        cookie.setComment(purpose);
-    }
-
     public void setDomain(String pattern) {
         cookie.setDomain(pattern);
     }
@@ -82,11 +67,6 @@ public class JDiscCookieWrapper {
        cookie.setValue(newValue);
     }
 
-    @Deprecated
-    public void setVersion(int version) {
-       cookie.setVersion(version);
-    }
-
     /**
      * Return com.yahoo.jdisc.http.Cookie
      *
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java
index 1c8383ae427..b603e7776f1 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterResponse.java
@@ -12,8 +12,6 @@ import java.util.List;
 
 /**
  * Servlet implementation for JDisc filter responses.
- * 
- * @since 5.27
  */
 class ServletFilterResponse extends DiscFilterResponse {
 
@@ -77,7 +75,7 @@ class ServletFilterResponse extends DiscFilterResponse {
     @Override
     public void setCookies(List<Cookie> cookies) {
         removeHeaders(HttpHeaders.Names.SET_COOKIE);
-        List<String> setCookieHeaders = Cookie.toSetCookieHeaderAll(cookies);
+        List<String> setCookieHeaders = Cookie.toSetCookieHeaders(cookies);
         setCookieHeaders.forEach(cookie -> addHeader(HttpHeaders.Names.SET_COOKIE, cookie));
     }
 }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java
index 5825d3f5194..48c8f577de9 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletResponse.java
@@ -18,8 +18,6 @@ import java.util.Map;
 /**
  * JDisc wrapper to use a {@link javax.servlet.http.HttpServletResponse}
  * with JDisc security filters.
- *
- * @since 5.26
  */
 public class ServletResponse extends HttpServletResponseWrapper implements ServletOrJdiscHttpResponse {
 
@@ -60,7 +58,7 @@ public class ServletResponse extends HttpServletResponseWrapper implements Servl
         }
         List<Cookie> ret = new LinkedList<>();
         for (String cookie : cookies) {
-            ret.addAll(Cookie.fromSetCookieHeader(cookie));
+            ret.add(Cookie.fromSetCookieHeader(cookie));
         }
         return ret;
     }
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java
index dab4f91f631..4c651f79666 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java
@@ -131,7 +131,6 @@ public class CookieTestCase {
     }
 
     @Test
-    @SuppressWarnings("deprecation")
     public void requireThatSetCookieCanBeDecoded() {
         final Cookie foo = new Cookie();
         foo.setName("foo.name");
@@ -141,7 +140,6 @@ public class CookieTestCase {
         foo.setMaxAge(0, TimeUnit.SECONDS);
         foo.setSecure(true);
         foo.setHttpOnly(true);
-        foo.setVersion(1);
         assertDecodeSetCookie(foo, "foo.name=foo.value;Max-Age=0;Path=path;Domain=domain;Secure;HTTPOnly;");
 
         final Cookie bar = new Cookie();
@@ -150,7 +148,6 @@ public class CookieTestCase {
         bar.setPath("path");
         bar.setDomain("domain");
         bar.setMaxAge(0, TimeUnit.SECONDS);
-        bar.setVersion(1);
         assertDecodeSetCookie(bar, "bar.name=bar.value;Max-Age=0;Path=path;Domain=domain;");
     }
 
@@ -193,11 +190,13 @@ public class CookieTestCase {
     }
 
     private static void assertEncodeCookie(String expectedResult, List<Cookie> cookies) {
-        assertThat(Cookie.toCookieHeader(cookies), equalTo(expectedResult));
+        String actual = Cookie.toCookieHeader(cookies);
+        String expectedResult1 = expectedResult;
+        assertThat(actual, equalTo(expectedResult1));
     }
 
     private static void assertEncodeSetCookie(List<String> expectedResult, List<Cookie> cookies) {
-        assertThat(Cookie.toSetCookieHeaderAll(cookies), containsInAnyOrder(expectedResult.toArray()));
+        assertThat(Cookie.toSetCookieHeaders(cookies), containsInAnyOrder(expectedResult.toArray()));
     }
 
     private static void assertDecodeCookie(List<Cookie> expected, String toDecode) {
@@ -205,7 +204,7 @@ public class CookieTestCase {
     }
 
     private static void assertDecodeSetCookie(final Cookie expected, String toDecode) {
-        assertThat(Cookie.fromSetCookieHeader(toDecode), containsInAnyOrder(expected));
+        assertThat(Cookie.fromSetCookieHeader(toDecode), equalTo(expected));
     }
 
     private static Cookie newCookie(final String name) {
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java
index 027d50317b2..d727be020cb 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java
@@ -92,7 +92,7 @@ public class HttpResponseTestCase {
         response.encodeSetCookieHeader(cookies);
         final List<String> headers = response.headers().get(HttpHeaders.Names.SET_COOKIE);
         assertEquals(1, headers.size());
-        assertEquals(Cookie.toSetCookieHeaderAll(cookies), headers);
+        assertEquals(Cookie.toSetCookieHeaders(cookies), headers);
     }
 
     @Test
@@ -102,7 +102,7 @@ public class HttpResponseTestCase {
         response.encodeSetCookieHeader(cookies);
         final List<String> headers = response.headers().get(HttpHeaders.Names.SET_COOKIE);
         assertEquals(2, headers.size());
-        assertEquals(Cookie.toSetCookieHeaderAll(Arrays.asList(new Cookie("foo", "bar"), new Cookie("baz", "cox"))),
+        assertEquals(Cookie.toSetCookieHeaders(Arrays.asList(new Cookie("foo", "bar"), new Cookie("baz", "cox"))),
                      headers);
     }
 
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapperTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapperTest.java
index 3439dc172ee..86ca424c90e 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapperTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/filter/JDiscCookieWrapperTest.java
@@ -7,7 +7,6 @@ import org.testng.annotations.Test;
 
 import java.util.concurrent.TimeUnit;
 
-@SuppressWarnings("deprecation")
 public class JDiscCookieWrapperTest {
 
     @Test
@@ -15,19 +14,15 @@ public class JDiscCookieWrapperTest {
         Cookie cookie = new Cookie("name", "value");
         JDiscCookieWrapper wrapper = JDiscCookieWrapper.wrap(cookie);
 
-        wrapper.setComment("comment");
         wrapper.setDomain("yahoo.com");
         wrapper.setMaxAge(10);
         wrapper.setPath("/path");
-        wrapper.setVersion(1);
 
         Assert.assertEquals(wrapper.getName(), cookie.getName());
         Assert.assertEquals(wrapper.getValue(), cookie.getValue());
         Assert.assertEquals(wrapper.getDomain(), cookie.getDomain());
-        Assert.assertEquals(wrapper.getComment(), cookie.getComment());
         Assert.assertEquals(wrapper.getMaxAge(), cookie.getMaxAge(TimeUnit.SECONDS));
         Assert.assertEquals(wrapper.getPath(), cookie.getPath());
-        Assert.assertEquals(wrapper.getVersion(), cookie.getVersion());
         Assert.assertEquals(wrapper.getSecure(), cookie.isSecure());
 
     }