CookieAgent to add support for HTTP cookie handling

[gabrtv]

This patch extents the twisted.web.client.Agent to add support for handling HTTP cookies. The new CookieLibAgent requires a cookieJar that implements the cookielib.CookieJar interface. Cookies are then automatically set and extracted during Agent request processing.

At the time of writing, this patch was built atop #3420 (Agent Persistent Connections).

This submission does not yet have tests. However it is performing relatively well inside a larger unit test infrastructure. I will try to add tests as my schedule permits, but I wanted to contribute the patch now in case others might find it useful.

[gabrtv]

CookieAgent implementation

[gabrtv]

I should note that CookieAgent supports FileCookieJars which presumably perform some blocking file i/o (such as cookielib.MozillaCookieJar). Not sure if that's a problem or not.

[exarkun]

Thanks for working on this gabrtv. It will be great to have cookie support for Agent in Twisted.

The primary comment I have about this patch is that it would be simpler and more reusable if it composed a cookie-aware Agent-alike with Agent instead of subclassing Agent. This will also remove the dependency on the persistent connections branch.

[gabrtv]

CookieAgent, composed

[gabrtv]

composed, minus dependency on persistent connections

[gabrtv]

CookieAgent patch is now composed, and should no longer have a dependency on agent persistent connections branch.

[exarkun]

This is meant to be in review.

[therve]

Thanks for the patch. The review:

• You need to add tests.

• The coding standard mentions 2 blank lines between methods, 3 between classes.

• def __init__(self, agent, cookieJar, *args, **kwargs): It seems the extra arguments are unused.

• It seems that the docstring of the request methods is an exact copy of the one of Agent.request. You should rather refer to Agent.request, and mentions the specific bits.

• There may be a design problem with the way you manage the cookie jar: an Agent is stateless, so you should be able to issue as many requests as you want against one. The jar is "global" to the CookieAgent, so is modified after each response.

• You set the cookie header directly on the header object. First, I think you forgot that headers is an optional argument, so it's necessarily existing. Then, I think we try not to modify the passed argument, so you should make a copy of the headers before.

• It would be nice if you could define a fake urllib2 Request class, as you did for the response.

[jonathanj]

(In [31792]) Branching to 'cookie-agent-4922'

[jonathanj]

(In [31805]) Apply cookieagent_composed2.patch from gabrtv. Address review commentary. Refs #4922.

[jonathanj]

Replying to therve:

* There may be a design problem with the way you manage the cookie jar: an Agent is stateless, so you should be able to issue as many requests as you want against one. The jar is "global" to the CookieAgent, so is modified after each response.

As we discussed on IRC, I think that this is probably how you want CookieAgent to work. If you need separate cookie jars use separate cookie jars and agents; I added a sentence in the CookieAgent docstring to clarify that the cookie jar will be mutated.

Addressed all your other review points.

[therve]

Looks great. Some comments:

• Please refer to the new IResponse interface instead of the Response class in docstrings.
• The NEWS fragment should be slightly rephrased I think. Maybe "A new HTTP cookie-aware Twisted Web Agent wrapper is included in twisted.web.client.CookieAgent".

Please merge once fixed.

[jonathanj]

(In [31863]) Refer to twisted.web.iweb.IResponse instead of twisted.web.client.Response, refs #4922.

[jonathanj]

(In [31868]) Merge cookies-agent-4922.

Author: gabrtv, jonathanj Reviewer: exarkun, therve Fixes: #4922

Introduce twisted.web.client.CookieAgent, an HTTP cookie-aware Agent wrapper, that will read and write cookies to a cookielib.CookieJar instance.