Feature Proposal: Add LWP parameters to TWiki::Func::getExternalResource

Motivation

The motivation comes from discussion at SingleSignOnForINCLUDE.

It is ideal for TWikiFuncDotPm to provide a way to customize internal LWP parameters, so that plugin authors can utilize advanced HTTP request to external resources.

Currently, getExternalResource is limited to a simple GET request to a particular URL, although it encapsulates historical enhancement at the TWiki level such as proxy settings via config.

Description and Documentation

Proposed interface:

getExternalResource($url, \@headers, \%params ) -> $response (HTTP::Response)

postExternalResource($url, $text, \@headers, \%params ) -> $response (HTTP::Response)

The interim implementation with getExternalResource($url, @headers) (non-ref @headers) is in consideration to be removed.

Examples

Add headers

my $res = TWiki::Func::getExternalResource($url, ['Cache-Control' => 'max-age=0']);

Set timeout

my $res = TWiki::Func::getExternalResource($url, undef, {timeout => 10});

Set HTTP::Cookies

my $res = TWiki::Func::getExternalResource($url, undef, {cookie_jar => $cookies});

POST

my $res = TWiki::Func::postExternalResource($url, $content);

Impact

Implementation

-- Contributors: MahiroAndo - 2012-10-18

Discussion

I like the getExternalResource($url, \@headers, \%lwpParams, $postContent) -> $response (HTTP::Response) interface.

If I understand correctly, you will detect the type of the second parameter, and for compatibility handle the call differently if it is a reference. Not sure how reliable this is. But should be OK if we un-document the old spec.

I checked, it turns out I added the @headers parameters in June, tracked in TWikibug:Item6892. I needed this feature for the SsoLoginContrib. I checked all extensions in SVN trunk, this contrib is the only one currently using the header feature. Since I have deployed it only on one site, and trunk code is not yet released, it is not too late to change the API. However, let's do it the way you propose if you feel the mode can be detected reliably.

On another note, in your examples, shouldn't the second example have an undef, as in:

my $res = TWiki::Func::getExternalResource($url, undef, {timeout => 10});

Same for other examples, shouldn't they have undef for unused params?

-- PeterThoeny - 2012-10-18

The idea I had in mind was to detect the parameter types in the getExternalResource regardless of the position of the arguments:

• An array ref is always a list of headers
• A hash ref is always for LWP
• A scalar is a POST content

It makes it easy to use the function because you don't need to remember which one comes first. It also works even when undef is added. The API can be better expressed this way:

• getExternalResource($url, [ \@headers | \%lwpParams | $postContent ]...)

If the above sounds counter-intuitive, it is no problem with me to implement it with the strict positions (2nd one is \@headers, 3rd one is \%lwpParams, and the 4th one is $postContent).

As for the backward compatibility one, thanks for checking. TBH, supporting the non-ref @headers makes things complicated in implementation. It would be simpler to support only one way. It looks like the only change that is necessary for SsoLoginContrib is to change @headers to \@headers, so I second your option to un-document it.

-- MahiroAndo - 2012-10-22

We have not used this type of interchangeable parameters in the Func API. I am OK to have it introduced. May be implement as you suggest, but document as fixed pos params?

I am OK with fixing the SsoLoginContrib.

-- PeterThoeny - 2012-10-23

How about named parameters, similar to \%options in TWikiFuncDotPm?

getExternalResource($url, {useragent => {...}, headers => [...], content => '...'})

(CPAN:RPC::XML::Client also uses a similar param for useragent.)

This way, we can add method => 'HEAD' for example, and possibly more, if necessary.

-- MahiroAndo - 2012-10-24

IMHO, nested hashes (ref to hash of (hash or array or string) are possibly hard to understand for entry level programmers. I recommend to document the list of parameters, and if you feel implement with detection of type.

Also, I find the name "getXyz" a bit confusing to "post" something. Semantically it might be better to distinguish between get and post, e.g. to have distinct functions:

getExternalResource( $url, \@headers, \%params );

postExternalResource( $url, $text, \@headers, \%params );

For both functions, headers and params can be optional. What do you think?

-- PeterThoeny - 2012-10-24

On additional parameters in the future like method => 'HEAD', I recommend to document the 3rd paramers just as params instead of lwpParams. That way we can add additional key=value pairs to the same hash ref in the future. The ones that lwp supports should be documented anyway, so we can document the non-lwp ones the same way. The only downside is that we can't clobber the namespace of lwp parameters with our own ones.

-- PeterThoeny - 2012-10-24

Sounds great. I like the idea of postExternalResource rather than posting with getExternalResource

I also agree with regarding params as more generic. I will try to implement and document these two functions.

-- MahiroAndo - 2012-10-24

Thank you for contributing to the refined spec. I feel we have a sound spec now.

-- PeterThoeny - 2012-10-25

Thank you too for all the suggestions. It has been in much better shape than I originally came up with!

Committed to SVN (TWikibug:Item7010). Please feel free to suggest or fix if anything.

-- MahiroAndo - 2012-10-25