You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by mg...@apache.org on 2013/02/19 15:59:16 UTC
git commit: WICKET-4989 WicketTester should send copies of its cookies
Updated Branches:
refs/heads/master e5058b118 -> 437a55f0b
WICKET-4989 WicketTester should send copies of its cookies
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/437a55f0
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/437a55f0
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/437a55f0
Branch: refs/heads/master
Commit: 437a55f0bc0b56fc21dc43e2bbda9cc5445dbed9
Parents: e5058b1
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
Authored: Tue Feb 19 16:58:19 2013 +0200
Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
Committed: Tue Feb 19 16:59:04 2013 +0200
----------------------------------------------------------------------
.../apache/wicket/protocol/http/mock/Cookies.java | 65 ++++++++++
.../protocol/http/mock/MockHttpServletRequest.java | 10 +-
.../http/mock/MockHttpServletResponse.java | 11 +-
.../wicket/util/tester/BaseWicketTester.java | 95 ++++++++++----
.../SetCookieAndRedirectStatefullTestPage.java | 2 +-
.../util/cookies/SetCookieAndRedirectTest.java | 4 +-
.../org/apache/wicket/util/tester/CookiePage.java | 21 ++--
.../wicket/util/tester/WicketTesterTest.java | 86 +++++++++-----
8 files changed, 214 insertions(+), 80 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/wicket/blob/437a55f0/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/Cookies.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/Cookies.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/Cookies.java
new file mode 100644
index 0000000..68f3083
--- /dev/null
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/Cookies.java
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.protocol.http.mock;
+
+import javax.servlet.http.Cookie;
+
+import org.apache.wicket.util.lang.Args;
+
+/**
+ * A helper class for dealing with cookies
+ */
+public final class Cookies
+{
+ /**
+ * Constructor.
+ */
+ private Cookies()
+ {}
+
+ /**
+ * Make a copy of the passed cookie.
+ *
+ * @param cookie
+ * The cookie to copy
+ * @return A copy of the passed cookie. May be {@code null} if the argument is {@code null}.
+ */
+ public static Cookie copyOf(Cookie cookie)
+ {
+ return cookie != null ? (Cookie) cookie.clone() : null;
+ }
+
+ /**
+ * Checks whether two cookies are equal.
+ * See http://www.ietf.org/rfc/rfc2109.txt, p.4.3.3
+ *
+ * @param c1
+ * the first cookie
+ * @param c2
+ * the second cookie
+ * @return {@code true} only if the cookies have the same name, path and domain
+ */
+ public static boolean isEqual(Cookie c1, Cookie c2)
+ {
+ Args.notNull(c1, "c1");
+ Args.notNull(c2, "c2");
+
+ return c1.getName().equals(c2.getName()) &&
+ ((c1.getPath() == null && c2.getPath() == null) || (c1.getPath().equals(c2.getPath()))) &&
+ ((c1.getDomain() == null && c2.getDomain() == null) || (c1.getDomain().equals(c2.getDomain())));
+ }
+}
http://git-wip-us.apache.org/repos/asf/wicket/blob/437a55f0/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/MockHttpServletRequest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/MockHttpServletRequest.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/MockHttpServletRequest.java
index e97ae3a..b760093 100755
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/MockHttpServletRequest.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/MockHttpServletRequest.java
@@ -457,7 +457,7 @@ public class MockHttpServletRequest implements HttpServletRequest
{
if (cookie.getName().equals(name))
{
- return cookie;
+ return Cookies.copyOf(cookie);
}
}
return null;
@@ -476,7 +476,11 @@ public class MockHttpServletRequest implements HttpServletRequest
return null;
}
Cookie[] result = new Cookie[cookies.size()];
- return cookies.toArray(result);
+ for (int i = 0; i < cookies.size(); i++)
+ {
+ result[i] = Cookies.copyOf(cookies.get(i));
+ }
+ return result;
}
/**
@@ -1308,7 +1312,7 @@ public class MockHttpServletRequest implements HttpServletRequest
public void setCookies(final Cookie[] theCookies)
{
cookies.clear();
- cookies.addAll(Arrays.asList(theCookies));
+ addCookies(Arrays.asList(theCookies));
}
/**
http://git-wip-us.apache.org/repos/asf/wicket/blob/437a55f0/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/MockHttpServletResponse.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/MockHttpServletResponse.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/MockHttpServletResponse.java
index af09060..a522bd5 100755
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/MockHttpServletResponse.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/mock/MockHttpServletResponse.java
@@ -108,9 +108,7 @@ public class MockHttpServletResponse implements HttpServletResponse, IMetaDataBu
while (iterator.hasNext())
{
Cookie old = iterator.next();
- if (cookie.getName().equals(old.getName()) &&
- ((cookie.getPath() == null && old.getPath() == null) || (cookie.getPath().equals(old.getPath()))) &&
- ((cookie.getDomain() == null && old.getDomain() == null) || (cookie.getDomain().equals(old.getDomain()))))
+ if (Cookies.isEqual(cookie, old))
{
iterator.remove();
}
@@ -296,7 +294,12 @@ public class MockHttpServletResponse implements HttpServletResponse, IMetaDataBu
*/
public List<Cookie> getCookies()
{
- return cookies;
+ List<Cookie> copies = new ArrayList<Cookie>();
+ for (Cookie cookie : cookies)
+ {
+ copies.add(Cookies.copyOf(cookie));
+ }
+ return copies;
}
/**
http://git-wip-us.apache.org/repos/asf/wicket/blob/437a55f0/wicket-core/src/main/java/org/apache/wicket/util/tester/BaseWicketTester.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/util/tester/BaseWicketTester.java b/wicket-core/src/main/java/org/apache/wicket/util/tester/BaseWicketTester.java
index fdd3502..a518419 100644
--- a/wicket-core/src/main/java/org/apache/wicket/util/tester/BaseWicketTester.java
+++ b/wicket-core/src/main/java/org/apache/wicket/util/tester/BaseWicketTester.java
@@ -29,6 +29,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -97,6 +98,7 @@ import org.apache.wicket.page.IPageManagerContext;
import org.apache.wicket.protocol.http.IMetaDataBufferingWebResponse;
import org.apache.wicket.protocol.http.WebApplication;
import org.apache.wicket.protocol.http.WicketFilter;
+import org.apache.wicket.protocol.http.mock.Cookies;
import org.apache.wicket.protocol.http.mock.MockHttpServletRequest;
import org.apache.wicket.protocol.http.mock.MockHttpServletResponse;
import org.apache.wicket.protocol.http.mock.MockHttpSession;
@@ -183,8 +185,6 @@ public class BaseWicketTester
private IRequestHandler forcedHandler;
private IFeedbackMessageFilter originalFeedbackMessageCleanupFilter;
- // Simulates the cookies maintained by the browser
- private final List<Cookie> browserCookies = Generics.newArrayList();
private ComponentInPage componentInPage;
@@ -369,10 +369,28 @@ public class BaseWicketTester
request.setServerPort(lastRequest.getServerPort());
}
- transferCookies();
+ transferRequestCookies();
response = new MockHttpServletResponse(request);
+ // Preserve response cookies in redirects
+ // XXX: is this really needed ? Browsers wont do that, but some
+ // Wicket tests assert that a cookie is in the response,
+ // even after redirects (see org.apache.wicket.util.cookies.SetCookieAndRedirectTest.statefulPage())
+ // They should assert that the cookie is in the next *request*
+ if (lastResponse != null && lastResponse.isRedirect())
+ {
+ List<Cookie> lastResponseCookies = lastResponse.getCookies();
+ for (Cookie cookie : lastResponseCookies)
+ {
+ if (cookie.getMaxAge() != 0)
+ {
+ // max-age==0 are already handled in #transferRequestCookies() above
+ response.addCookie(cookie);
+ }
+ }
+ }
+
ServletWebRequest servletWebRequest = newServletWebRequest();
requestCycle = application.createRequestCycle(servletWebRequest,
newServletWebResponse(servletWebRequest));
@@ -417,11 +435,25 @@ public class BaseWicketTester
}
/**
- * Copies all cookies with a positive age from the last response to the request that is going to
- * be used for the next cycle.
+ * Simulates browser behavior by preserving all non-removed cookies from
+ * the previous request.
+ * A cookie is removed if the response contains a cookie with the same
+ * name, path and domain and max-age=0
*/
- private void transferCookies()
+ private void transferRequestCookies()
{
+ List<Cookie> lastRequestCookies = new ArrayList<Cookie>();
+
+ // copy all cookies from the previous request
+ if (lastRequest != null && lastRequest.getCookies() != null)
+ {
+ for (Cookie lastRequestCookie : lastRequest.getCookies())
+ {
+ lastRequestCookies.add(lastRequestCookie);
+ }
+ }
+
+ // filter out all removed cookies
if (lastResponse != null)
{
List<Cookie> cookies = lastResponse.getCookies();
@@ -432,13 +464,40 @@ public class BaseWicketTester
// maxAge == -1 -> means session cookie
// maxAge == 0 -> delete the cookie
// maxAge > 0 -> the cookie will expire after this age
- if (cookie.getMaxAge() != 0)
+ if (cookie.getMaxAge() == 0)
{
- request.addCookie(cookie);
+ Iterator<Cookie> cookieIterator = lastRequestCookies.iterator();
+ while (cookieIterator.hasNext())
+ {
+ Cookie lastRequestCookie = cookieIterator.next();
+ if (Cookies.isEqual(lastRequestCookie, cookie))
+ {
+ cookieIterator.remove();
+ }
+ }
+ }
+ else
+ {
+ boolean newlyCreated = true;
+ for (Cookie oldCookie : lastRequestCookies)
+ {
+ if (Cookies.isEqual(cookie, oldCookie))
+ {
+ newlyCreated = false;
+ break;
+ }
+ }
+ if (newlyCreated)
+ {
+ lastRequestCookies.add(cookie);
+ }
}
}
}
}
+
+ // transfer only the non-removed ones
+ request.addCookies(lastRequestCookies);
}
/**
@@ -627,15 +686,7 @@ public class BaseWicketTester
try
{
- if (getLastResponse() != null)
- {
- // transfer cookies from previous response to this request, quirky but how old stuff
- // worked...
- for (Cookie cookie : getLastResponse().getCookies())
- {
- request.addCookie(cookie);
- }
- }
+ transferRequestCookies();
applyRequest();
requestCycle.scheduleRequestHandlerAfterCurrent(null);
@@ -781,16 +832,6 @@ public class BaseWicketTester
previousRequests.add(request);
previousResponses.add(response);
-
- // transfer cookies from previous request to previous response, quirky but how old stuff
- // worked...
- if (lastRequest.getCookies() != null)
- {
- for (Cookie cookie : lastRequest.getCookies())
- {
- lastResponse.addCookie(cookie);
- }
- }
}
/**
http://git-wip-us.apache.org/repos/asf/wicket/blob/437a55f0/wicket-core/src/test/java/org/apache/wicket/util/cookies/SetCookieAndRedirectStatefullTestPage.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/util/cookies/SetCookieAndRedirectStatefullTestPage.java b/wicket-core/src/test/java/org/apache/wicket/util/cookies/SetCookieAndRedirectStatefullTestPage.java
index 69c4a48..9c717e8 100644
--- a/wicket-core/src/test/java/org/apache/wicket/util/cookies/SetCookieAndRedirectStatefullTestPage.java
+++ b/wicket-core/src/test/java/org/apache/wicket/util/cookies/SetCookieAndRedirectStatefullTestPage.java
@@ -24,7 +24,7 @@ import org.apache.wicket.model.Model;
/**
- * Statefull form page which sets a cookie and calls setResponsePage()
+ * Stateful form page which sets a cookie and calls setResponsePage()
*
* @author Bertrand Guay-Paquet
*/
http://git-wip-us.apache.org/repos/asf/wicket/blob/437a55f0/wicket-core/src/test/java/org/apache/wicket/util/cookies/SetCookieAndRedirectTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/util/cookies/SetCookieAndRedirectTest.java b/wicket-core/src/test/java/org/apache/wicket/util/cookies/SetCookieAndRedirectTest.java
index 0dd2617..841b890 100644
--- a/wicket-core/src/test/java/org/apache/wicket/util/cookies/SetCookieAndRedirectTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/util/cookies/SetCookieAndRedirectTest.java
@@ -35,10 +35,10 @@ public class SetCookieAndRedirectTest extends WicketTestCase
private static final String cookieValue = "cookieValue";
/**
- * Validate proper cookie value set with statefull page
+ * Validate proper cookie value set with stateful page
*/
@Test
- public void statefullPage()
+ public void statefulPage()
{
tester.startPage(SetCookieAndRedirectStatefullTestPage.class);
FormTester formTester = tester.newFormTester("form");
http://git-wip-us.apache.org/repos/asf/wicket/blob/437a55f0/wicket-core/src/test/java/org/apache/wicket/util/tester/CookiePage.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/util/tester/CookiePage.java b/wicket-core/src/test/java/org/apache/wicket/util/tester/CookiePage.java
index cda3968..0c4d97f 100644
--- a/wicket-core/src/test/java/org/apache/wicket/util/tester/CookiePage.java
+++ b/wicket-core/src/test/java/org/apache/wicket/util/tester/CookiePage.java
@@ -16,9 +16,11 @@
*/
package org.apache.wicket.util.tester;
-import junit.framework.Assert;
+import javax.servlet.http.Cookie;
-import org.apache.wicket.util.cookies.CookieUtils;
+import junit.framework.Assert;
+import org.apache.wicket.request.http.WebRequest;
+import org.apache.wicket.request.http.WebResponse;
/**
* A test page for https://issues.apache.org/jira/browse/WICKET-4289
@@ -42,15 +44,6 @@ public class CookiePage extends DummyHomePage
{
cookieName = name;
cookieValue = value;
-
- doAssert();
- }
-
- private void doAssert()
- {
- CookieUtils utils = new CookieUtils();
- String v = utils.load(cookieName);
- Assert.assertEquals(cookieValue, v);
}
@Override
@@ -58,7 +51,11 @@ public class CookiePage extends DummyHomePage
{
super.onConfigure();
- doAssert();
+ Cookie cookie = ((WebRequest) getRequest()).getCookie(cookieName);
+ Assert.assertEquals(cookieValue, cookie.getValue());
+
+ WebResponse response = (WebResponse) getResponse();
+ response.addCookie(cookie);
}
http://git-wip-us.apache.org/repos/asf/wicket/blob/437a55f0/wicket-core/src/test/java/org/apache/wicket/util/tester/WicketTesterTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/util/tester/WicketTesterTest.java b/wicket-core/src/test/java/org/apache/wicket/util/tester/WicketTesterTest.java
index b8612ff..cedd26d 100644
--- a/wicket-core/src/test/java/org/apache/wicket/util/tester/WicketTesterTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/util/tester/WicketTesterTest.java
@@ -73,6 +73,7 @@ import org.apache.wicket.util.tester.apps_1.SuccessPage;
import org.apache.wicket.util.tester.apps_1.ViewBook;
import org.apache.wicket.util.tester.apps_6.LinkPage;
import org.apache.wicket.util.tester.apps_6.ResultPage;
+import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -285,7 +286,7 @@ public class WicketTesterTest extends WicketTestCase
tester.assertRenderedPage(LinkPage.class);
tester.getComponentFromLastRenderedPage("ajaxLinkWithSetResponsePageClass").setEnabled(
- false);
+ false);
try
{
tester.assertEnabled("ajaxLinkWithSetResponsePageClass");
@@ -698,7 +699,7 @@ public class WicketTesterTest extends WicketTestCase
FormTester form = tester.newFormTester("form");
form.setValue("name", "New name");
tester.executeAjaxEvent(MockPageWithFormAndAjaxFormSubmitBehavior.EVENT_COMPONENT,
- "onclick");
+ "onclick");
MockPageWithFormAndAjaxFormSubmitBehavior page = (MockPageWithFormAndAjaxFormSubmitBehavior)tester.getLastRenderedPage();
Pojo pojo = page.getPojo();
@@ -864,7 +865,7 @@ public class WicketTesterTest extends WicketTestCase
BlockedResourceLinkPage.class.getSimpleName() + ".html,xml";
tester.executeUrl(url);
assertNull("Comma separated extensions are not supported and wont find any resource",
- tester.getLastResponse());
+ tester.getLastResponse());
}
/**
@@ -925,34 +926,6 @@ public class WicketTesterTest extends WicketTestCase
}
/**
- *
- */
- @Test
- public void cookieIsFoundOnNextRequestWhenAddedToResponse()
- {
- // Test that maxAge == -1 (Default) works properly
- tester.startPage(CreateBook.class);
- Cookie cookie = new Cookie("name", "value");
- tester.getLastResponse().addCookie(cookie);
- tester.startPage(CreateBook.class);
- assertEquals("value", tester.getLastResponse().getCookies().iterator().next().getValue(),
- "value");
-
- tester.startPage(CreateBook.class);
- cookie = new Cookie("name", "value");
- cookie.setMaxAge(60);
- tester.getLastResponse().addCookie(cookie);
- tester.startPage(CreateBook.class);
- assertEquals("value", tester.getLastResponse().getCookies().iterator().next().getValue(),
- "value");
-
- // Should copy persisted cookie from browser
- tester.startPage(CreateBook.class);
- assertEquals("value", tester.getLastResponse().getCookies().iterator().next().getValue(),
- "value");
- }
-
- /**
* Test for WICKET-3123
*/
@Test
@@ -1344,4 +1317,55 @@ public class WicketTesterTest extends WicketTestCase
tester.submitForm(page.form);
assertEquals(null, page.text);
}
+
+ /**
+ * A cookie set in the request headers should not be
+ * expected in the response headers unless the page
+ * sets it explicitly.
+ *
+ * https://issues.apache.org/jira/browse/WICKET-4989
+ */
+ @Test
+ public void cookieSetInRequestShouldNotBeInResponse()
+ {
+ //start and render the test page
+ tester.getRequest().addCookie(new Cookie("dummy", "sample"));
+ tester.startPage(tester.getApplication().getHomePage());
+
+ //assert rendered page class
+ tester.assertRenderedPage(tester.getApplication().getHomePage());
+
+ Assert.assertEquals("The cookie should not be in the response unless explicitly set",
+ 0, tester.getLastResponse().getCookies().size());
+
+ // The cookie should be in each following request unless the server code
+ // schedules it for removal it with cookie.setMaxAge(0)
+ Assert.assertEquals("The cookie should be in each following request",
+ 1, tester.getRequest().getCookies().length);
+ }
+
+ /**
+ * The response cookie should not be the same instance as the request
+ * cookie.
+ *
+ * https://issues.apache.org/jira/browse/WICKET-4989
+ */
+ @Test
+ public void doNotReuseTheSameInstanceOfTheCookieForRequestAndResponse()
+ {
+ //start and render the test page
+ String cookieName = "cookieName";
+ String cookieValue = "cookieValue";
+ Cookie requestCookie = new Cookie(cookieName, cookieValue);
+ tester.getRequest().addCookie(requestCookie);
+ tester.startPage(new CookiePage(cookieName, cookieValue));
+
+ //assert rendered page class
+ tester.assertRenderedPage(CookiePage.class);
+
+ Cookie responseCookie = tester.getLastResponse().getCookies().get(0);
+ requestCookie.setValue("valueChanged");
+
+ Assert.assertEquals(cookieValue, responseCookie.getValue());
+ }
}