groovy
  1. groovy
  2. GROOVY-3921

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

        Hide
        CÚdric 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
        CÚdric 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
        Ronny L°vtangen added a comment - Thread on the user list: http://groovy.329449.n5.nabble.com/Timeout-on-URL-text-td4459161.html
        Hide
        CÚdric 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
        CÚdric 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
        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
        CÚdric 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
        CÚdric 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
        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:
            CÚdric Champeau
            Reporter:
            Tim Yates
          • Votes:
            3 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: