Permalink
Browse files

Even better handling of redirects and duplicate HTTP requests.

  • Loading branch information...
1 parent 8d300c7 commit 3436672f3a791e8910ce6da27e8097b3006208ee @mattbasta committed Jan 7, 2013
Showing with 41 additions and 20 deletions.
  1. +15 −0 appvalidator/errorbundle/metadatamixin.py
  2. +16 −8 appvalidator/testcases/webappbase.py
  3. +10 −12 tests/test_webapp_resources.py
@@ -26,6 +26,19 @@ def get_resource(self, name):
else:
return False
+ def get_or_create(self, name, default, pushable=False):
+ """Retrieve an object that has been stored by another test, or create
+ it if it does not exist.
+
+ """
+
+ if name in self.resources:
+ return self.resources[name]
+ if name in self.pushable_resources:
+ return self.pushable_resources[name]
+ else:
+ return self.save_resource(name, default, pushable=pushable)
+
def save_resource(self, name, resource, pushable=False):
"""Save an object such that it can be used by other tests."""
@@ -34,6 +47,8 @@ def save_resource(self, name, resource, pushable=False):
else:
self.resources[name] = resource
+ return resource
+
def _extend_json(self):
"""Output the metadata as part of the main JSON blob."""
extension = super(MetadataMixin, self)._extend_json() or {}
@@ -47,6 +47,7 @@ def try_get_data_uri(data_url):
def try_get_resource(err, package, url, filename, resource_type="URL",
max_size=True):
+
# Try to process data URIs first.
if url.startswith("data:"):
try:
@@ -76,6 +77,10 @@ def try_get_resource(err, package, url, filename, resource_type="URL",
filename=filename)
return
+ http_cache = err.get_or_create('http_cache', {})
+ if url in http_cache:
+ return http_cache[url]
+
def generic_http_error():
err.error(
err_id=("resources", "null_response"),
@@ -87,7 +92,8 @@ def generic_http_error():
filename=filename)
try:
- request = requests.get(url, prefetch=False)
+ request = requests.get(url, prefetch=False, allow_redirects=True,
+ timeout=3)
data = request.raw.read(constants.MAX_RESOURCE_SIZE)
# Check that there's not more data than the max size.
if max_size and request.raw.read(1):
@@ -107,10 +113,9 @@ def generic_http_error():
# Some versions of requests don't support close().
pass
- final_status = request.status_code
- final_status -= final_status % 100
+ http_cache[url] = data
- if not data and final_status != 300:
+ if not data:
generic_http_error()
return data
@@ -132,10 +137,6 @@ def generic_http_error():
"an invalid URL was encountered.",
"URL: %s" % url],
filename=filename)
- except (requests.exceptions.ConnectionError,
- requests.exceptions.Timeout,
- requests.exceptions.HTTPError):
- generic_http_error()
except requests.exceptions.TooManyRedirects:
err.error(
err_id=("resources", "too_many_redirects"),
@@ -146,6 +147,13 @@ def generic_http_error():
"permanent URL in an app.",
"Requested resource: %s" % url],
filename=filename)
+ except (requests.exceptions.ConnectionError,
+ requests.exceptions.Timeout,
+ requests.exceptions.HTTPError):
+ generic_http_error()
+
+ # Save the failed request to the cache.
+ http_cache[url] = None
def test_icon(err, data, url, size):
@@ -110,6 +110,16 @@ def test_TooManyRedirects(self):
appbase.try_get_resource(self.err, None, "http://foo.bar/", "")
self.assert_failed(with_errors=True)
+ @mock_requests(reqexc.TooManyRedirects, "Duplicate error")
+ def test_not_duplicated(self, r_g):
+ r_g.side_effect = reqexc.Timeout
+
+ appbase.try_get_resource(self.err, None, "http://foo.bar/", "")
+ self.assert_failed(with_errors=True)
+ appbase.try_get_resource(self.err, None, "http://foo.bar/", "")
+ assert len(self.err.errors) == 1, (
+ "HTTP errors should not be duplicated.")
+
class TestDataOutput(TestCase):
@@ -152,18 +162,6 @@ def test_empty(self, r_g):
self.err, None, "http://foo.bar/", ""), "")
self.assert_failed(with_errors=True)
- @patch("requests.get")
- @patch("appvalidator.constants.MAX_RESOURCE_SIZE", 100)
- def test_empty_redirect(self, r_g):
- empty_response = Mock()
- empty_response.raw.read.return_value = ""
- empty_response.status_code = 345
- r_g.return_value = empty_response
-
- eq_(appbase.try_get_resource(
- self.err, None, "http://foo.bar/", ""), "")
- self.assert_silent()
-
class TestResourcePolling(TestCase):

0 comments on commit 3436672

Please sign in to comment.