From d46795bdb74edcd06f964a83c815d2fa7bf59c92 Mon Sep 17 00:00:00 2001
From: Markus Unterwaditzer <markus@unterwaditzer.net>
Date: Mon, 26 May 2014 22:19:35 +0200
Subject: [PATCH] Simplify error handling

---
 tests/storage/dav/test_main.py |  4 ++--
 vdirsyncer/storage/dav.py      | 35 ++++++++++++----------------------
 vdirsyncer/storage/http.py     |  1 -
 vdirsyncer/utils/__init__.py   |  8 +++++++-
 4 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/tests/storage/dav/test_main.py b/tests/storage/dav/test_main.py
index bb1c8cd..4ef21b8 100644
--- a/tests/storage/dav/test_main.py
+++ b/tests/storage/dav/test_main.py
@@ -197,7 +197,6 @@ class TestCaldavStorage(DavStorageTests):
 
         def request(session, method, url, data=None, headers=None, auth=None,
                     verify=None):
-            assert method == 'OPTIONS'
             assert url == args['url']
             calls.append(None)
 
@@ -209,7 +208,8 @@ class TestCaldavStorage(DavStorageTests):
         monkeypatch.setattr('requests.sessions.Session.request', request)
 
         with pytest.raises(ValueError):
-            self.storage_class(**args)
+            s = self.storage_class(**args)
+            list(s.list())
         assert len(calls) == 1
 
 
diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py
index f4c2a6a..a1c4a28 100644
--- a/vdirsyncer/storage/dav.py
+++ b/vdirsyncer/storage/dav.py
@@ -76,17 +76,6 @@ class DavStorage(Storage):
         self.parsed_url = utils.urlparse.urlparse(self.url)
         self.collection = collection
 
-        headers = self._default_headers()
-        headers['Depth'] = 1
-        response = self._request(
-            'OPTIONS',
-            '',
-            headers=headers
-        )
-        response.raise_for_status()
-        if self.dav_header not in response.headers.get('DAV', ''):
-            raise ValueError('URL is not a collection')
-
     def _default_headers(self):
         return {
             'User-Agent': self.useragent,
@@ -131,17 +120,21 @@ class DavStorage(Storage):
         assert path.startswith(self.parsed_url.path)
         if self._session is None:
             self._session = requests.session()
+            self._check_collection()
         url = self.parsed_url.scheme + '://' + self.parsed_url.netloc + path
         return utils.request(method, url, data=data, headers=headers,
                              session=self._session, **self._settings)
 
-    @staticmethod
-    def _check_response(response):
-        if response.status_code == 412:
-            raise exceptions.PreconditionFailed(response.reason)
-        if response.status_code == 404:
-            raise exceptions.NotFoundError(response.reason)
-        response.raise_for_status()
+    def _check_collection(self):
+        headers = self._default_headers()
+        headers['Depth'] = 1
+        response = self._request(
+            'OPTIONS',
+            '',
+            headers=headers
+        )
+        if self.dav_header not in response.headers.get('DAV', ''):
+            raise ValueError('URL is not a collection')
 
     def get(self, href):
         ((actual_href, item, etag),) = self.get_multi([href])
@@ -163,7 +156,6 @@ class DavStorage(Storage):
             data=data,
             headers=self._default_headers()
         )
-        self._check_response(response)
         root = etree.XML(response.content)  # etree only can handle bytes
         rv = []
         hrefs_left = set(hrefs)
@@ -207,7 +199,6 @@ class DavStorage(Storage):
             data=item.raw.encode('utf-8'),
             headers=headers
         )
-        self._check_response(response)
         etag = response.headers.get('etag', None)
         if not etag:
             item2, etag = self.get(href)
@@ -233,12 +224,11 @@ class DavStorage(Storage):
             'If-Match': etag
         })
 
-        response = self._request(
+        self._request(
             'DELETE',
             href,
             headers=headers
         )
-        self._check_response(response)
 
     def _list(self, xml):
         headers = self._default_headers()
@@ -261,7 +251,6 @@ class DavStorage(Storage):
             data=xml,
             headers=headers
         )
-        response.raise_for_status()
         root = etree.XML(response.content)
         for element in root.iter('{DAV:}response'):
             etag = element.find('{DAV:}propstat').find(
diff --git a/vdirsyncer/storage/http.py b/vdirsyncer/storage/http.py
index 4b5fe04..8878de6 100644
--- a/vdirsyncer/storage/http.py
+++ b/vdirsyncer/storage/http.py
@@ -103,7 +103,6 @@ class HttpStorage(Storage):
 
     def list(self):
         r = request('GET', self.url, **self._settings)
-        r.raise_for_status()
         self._items = {}
         rv = []
         for item in split_collection(r.text):
diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py
index 46462e1..ee91c81 100644
--- a/vdirsyncer/utils/__init__.py
+++ b/vdirsyncer/utils/__init__.py
@@ -11,7 +11,7 @@ import os
 import sys
 import requests
 
-from .. import log
+from .. import log, exceptions
 
 
 logger = log.get(__name__)
@@ -218,6 +218,12 @@ def request(method, url, data=None, headers=None, auth=None, verify=None,
     logger.debug(r.headers)
     logger.debug(r.content)
 
+    if r.status_code == 412:
+        raise exceptions.PreconditionFailed(r.reason)
+    if r.status_code == 404:
+        raise exceptions.NotFoundError(r.reason)
+
+    r.raise_for_status()
     return r