Merge pull request #1 from allrod5/do-not-raise

 Better handle constructor parameters

.travis.yml

@@ -1,7 +1,6 @@
 language: python
 
 python:
-  - 2.6
   - 2.7
   - 3.5
   - 3.6

staticmaps_signature/__init__.py

@@ -1,6 +1,6 @@
 from staticmaps_signature.signature import StaticMapURLSigner
 
 __all__ = [StaticMapURLSigner]
-__version__ = '0.1.4'
+__version__ = '0.2.0'
 __author__ = 'Rodrigo Martins de Oliveira'
 __email__ = 'allrod5@hotmail.com'

staticmaps_signature/signature.py

@@ -32,9 +32,16 @@ def __init__(
         to sign will already contain the `&key` or `&client_id` query
         parameter.
 
-        Parameters `client_id` and `public_key` are mutually exclusive
-        and must not be both set. When parameter `client_id` is set,
-        then setting also parameter `private_key` is mandatory.
+        Parameters `client_id` and `public_key` are mutually exclusive.
+        When both are provided, `client_id` will be used in favor of
+        `public_key` if a `private_key` is provided, `public_key` will
+        be used otherwise.
+
+        When parameter `client_id` is set and `public_key` is not, then
+        parameter `private_key` is mandatory.
+
+        If unable to sign the URL by any reason then a warning will be
+        logged and the original URL will be returned as it is.
 
         Args:
         client_id       - StaticMap Client ID
@@ -48,18 +55,19 @@ def __init__(
         self.verify_endpoint = verify_endpoint
         self.staticmap_api_endpoint = urlparse.urlparse(
             "https://maps.googleapis.com/maps/api/staticmap")
-        if self.client_id is not None:
-            if self.public_key is not None:
-                raise ValueError(
-                    "Parameters `client_id` and `public_key` are"
-                    " mutually exclusive")
-            if self.private_key is None:
-                raise ValueError(
-                    "Parameter `private_key` is required when"
-                    " using `client_id`")
-        elif self.public_key is None and self.private_key is None:
-            raise ValueError(
-                "At least one of `public_key` or `private_key` must be set")
+        self.url_model = "{scheme}://{netloc}{path}?{query_string}"
+        self.no_op = self.public_key is None and self.private_key is None
+
+        if self.no_op:
+            warning = ("{motive} therefore no signing will be performed"
+                       " by StaticMapURLSigner")
+            if self.client_id is None:
+                motive = ("`public_key`, `client_id` and `private_key`"
+                          " are all None")
+            else:
+                motive = "`client_id` was provided but `private_key` is None"
+
+            logging.warning(warning.format(motive=motive))
 
     def sign_url(self, input_url):
         # type: (str) -> str
@@ -85,26 +93,40 @@ def sign_url(self, input_url):
         if not input_url:
             raise ValueError("`input_url` cannot be None")
 
-        scheme, netloc, path, _, query, _ = urlparse.urlparse(input_url)
-
-        if self.verify_endpoint:
-            if scheme != self.staticmap_api_endpoint.scheme:
-                logging.warning(
-                    "URL scheme `%s` remapped to `%s`", scheme,
-                    self.staticmap_api_endpoint.scheme)
-                scheme = self.staticmap_api_endpoint.scheme
-            if netloc != self.staticmap_api_endpoint.netloc:
-                logging.warning(
-                    "URL netloc `%s` remapped to `%s`", netloc,
-                    self.staticmap_api_endpoint.netloc)
-                netloc = self.staticmap_api_endpoint.netloc
-            if path != self.staticmap_api_endpoint.path:
-                logging.warning(
-                    "URL path `%s` remapped to `%s`", path,
-                    self.staticmap_api_endpoint.path)
-                path = self.staticmap_api_endpoint.path
-
-        if self.client_id is not None:
+        parsed_url = (self._get_valid_endpoint(*urlparse.urlparse(input_url))
+                      if self.verify_endpoint
+                      else urlparse.urlparse(input_url))
+
+        if not self.no_op:
+            parsed_url = self._sign(*parsed_url)
+
+        scheme, netloc, path, _, query, _ = parsed_url
+
+        # Return signed URL
+        return self.url_model.format(
+            scheme=scheme, netloc=netloc, path=path, query_string=query)
+
+    def _get_valid_endpoint(self, scheme, netloc, path,
+                            params, query, fragment):
+        if scheme != self.staticmap_api_endpoint.scheme:
+            logging.warning(
+                "URL scheme `%s` remapped to `%s`", scheme,
+                self.staticmap_api_endpoint.scheme)
+            scheme = self.staticmap_api_endpoint.scheme
+        if netloc != self.staticmap_api_endpoint.netloc:
+            logging.warning(
+                "URL netloc `%s` remapped to `%s`", netloc,
+                self.staticmap_api_endpoint.netloc)
+            netloc = self.staticmap_api_endpoint.netloc
+        if path != self.staticmap_api_endpoint.path:
+            logging.warning(
+                "URL path `%s` remapped to `%s`", path,
+                self.staticmap_api_endpoint.path)
+            path = self.staticmap_api_endpoint.path
+        return scheme, netloc, path, params, query, fragment
+
+    def _sign(self, scheme, netloc, path, params, query, fragment):
+        if self.client_id is not None and self.private_key is not None:
             query_string = "client_id={client_id}&{query_params}".format(
                 client_id=self.client_id, query_params=query)
         elif self.public_key is not None:
@@ -113,31 +135,23 @@ def sign_url(self, input_url):
         else:
             query_string = "{query_params}".format(query_params=query)
 
-        url_model = "{scheme}://{netloc}{path}?{query_string}"
-
-        if not self.private_key:
-            return url_model.format(
-                scheme=scheme, netloc=netloc, path=path,
-                query_string=query_string)
+        if self.private_key:
+            # We only need to sign the path+query part of the string
+            url_to_sign = path + "?" + query_string
 
-        # We only need to sign the path+query part of the string
-        url_to_sign = path + "?" + query_string
+            # Decode the private key into its binary format
+            # We need to decode the URL-encoded private key
+            decoded_key = base64.urlsafe_b64decode(self.private_key)
 
-        # Decode the private key into its binary format
-        # We need to decode the URL-encoded private key
-        decoded_key = base64.urlsafe_b64decode(self.private_key)
+            # Create a signature using the private key and the URL-encoded
+            # string using HMAC SHA1. This signature will be binary.
+            signature = hmac.new(
+                decoded_key, str.encode(url_to_sign), hashlib.sha1)
 
-        # Create a signature using the private key and the URL-encoded
-        # string using HMAC SHA1. This signature will be binary.
-        signature = hmac.new(
-            decoded_key, str.encode(url_to_sign), hashlib.sha1)
+            # Encode the binary signature into base64 for use within a URL
+            encoded_signature = base64.urlsafe_b64encode(signature.digest())
 
-        # Encode the binary signature into base64 for use within a URL
-        encoded_signature = base64.urlsafe_b64encode(signature.digest())
+            query_string += "&signature={signature}".format(
+                signature=encoded_signature.decode())
 
-        query_string += "&signature={signature}".format(
-            signature=encoded_signature.decode())
-
-        # Return signed URL
-        return url_model.format(
-            scheme=scheme, netloc=netloc, path=path, query_string=query_string)
+        return scheme, netloc, path, params, query_string, fragment

tests/test_signature.py

@@ -1,36 +1,35 @@
+import pytest
+
 try:
     import urlparse
 except ImportError:
     import urllib.parse as urlparse
 
-import pytest
-
 from staticmaps_signature import StaticMapURLSigner
 
 CLIENT_ID = "Zy4aSIA1Q7KXFsGy4ulx1qS0-PQXefghOBcPH2E"
 PUBLIC_KEY = "Zy4aSIA1Q7KXFsGy4ulx1qS0-PQXefghOBcPH2E"
 PRIVATE_KEY = "cwAPISuAyZSrGwXG-qzjMLPPvRE="
 
 
-class TestStaticMapURLSigner(object):
-    def test_init(self):
-        with pytest.raises(ValueError):
-            StaticMapURLSigner()
-
-        with pytest.raises(ValueError):
-            StaticMapURLSigner(
-                client_id=CLIENT_ID, public_key=PUBLIC_KEY,
-                private_key=PRIVATE_KEY)
+@pytest.fixture('function')
+def logging_stub(mocker):
+    return mocker.patch("staticmaps_signature.signature.logging")
 
-        with pytest.raises(ValueError):
-            StaticMapURLSigner(client_id=CLIENT_ID)
 
-        StaticMapURLSigner(client_id=CLIENT_ID, private_key=PRIVATE_KEY)
-        StaticMapURLSigner(public_key=PUBLIC_KEY, private_key=PRIVATE_KEY)
+@pytest.mark.usefixtures('logging_stub')
+class TestStaticMapURLSigner(object):
+    def test_init(self):
+        StaticMapURLSigner()
+        StaticMapURLSigner(client_id=CLIENT_ID)
         StaticMapURLSigner(public_key=PUBLIC_KEY)
         StaticMapURLSigner(private_key=PRIVATE_KEY)
+        StaticMapURLSigner(client_id=CLIENT_ID, private_key=PRIVATE_KEY)
+        StaticMapURLSigner(public_key=PUBLIC_KEY, private_key=PRIVATE_KEY)
+        StaticMapURLSigner(client_id=CLIENT_ID, public_key=PUBLIC_KEY,
+                           private_key=PRIVATE_KEY)
 
-    def test_signature(self, mocker):
+    def test_signature(self):
         # given
         request_url = (
             "https://maps.googleapis.com/maps/api/staticmap"
@@ -52,7 +51,6 @@ def test_signature(self, mocker):
             "https://maps.googleapis.com/staticmap"
             "?center=-23.5509518,-46.6921805&markers=-23.5509518,-46.6921805"
             "&zoom=15&size=300x200&maptype=roadmap")
-        logging = mocker.patch("staticmaps_signature.signature.logging")
         client_id_signer = StaticMapURLSigner(
             client_id=CLIENT_ID, private_key=PRIVATE_KEY)
         public_key_signer = StaticMapURLSigner(
@@ -100,4 +98,3 @@ def test_signature(self, mocker):
                 == client_id_signer.staticmap_api_endpoint.path)
         assert (urlparse.urlparse(uncorrected_netloc).netloc
                 == urlparse.urlparse(bad_endpoint_netloc_url).netloc)
-        assert logging.warning.call_count == 3