groovy

Add connection configuration options to URL.getText(), URL.newInputStream() and URL.newReader()

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 1.8.1
  • Component/s: groovy-jdk
  • Labels:
    None
  • Patch Submitted:
    Yes
  • Number of attachments :
    1

Description

There was a question on StackOverflow about how to set a timeout for URL.text

http://stackoverflow.com/questions/1839435/setting-timeout-for-new-url-text-in-groovy-grails

I have attached a patch which allows you to do:

def s = new URL( 'http://www.google.com' ).getText( 10 )

(which will give you a 10 millisecond timeout on both the connection and read – and return null if this timeout fires)

No Tests Supplied

I have no idea how you would test this in a non-contrived way, I have looked around for a URL.text test and it seems that I would need to mock the URL class, but I can't think how to do the timeout testing with this object

Hope it's ok...

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Cedric Champeau added a comment -

Patch will not be applied as is. We'll make an optional map argument instead, which allows configuring several things like the timeout.

Show
Cedric Champeau added a comment - Patch will not be applied as is. We'll make an optional map argument instead, which allows configuring several things like the timeout.
Hide
Permalink
Ronny Løvtangen added a comment -

Thread on the user list: http://groovy.329449.n5.nabble.com/Timeout-on-URL-text-td4459161.html

Show
Ronny Løvtangen added a comment - Thread on the user list: http://groovy.329449.n5.nabble.com/Timeout-on-URL-text-td4459161.html
Hide
Permalink
Cedric Champeau added a comment -

URL.getText, URL.newInputStream and URL.newReader now take an optional map argument which supports the following keys :

  • connectTimeout
  • readTimeout
  • useCaches
  • allowUserInteraction
  • requestProperties : a map <string,string> of request properties

Those options are passed to URLConnection, and you can combine it with the charset parameter :

url.text
url.getText(connectTimeout:500, readTimeout:2000)
url.getText('utf-8')
url.getText('utf-8', connectTimeout: 500)
url.getText('utf-8', connectTimeout: 500, readTimeout:2000)
url.newReader(useCaches:true)
url.newInputStream(useCaches:true)
url.getText(requestProperties:['User-agent':'GroovyBot'])

Fixed in 1.8.1 and trunk. I don't think we should backport it to 1.7.x, but if it is required, please comment on this issue.

Show
Cedric Champeau added a comment - URL.getText, URL.newInputStream and URL.newReader now take an optional map argument which supports the following keys : connectTimeout readTimeout useCaches allowUserInteraction requestProperties : a map <string,string> of request properties Those options are passed to URLConnection, and you can combine it with the charset parameter : url.text url.getText(connectTimeout:500, readTimeout:2000) url.getText('utf-8') url.getText('utf-8', connectTimeout: 500) url.getText('utf-8', connectTimeout: 500, readTimeout:2000) url.newReader(useCaches: true ) url.newInputStream(useCaches: true ) url.getText(requestProperties:['User-agent':'GroovyBot']) Fixed in 1.8.1 and trunk. I don't think we should backport it to 1.7.x, but if it is required, please comment on this issue.
Hide
Permalink
Ronny Løvtangen added a comment -

This is great!

For consistency, shouldn't charset be part of the map argument as well?
url.getText(charset: 'utf-8', connectTimeout: 500, readTimeout:2000)
Then there would be no need for the getText(String, Map) variant, only getText(Map) and getText(String).

Show
Ronny Løvtangen added a comment - This is great! For consistency, shouldn't charset be part of the map argument as well? url.getText(charset: 'utf-8', connectTimeout: 500, readTimeout:2000) Then there would be no need for the getText(String, Map) variant, only getText(Map) and getText(String).
Hide
Permalink
Cedric Champeau added a comment -

I thought about it, but it would not make sense for #newInputStream. I prefer having a coherent API where all the variants accept the same parameters.

Show
Cedric Champeau added a comment - I thought about it, but it would not make sense for #newInputStream. I prefer having a coherent API where all the variants accept the same parameters.
Hide
Permalink
Evgeny Goldin added a comment -

Cedric, does it make sense to have a similar configuration Map for getBytes()?

Show
Evgeny Goldin added a comment - Cedric, does it make sense to have a similar configuration Map for getBytes()?

People

  • Assignee:
    Cedric Champeau
    Reporter:
    Tim Yates
Vote (3)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: