Merge pull request burnash#427 from burnash/fix/open_by_url

fix regex for burnash#349, refactor id extraction; add tests

gspread/client.py

@@ -8,7 +8,6 @@
 Google Data API.
 
 """
-import re
 import json
 
 try:
@@ -22,17 +21,13 @@
 from .httpsession import HTTPSession, HTTPError
 from .models import Spreadsheet
 from .urls import construct_url
-from .utils import finditem
-from .exceptions import (SpreadsheetNotFound, NoValidUrlKeyFound,
-                         UpdateCellError, RequestError)
+from .utils import finditem, extract_id_from_url
+from .exceptions import (SpreadsheetNotFound, UpdateCellError, RequestError)
 
 
 AUTH_SERVER = 'https://www.google.com'
 SPREADSHEETS_SERVER = 'spreadsheets.google.com'
 
-_url_key_re_v1 = re.compile(r'key=([^&#]+)')
-_url_key_re_v2 = re.compile(r'spreadsheets/d/([^&#]+)/edit')
-
 
 class Client(object):
 
@@ -111,14 +106,9 @@ def open_by_key(self, key):
         for elem in feed.findall(_ns('entry')):
             alter_link = finditem(lambda x: x.get('rel') == 'alternate',
                                   elem.findall(_ns('link')))
-            m = _url_key_re_v1.search(alter_link.get('href'))
-            if m and m.group(1) == key:
-                return Spreadsheet(self, elem)
-
-            m = _url_key_re_v2.search(alter_link.get('href'))
-            if m and m.group(1) == key:
+            spreadsheet_id = extract_id_from_url(alter_link.get('href'))
+            if spreadsheet_id == key:
                 return Spreadsheet(self, elem)
-
         else:
             raise SpreadsheetNotFound
 
@@ -136,17 +126,7 @@ def open_by_url(self, url):
         >>> c.open_by_url('https://docs.google.com/spreadsheet/ccc?key=0Bm...FE&hl')
 
         """
-        m1 = _url_key_re_v1.search(url)
-        if m1:
-            return self.open_by_key(m1.group(1))
-
-        else:
-            m2 = _url_key_re_v2.search(url)
-            if m2:
-                return self.open_by_key(m2.group(1))
-
-            else:
-                raise NoValidUrlKeyFound
+        return self.open_by_key(extract_id_from_url(url))
 
     def openall(self, title=None):
         """Opens all available spreadsheets.

gspread/utils.py

@@ -11,11 +11,14 @@
 import re
 from xml.etree import ElementTree
 
-from .exceptions import IncorrectCellLabel
+from .exceptions import IncorrectCellLabel, NoValidUrlKeyFound
 
 MAGIC_NUMBER = 64
 CELL_ADDR_RE = re.compile(r'([A-Za-z]+)([1-9]\d*)')
 
+URL_KEY_V1_RE = re.compile(r'key=([^&#]+)')
+URL_KEY_V2_RE = re.compile(r'/spreadsheets/d/([a-zA-Z0-9-_]+)')
+
 
 def finditem(func, seq):
     """Finds and returns first item in iterable for which func(item) is True.
@@ -164,7 +167,17 @@ def a1_to_rowcol(label):
     else:
         raise IncorrectCellLabel(label)
 
-    return (row, col)
+
+def extract_id_from_url(url):
+    m2 = URL_KEY_V2_RE.search(url)
+    if m2:
+        return m2.group(1)
+
+    m1 = URL_KEY_V1_RE.search(url)
+    if m1:
+        return m1.group(1)
+
+    raise NoValidUrlKeyFound
 
 
 if __name__ == '__main__':

tests/test.py

@@ -13,6 +13,7 @@
 from oauth2client.service_account import ServiceAccountCredentials
 
 import gspread
+from gspread import utils
 
 try:
     unicode
@@ -47,6 +48,39 @@ def gen_value(prefix=None):
         return unicode(uuid.uuid4())
 
 
+class UtilsTest(unittest.TestCase):
+    def test_extract_id_from_url(self):
+        url_id_list = [
+            # New-style url
+            ('https://docs.google.com/spreadsheets/d/'
+             '1qpyC0X3A0MwQoFDE8p-Bll4hps/edit#gid=0',
+             '1qpyC0X3A0MwQoFDE8p-Bll4hps'),
+
+            ('https://docs.google.com/spreadsheets/d/'
+             '1qpyC0X3A0MwQoFDE8p-Bll4hps/edit',
+             '1qpyC0X3A0MwQoFDE8p-Bll4hps'),
+
+            ('https://docs.google.com/spreadsheets/d/'
+             '1qpyC0X3A0MwQoFDE8p-Bll4hps',
+             '1qpyC0X3A0MwQoFDE8p-Bll4hps'),
+
+            # Old-style url
+            ('https://docs.google.com/spreadsheet/'
+             'ccc?key=1qpyC0X3A0MwQoFDE8p-Bll4hps&usp=drive_web#gid=0',
+             '1qpyC0X3A0MwQoFDE8p-Bll4hps')
+        ]
+
+        for url, id in url_id_list:
+            self.assertEqual(id, utils.extract_id_from_url(url))
+
+    def test_no_extract_id_from_url(self):
+        self.assertRaises(
+            gspread.NoValidUrlKeyFound,
+            utils.extract_id_from_url,
+            'http://example.org'
+        )
+
+
 class GspreadTest(unittest.TestCase):
 
     @classmethod