Separate out the URL regex in geturl()

[coleshaw]

To make it a little easier to test, I suggest separating out the two regex lines in geturl:

result = re.search('((http[s]?):\/)?\/?([^:\/\s]+)((\/\w+)*\/)([\w\-\.]+[^#?\s]+)(' + ext + ')', x)
fileurl = result.group(0) # just the url

into a separate method, then write a ton of tests against that one. Test for the three URL things that you mentioned in #1 . If any of the inputs are invalid, then this new method should throw an exception that geturl should handle / catch appropriately.

• Invalid URLs (404 / 403 / 500 status codes).
• Invalid characters
• Escaping (does Telegram need the URL to be escaped? You can use urllib.parse.quote for that)

Also, are you limiting it to one URL? Are authors allowed to paste multiple URLs in a message? Right now the code only handles the first one, but if there isn't anything preventing the authors from putting in multiple, then the system won't behave as expected. So I would consider some sort of form validation or change geturl to handle multiple results from the regex...and also add a test for that. (0 URLs, 1 URL, 2 URLs).

A comment with examples for what that regex is supposed to filter would also be handy. I can tell it's a URL of sorts, but not the path stuff...and if the URL needs to be escaped before sending it off to Telegram, it would be good to add that to the method.

[coleshaw]

So I take back what I said about handling invalid URLs. I think it depends on if / how you're doing form validation (that would be a better spot for checking invalid URLs), and where Telegram blows up if it gets one. But one downside to doing it in the method here is that the check would slow down the response back to the user. So it depends if it's more important to return a valid URL than response speed. Up to the requirements and your best judgement, I think, of the tradeoffs involved...