WIP: New loading

pycaching/cache.py

@@ -5,6 +5,7 @@
 import re
 import enum
 import os
+from urllib.parse import urlparse, parse_qs
 from pycaching import errors
 from pycaching.geo import Point
 from pycaching.trackable import Trackable
@@ -108,7 +109,7 @@ class Cache(object):
         "print_page": "seek/cdpf.aspx",
     }
 
-    def __init__(self, geocaching, wp, **kwargs):
+    def __init__(self, geocaching, wp=None, **kwargs):
         """Create a cache instance.
 
         :param .Geocaching geocaching: Reference to :class:`.Geocaching` instance, used for loading
@@ -124,8 +125,8 @@ def __init__(self, geocaching, wp, **kwargs):
 
         known_kwargs = {"name", "type", "location", "original_location", "state", "found", "size",
                         "difficulty", "terrain", "author", "hidden", "attributes", "summary",
-                        "description", "hint", "favorites", "pm_only", "url", "waypoints", "_logbook_token",
-                        "_trackable_page_url", "guid"}
+                        "description", "hint", "favorites", "pm_only", "waypoints", "guid",
+                        "_logbook_token", "_trackable_page_url"}
 
         for name in known_kwargs:
             if name in kwargs:
@@ -180,12 +181,13 @@ def wp(self, wp):
         self._wp = wp
 
     @property
+    @lazy_loaded
     def guid(self):
         """The cache GUID. An identifier used at some places on geoaching.com
 
         :type: :class:`str`
         """
-        return getattr(self, "_guid", None)
+        return self._guid
 
     @guid.setter
     def guid(self, guid):
@@ -531,32 +533,47 @@ def _trackable_page_url(self, trackable_page_url):
         self.__trackable_page_url = trackable_page_url
 
     def load(self):
-        """Load all possible cache details.
+        """Load cache details.
+
+        .. note::
+           This method is called automatically when you access a property which isn't yet filled in
+           (so-called "lazy loading"). You don't have to call it explicitly.
 
-        Use full cache details page. Therefore all possible properties are filled in, but the
+        This method picks the loading method based on the available cache ID:
+
+          + If a GUID is known, it uses :meth:`.Cache.load_print`.
+          + Else if a WP is known, it uses :meth:`.Cache.load_normal`.
+          + Else, it throws a :class:`.LoadError`.
+
+        For details on different loading methods, please see their documentation. Feel free not to
+        call this method, but please use directly that one, which better suits your needs.
+
+        :raise .PMOnlyException: If cache is PM only and current user is basic member.
+        :raise .LoadError: If cache cannot be loaded.
+        """
+        if hasattr(self, "_guid"):
+            return self.load_print()
+        elif hasattr(self, "_wp"):
+            return self.load_normal()
+        else:
+            raise errors.LoadError("Cache lacks info for loading")
+
+    def load_normal(self):
+        """Load all cache details.
+
+        It uses a full cache details page. Therefore all possible properties are filled in, but the
         loading is a bit slow.
 
         If you want to load basic details about a PM only cache, the :class:`.PMOnlyException` is
-        still thrown, but avaliable details are filled in. If you know, that the cache you are
+        thrown, but all available details are filled in. If you know, that the cache you are
         loading is PM only, please consider using :meth:`load_quick` as it will load the same
-        details, but quicker.
-
-        .. note::
-           This method is called automatically when you access a property which isn't yet filled in
-           (so-called "lazy loading"). You don't have to call it explicitly.
+        details, but faster.
 
         :raise .PMOnlyException: If cache is PM only and current user is basic member.
         :raise .LoadError: If cache loading fails (probably because of not existing cache).
         """
         try:
-            # pick url based on what info we have right now
-            if hasattr(self, "url"):
-                root = self.geocaching._request(self.url)
-            elif hasattr(self, "_wp"):
-                root = self.geocaching._request(self._urls["cache_details"],
-                                                params={"wp": self._wp})
-            else:
-                raise errors.LoadError("Cache lacks info for loading")
+            root = self.geocaching._request(self._urls["cache_details"], params={"wp": self._wp})
         except errors.Error as e:
             # probably 404 during cache loading - cache not exists
             raise errors.LoadError("Error in loading cache") from e
@@ -566,7 +583,7 @@ def load(self):
 
         cache_details = root.find(id="ctl00_divContentMain") if self.pm_only else root.find(id="cacheDetails")
 
-        # details also avaliable for basic members for PM only caches -----------------------------
+        # details also available for basic members for PM only caches -----------------------------
 
         if self.pm_only:
             self.wp = cache_details.find("li", "li__gccode").text.strip()
@@ -586,6 +603,7 @@ def load(self):
             self.size = Size.from_string(details[8])
 
             self.favorites = int(details[11])
+
         else:
             # parse from <title> - get first word
             try:
@@ -614,6 +632,11 @@ def load(self):
             raise errors.PMOnlyException()
 
         # details not avaliable for basic members for PM only caches ------------------------------
+
+        parsed_url = urlparse(root.find("link", rel="canonical").get("href"))
+        query_string = parse_qs(parsed_url.query)
+        self.guid = query_string['guid'][0]
+
         pm_only_warning = root.find("p", "Warning NoBottomSpacing")
         self.pm_only = pm_only_warning and ("Premium Member Only" in pm_only_warning.text) or False
 
@@ -662,27 +685,41 @@ def load(self):
         else:
             self._trackable_page_url = None
 
-        # Additional Waypoints
+        # additional Waypoints
         self.waypoints = Waypoint.from_html(root, "ctl00_ContentBody_Waypoints")
 
-        logging.debug("Cache loaded: {}".format(self))
+        logging.debug("Cache loaded (normal): {}".format(self))
 
     def load_quick(self):
         """Load basic cache details.
 
-        Use information from geocaching map tooltips. Therefore loading is very quick, but
-        the only loaded properties are: `name`, `type`, `state`, `size`, `difficulty`, `terrain`,
-        `hidden`, `author`, `favorites` and `pm_only`.
+        Uses a data from geocaching map tooltips. Therefore loading is very quick, but the only
+        loaded properties are:
+
+          + `name`
+          + `type`
+          + `state`
+          + `size`
+          + `difficulty`
+          + `terrain`
+          + `hidden`
+          + `author`
+          + `favorites`
+          + `pm_only`
 
         :raise .LoadError: If cache loading fails (probably because of not existing cache).
         """
-        res = self.geocaching._request(self._urls["tiles_server"],
-                                       params={"i": self.wp},
-                                       expect="json")
+        try:
+            res = self.geocaching._request(self._urls["tiles_server"],
+                                           params={"i": self._wp},
+                                           expect="json")
 
-        if res["status"] == "failed" or len(res["data"]) != 1:
-            msg = res["msg"] if "msg" in res else "Unknown error (probably not existing cache)"
-            raise errors.LoadError("Cache {} cannot be loaded: {}".format(self, msg))
+            if res["status"] == "failed" or len(res["data"]) != 1:
+                msg = res["msg"] if "msg" in res else "Unknown error (probably not existing cache)"
+                raise errors.LoadError(msg)
+
+        except errors.Error as e:
+            raise errors.LoadError("Error in loading cache") from e
 
         data = res["data"][0]
 
@@ -699,35 +736,49 @@ def load_quick(self):
         self.pm_only = data["subrOnly"]
         self.guid = res["data"][0]["g"]
 
-        logging.debug("Cache loaded: {}".format(self))
+        logging.debug("Cache loaded (quick): {}".format(self))
+
+    def load_print(self):
+        """Load most of the cache details.
+
+        Uses a cache print page. This is significantly faster, but requires a GUID to be set. If
+        the GUID is missing, it calls :meth:`.Cache.load_quick` to get it by a WP, which is both
+        still faster than :meth:`.Cache.load_normal`!
 
-    def load_by_guid(self):
-        """Load cache details using the GUID to request and parse the caches
-        'print-page'. Loading as many properties as possible except the
-        following ones, since they are not present on the 'print-page':
+        However, not all properties are presented on the print page, so the following ones are not
+        loaded:
 
-          + original_location
-          + state
-          + found
-          + pm_only
+          + `original_location`
+          + `state`
+          + `found`
+          + `pm_only`
 
-        :raise .PMOnlyException: If the PM only warning is shown on the page
+        Also, in comparison to :meth:`.Cache.load_normal` – if the cache is PM-only, this method
+        doesn't load anything.
+
+        :raise .PMOnlyException: If the cache is PM only.
+        :raise .LoadError: If cache loading fails (probably because of not existing cache).
         """
-        # If GUID has not yet been set, load it using the "tiles_server"
-        # utilizing `load_quick()`
-        if not self.guid:
+        # if GUID has not yet been set, load it using the `load_quick()`
+        # the getattr() will prevent lazy-loading of GUID
+        if not getattr(self, "_guid", None):
             self.load_quick()
 
-        res = self.geocaching._request(self._urls["print_page"],
-                                       params={"guid": self.guid})
+        try:
+            res = self.geocaching._request(self._urls["print_page"], params={"guid": self._guid})
+        except errors.Error as e:
+            raise errors.LoadError("Error in loading cache") from e
+
         if res.find("p", "Warning") is not None:
             raise errors.PMOnlyException()
+
+        self.wp = res.find(id="Header").find_all("h1")[-1].text
+
         content = res.find(id="Content")
 
         self.name = content.find("h2").text
 
-        self.location = Point.from_string(
-            content.find("p", "LatLong Meta").text)
+        self.location = Point.from_string(content.find("p", "LatLong Meta").text)
 
         type_img = os.path.basename(content.find("img").get("src"))
         self.type = Type.from_filename(os.path.splitext(type_img)[0])
@@ -746,7 +797,8 @@ def load_by_guid(self):
         hidden_p = content.find("p", text=re.compile("Placed Date:"))
         self.hidden = hidden_p.text.replace("Placed Date:", "").strip()
 
-        attr_img = content.find_all("img", src=re.compile("\/attributes\/"))
+        attributes_widget = content.find("div", "sortables")
+        attr_img = attributes_widget.find_all("img", src=re.compile("\/attributes\/"))
         attributes_raw = [
             os.path.basename(_.get("src")).rsplit("-", 1) for _ in attr_img
         ]
@@ -755,19 +807,19 @@ def load_by_guid(self):
             in attributes_raw if not appendix.startswith("blank")
         }
 
-        self.summary = content.find(
-            "h2", text="Short Description").find_next("div").text
+        self.summary = content.find("h2", text="Short Description").find_next("div").text
 
-        self.description = content.find(
-            "h2", text="Long Description").find_next("div").text
+        self.description = content.find("h2", text="Long Description").find_next("div").text
 
         self.hint = content.find(id="uxEncryptedHint").text
 
-        self.favorites = content.find(
-            "strong", text=re.compile("Favorites:")).parent.text.split()[-1]
+        favorites = content.find("strong", text=re.compile("Favorites:"))
+        self.favorites = 0 if favorites is None else int(favorites.parent.text.split()[-1])
 
         self.waypoints = Waypoint.from_html(content, "Waypoints")
 
+        logging.debug("Cache loaded (print): {}".format(self))
+
     def _logbook_get_page(self, page=0, per_page=25):
         """Load one page from logbook.
 

test/test_cache.py

@@ -171,6 +171,7 @@ def test_log_page_url(self):
 
 
 class TestMethods(unittest.TestCase):
+    _multiprocess_shared_ = True
 
     @classmethod
     def setUpClass(cls):
@@ -179,13 +180,31 @@ def setUpClass(cls):
         cls.c = Cache(cls.gc, "GC1PAR2")
         cls.c.load()
 
-    def test_load(self):
+    @mock.patch("pycaching.Cache.load_normal")
+    @mock.patch("pycaching.Cache.load_print")
+    def test_load(self, mock_load_print, mock_load_normal):
+        with self.subTest("use load_print for GUID"):
+            cache = Cache(self.gc, guid="5f45114d-1d79-4fdb-93ae-8f49f1d27188")
+            cache.load()
+            self.assertTrue(mock_load_print.called)
+
+        with self.subTest("use load_normal for WP"):
+            cache = Cache(self.gc, wp="GC4808G")
+            cache.load()
+            self.assertTrue(mock_load_normal.called)
+
+        with self.subTest("fail for no loading info"):
+            with self.assertRaises(LoadError):
+                cache = Cache(self.gc)
+                cache.load()
+
+    def test_load_normal(self):
         with self.subTest("normal (with explicit call of load())"):
             cache = Cache(self.gc, "GC4808G")
-            cache.load()
+            cache.load_normal()
             self.assertEqual("Nekonecne ticho", cache.name)
 
-        with self.subTest("normal"):
+        with self.subTest("normal (lazy-loaded)"):
             cache = Cache(self.gc, "GC4808G")
             self.assertEqual("Nekonecne ticho", cache.name)
 
@@ -196,12 +215,12 @@ def test_load(self):
         with self.subTest("PM only"):
             with self.assertRaises(PMOnlyException):
                 cache = Cache(self.gc, "GC3AHDM")
-                cache.load()
+                cache.load_normal()
 
         with self.subTest("fail"):
             with self.assertRaises(LoadError):
                 cache = Cache(self.gc, "GC123456")
-                cache.load()
+                cache.load_normal()
 
     def test_load_quick(self):
         with self.subTest("normal"):
@@ -216,12 +235,11 @@ def test_load_quick(self):
                 cache = Cache(self.gc, "GC123456")
                 cache.load_quick()
 
-    @mock.patch("pycaching.Cache.load")
-    @mock.patch("pycaching.Cache.load_quick")
-    def test_load_by_guid(self, mock_load_quick, mock_load):
+    @mock.patch.object(Cache, "load_quick")
+    def test_load_print(self, mock_load_quick):
         with self.subTest("normal"):
             cache = Cache(self.gc, "GC2WXPN", guid="5f45114d-1d79-4fdb-93ae-8f49f1d27188")
-            cache.load_by_guid()
+            cache.load_print()
             self.assertEqual(cache.name, "Der Schatz vom Luftschloss")
             self.assertEqual(cache.location, Point("N 49° 57.895' E 008° 12.988'"))
             self.assertEqual(cache.type, Type.mystery)
@@ -248,12 +266,12 @@ def test_load_by_guid(self, mock_load_quick, mock_load):
         with self.subTest("PM-only"):
             cache = Cache(self.gc, "GC6MKEF", guid="53d34c4d-12b5-4771-86d3-89318f71efb1")
             with self.assertRaises(PMOnlyException):
-                cache.load_by_guid()
+                cache.load_print()
 
         with self.subTest("calls load_quick if no guid"):
             cache = Cache(self.gc, "GC2WXPN")
             with self.assertRaises(Exception):
-                cache.load_by_guid()  # Raises error since we mocked load_quick()
+                cache.load_print()  # raises error since we mocked load_quick()
             self.assertTrue(mock_load_quick.called)
 
     def test_load_trackables(self):

test/test_geo.py

@@ -179,6 +179,7 @@ def test_diagonal(self):
 
 
 class TestTile(unittest.TestCase):
+    _multiprocess_shared_ = True
 
     # see
     # http://gis.stackexchange.com/questions/8650/how-to-measure-the-accuracy-of-latitude-and-longitude