From f80623e025796a5950589dcd28504e99f93add2d Mon Sep 17 00:00:00 2001
From: Athanasius <github@miggy.org>
Date: Mon, 23 Aug 2021 16:28:26 +0100
Subject: [PATCH] CAPI: Convert full Update flow to class passing

* Base the following on common EDMCCAPIReturn: EDMCFailedrequest,
  EDMCCAPIRequest, EDMCCAPIResponse.  This saves repeating a bunch of
  variable types and comments.
* Use the above throughout the 'Update' button flow.
* Still need to address 'Save Raw Data', i.e. AppWindow.save_raw().
---
 EDMarketConnector.py | 133 ++++++++++++++++++++++++-------------------
 companion.py         | 118 ++++++++++++++++++++++++++------------
 2 files changed, 157 insertions(+), 94 deletions(-)

diff --git a/EDMarketConnector.py b/EDMarketConnector.py
index fcf56211..170a9a86 100755
--- a/EDMarketConnector.py
+++ b/EDMarketConnector.py
@@ -935,77 +935,86 @@ class AppWindow(object):
 
         querytime = int(time())
         logger.trace_if('capi.worker', 'Requesting full station data')
-        companion.session.station(querytime=querytime, play_sound=play_sound)
+        companion.session.station(
+            querytime=querytime, retrying=retrying, play_sound=play_sound
+        )
         config.set('querytime', querytime)
 
-
-    def capi_handle_response(self, event=None):
+    def capi_handle_response(self, event=None):  # noqa: C901, CCR001
         """Handle the resulting data from a CAPI query."""
-        play_bad = False
+        play_bad: bool = False
         err: Optional[str] = None
 
-        data: Union[companion.CAPIData, companion.CAPIFailedRequest]
-        querytime: int
-        play_sound: bool
-        auto_update: bool
+        capi_response: Union[companion.CAPIFailedRequest, companion.EDMCCAPIResponse]
         try:
             logger.trace_if('capi.worker', 'Pulling answer off queue')
-            data, querytime, play_sound, auto_update = self.capi_response_queue.get(block=False)
-            if isinstance(data, companion.CAPIFailedRequest):
-                logger.trace_if('capi.worker', f'Failed Request: {data.message}')
-                if data.exception:
-                    raise data.exception
+            capi_response = self.capi_response_queue.get(block=False)
+            if isinstance(capi_response, companion.CAPIFailedRequest):
+                logger.trace_if('capi.worker', f'Failed Request: {capi_response.message}')
+                if capi_response.exception:
+                    raise capi_response.exception
 
                 else:
-                    raise ValueError(data.message)
+                    raise ValueError(capi_response.message)
 
             logger.trace_if('capi.worker', 'Answer is not a Failure')
+            if not isinstance(capi_response, companion.EDMCCAPIResponse):
+                msg = f"Response was neither CAPIFailedRequest nor EDMCAPIResponse: {type(capi_response)}"
+                logger.error(msg)
+                raise ValueError(msg)
+
             # Validation
-            if 'commander' not in data:
+            if 'commander' not in capi_response.capi_data:
                 # This can happen with EGS Auth if no commander created yet
                 # LANG: No data was returned for the commander from the Frontier CAPI
                 err = self.status['text'] = _('CAPI: No commander data returned')
 
-            elif not data.get('commander', {}).get('name'):
+            elif not capi_response.capi_data.get('commander', {}).get('name'):
                 # LANG: We didn't have the commander name when we should have
                 err = self.status['text'] = _("Who are you?!")  # Shouldn't happen
 
-            elif (not data.get('lastSystem', {}).get('name')
-                  or (data['commander'].get('docked')
-                      and not data.get('lastStarport', {}).get('name'))):
+            elif (not capi_response.capi_data.get('lastSystem', {}).get('name')
+                  or (capi_response.capi_data['commander'].get('docked')
+                      and not capi_response.capi_data.get('lastStarport', {}).get('name'))):
                 # LANG: We don't know where the commander is, when we should
                 err = self.status['text'] = _("Where are you?!")  # Shouldn't happen
 
-            elif not data.get('ship', {}).get('name') or not data.get('ship', {}).get('modules'):
+            elif (
+                    not capi_response.capi_data.get('ship', {}).get('name')
+                    or not capi_response.capi_data.get('ship', {}).get('modules')
+            ):
                 # LANG: We don't know what ship the commander is in, when we should
                 err = self.status['text'] = _("What are you flying?!")  # Shouldn't happen
 
-            elif monitor.cmdr and data['commander']['name'] != monitor.cmdr:
+            elif monitor.cmdr and capi_response.capi_data['commander']['name'] != monitor.cmdr:
                 # Companion API Commander doesn't match Journal
                 logger.trace_if('capi.worker', 'Raising CmdrError()')
                 raise companion.CmdrError()
 
-            elif auto_update and not monitor.state['OnFoot'] and not data['commander'].get('docked'):
+            elif (
+                    capi_response.auto_update and not monitor.state['OnFoot']
+                    and not capi_response.capi_data['commander'].get('docked')
+            ):
                 # auto update is only when just docked
-                logger.warning(f"{auto_update!r} and not {monitor.state['OnFoot']!r} and "
-                               f"not {data['commander'].get('docked')!r}")
+                logger.warning(f"{capi_response.auto_update!r} and not {monitor.state['OnFoot']!r} and "
+                               f"not {capi_response.capi_data['commander'].get('docked')!r}")
                 raise companion.ServerLagging()
 
-            elif data['lastSystem']['name'] != monitor.system:
+            elif capi_response.capi_data['lastSystem']['name'] != monitor.system:
                 # CAPI system must match last journal one
-                logger.warning(f"{data['lastSystem']['name']!r} != {monitor.system!r}")
+                logger.warning(f"{capi_response.capi_data['lastSystem']['name']!r} != {monitor.system!r}")
                 raise companion.ServerLagging()
 
-            elif data['lastStarport']['name'] != monitor.station:
+            elif capi_response.capi_data['lastStarport']['name'] != monitor.station:
                 if monitor.state['OnFoot'] and monitor.station:
-                    logger.warning(f"({data['lastStarport']['name']!r} != {monitor.station!r}) AND "
+                    logger.warning(f"({capi_response.capi_data['lastStarport']['name']!r} != {monitor.station!r}) AND "
                                    f"{monitor.state['OnFoot']!r} and {monitor.station!r}")
                     raise companion.ServerLagging()
 
                 else:
                     last_station = None
-                    if data['commander']['docked']:
-                        last_station = data['lastStarport']['name']
+                    if capi_response.capi_data['commander']['docked']:
+                        last_station = capi_response.capi_data['lastStarport']['name']
 
                     if monitor.station is None:
                         # Likely (re-)Embarked on ship docked at an EDO settlement.
@@ -1017,33 +1026,40 @@ class AppWindow(object):
 
                     if last_station != monitor.station:
                         # CAPI lastStarport must match
-                        logger.warning(f"({data['lastStarport']['name']!r} != {monitor.station!r}) AND "
-                                       f"{last_station!r} != {monitor.station!r}")
+                        logger.warning(f"({capi_response.capi_data['lastStarport']['name']!r} != {monitor.station!r})"
+                                       f" AND {last_station!r} != {monitor.station!r}")
                         raise companion.ServerLagging()
 
-                self.capi_query_holdoff_time = querytime + companion.capi_query_cooldown
+                self.capi_query_holdoff_time = capi_response.querytime + companion.capi_query_cooldown
 
-            elif not monitor.state['OnFoot'] and data['ship']['id'] != monitor.state['ShipID']:
+            elif not monitor.state['OnFoot'] and capi_response.capi_data['ship']['id'] != monitor.state['ShipID']:
                 # CAPI ship must match
                 logger.warning(f"not {monitor.state['OnFoot']!r} and "
-                               f"{data['ship']['id']!r} != {monitor.state['ShipID']!r}")
+                               f"{capi_response.capi_data['ship']['id']!r} != {monitor.state['ShipID']!r}")
                 raise companion.ServerLagging()
 
-            elif not monitor.state['OnFoot'] and data['ship']['name'].lower() != monitor.state['ShipType']:
+            elif (
+                    not monitor.state['OnFoot']
+                    and capi_response.capi_data['ship']['name'].lower() != monitor.state['ShipType']
+            ):
                 # CAPI ship type must match
                 logger.warning(f"not {monitor.state['OnFoot']!r} and "
-                               f"{data['ship']['name'].lower()!r} != {monitor.state['ShipType']!r}")
+                               f"{capi_response.capi_data['ship']['name'].lower()!r} != "
+                               f"{monitor.state['ShipType']!r}")
                 raise companion.ServerLagging()
 
             else:
                 # TODO: Change to depend on its own CL arg
                 if __debug__:  # Recording
-                    companion.session.dump_capi_data(data)
+                    companion.session.dump_capi_data(capi_response.capi_data)
 
                 if not monitor.state['ShipType']:  # Started game in SRV or fighter
-                    self.ship['text'] = ship_name_map.get(data['ship']['name'].lower(), data['ship']['name'])
-                    monitor.state['ShipID'] = data['ship']['id']
-                    monitor.state['ShipType'] = data['ship']['name'].lower()
+                    self.ship['text'] = ship_name_map.get(
+                        capi_response.capi_data['ship']['name'].lower(),
+                        capi_response.capi_data['ship']['name']
+                    )
+                    monitor.state['ShipID'] = capi_response.capi_data['ship']['id']
+                    monitor.state['ShipType'] = capi_response.capi_data['ship']['name'].lower()
 
                     if not monitor.state['Modules']:
                         self.ship.configure(state=tk.DISABLED)
@@ -1053,43 +1069,42 @@ class AppWindow(object):
                     self.ship.configure(state=True)
 
                 if monitor.state.get('SuitCurrent') is not None:
-                    if (loadout := data.get('loadout')) is not None:
+                    if (loadout := capi_response.capi_data.get('loadout')) is not None:
                         if (suit := loadout.get('suit')) is not None:
                             if (suitname := suit.get('edmcName')) is not None:
                                 # We've been paranoid about loadout->suit->suitname, now just assume loadouts is there
                                 loadout_name = index_possibly_sparse_list(
-                                    data['loadouts'], loadout['loadoutSlotId']
+                                    capi_response.capi_data['loadouts'], loadout['loadoutSlotId']
                                 )['name']
 
                                 self.suit['text'] = f'{suitname} ({loadout_name})'
 
                 self.suit_show_if_set()
 
-                if data['commander'].get('credits') is not None:
-                    monitor.state['Credits'] = data['commander']['credits']
-                    monitor.state['Loan'] = data['commander'].get('debt', 0)
+                if capi_response.capi_data['commander'].get('credits') is not None:
+                    monitor.state['Credits'] = capi_response.capi_data['commander']['credits']
+                    monitor.state['Loan'] = capi_response.capi_data['commander'].get('debt', 0)
 
                 # stuff we can do when not docked
-                err = plug.notify_newdata(data, monitor.is_beta)
+                err = plug.notify_newdata(capi_response.capi_data, monitor.is_beta)
                 self.status['text'] = err and err or ''
                 if err:
                     play_bad = True
 
                 # Export market data
-                if not self.export_market_data(data):
+                if not self.export_market_data(capi_response.capi_data):
                     err = 'Error: Exporting Market data'
                     play_bad = True
 
-                self.capi_query_holdoff_time = querytime + companion.capi_query_cooldown
+                self.capi_query_holdoff_time = capi_response.querytime + companion.capi_query_cooldown
 
         except queue.Empty:
-                logger.error('There was no response in the queue!')
-                # TODO: Set status text
-                return
+            logger.error('There was no response in the queue!')
+            # TODO: Set status text
+            return
 
         except companion.CredentialsError:
             # Redirected back to Auth server - force full re-authentication
-            companion.session.dump(r)
             companion.session.invalidate()
             companion.session.retrying = False
             companion.session.login()
@@ -1097,7 +1112,7 @@ class AppWindow(object):
         # Companion API problem
         except companion.ServerLagging as e:
             err = str(e)
-            if retrying:
+            if capi_response.retrying:
                 self.status['text'] = err
                 play_bad = True
 
@@ -1124,13 +1139,13 @@ class AppWindow(object):
 
         if not err:  # not self.status['text']:  # no errors
             # LANG: Time when we last obtained Frontier CAPI data
-            self.status['text'] = strftime(_('Last updated at %H:%M:%S'), localtime(querytime))
+            self.status['text'] = strftime(_('Last updated at %H:%M:%S'), localtime(capi_response.querytime))
 
-        if play_sound and play_bad:
+        if capi_response.play_sound and play_bad:
             hotkeymgr.play_bad()
 
         # Update Odyssey Suit data
-        companion.session.suit_update(data)
+        companion.session.suit_update(capi_response.capi_data)
 
         self.update_suit_text()
         self.suit_show_if_set()
@@ -1413,7 +1428,9 @@ class AppWindow(object):
         if time() < self.capi_query_holdoff_time:
             # Update button in main window
             self.button['text'] = self.theme_button['text'] \
-                = _('cooldown {SS}s').format(SS=int(self.capi_query_holdoff_time - time()))  # LANG: Cooldown on 'Update' button
+                = _('cooldown {SS}s').format(  # LANG: Cooldown on 'Update' button
+                    SS=int(self.capi_query_holdoff_time - time())
+            )
             self.w.after(1000, self.cooldown)
 
         else:
diff --git a/companion.py b/companion.py
index a689e957..04f5bd74 100644
--- a/companion.py
+++ b/companion.py
@@ -473,33 +473,52 @@ class Auth(object):
         return base64.urlsafe_b64encode(text).decode().replace('=', '')
 
 
-class EDMCCAPIRequest():
+class EDMCCAPIReturn:
+    """Base class for Request, Failure or Response."""
+
+    def __init__(
+        self, querytime: int, retrying: bool = False,
+        play_sound: bool = False, auto_update: bool = False
+    ):
+        self.querytime: int = querytime  # When this query is considered to have started (time_t).
+        self.retrying: bool = retrying  # Whether this is already a retry.
+        self.play_sound: bool = play_sound  # Whether to play good/bad sounds for success/failure.
+        self.auto_update: bool = auto_update  # Whether this was automatically triggered.
+
+
+class EDMCCAPIRequest(EDMCCAPIReturn):
     """Encapsulates a request for CAPI data."""
 
-    endpoint: str  # The CAPI query to perform.
-    querytime: int  # When this query is considered to have started (time_t).
-    play_sound: bool  # Whether to play good/bad sounds for success/failure.
-    auto_update: bool  # Whether this was automatically triggered.
-
-    def __init__(self, endpoint: str, querytime: int, play_sound: bool = False, auto_update: bool = False):
-        self.endpoint = endpoint
-        self.querytime = querytime
-        self.play_sound = play_sound
-        self.auto_update = auto_update
+    def __init__(
+        self, endpoint: str,
+        querytime: int, retrying: bool = False, play_sound: bool = False, auto_update: bool = False
+    ):
+        super().__init__(querytime=querytime, retrying=retrying, play_sound=play_sound, auto_update=auto_update)
+        self.endpoint: str = endpoint  # The CAPI query to perform.
 
 
-class EDMCCAPIResponse():
+class EDMCCAPIResponse(EDMCCAPIReturn):
     """Encapsulates a response from CAPI quer(y|ies)."""
 
-    ...
+    def __init__(
+            self, capi_data: CAPIData,
+            querytime: int, retrying: bool = False, play_sound: bool = False, auto_update: bool = False
+    ):
+        super().__init__(querytime=querytime, retrying=retrying, play_sound=play_sound, auto_update=auto_update)
+        self.capi_data: CAPIData = capi_data  # Frontier CAPI response, possibly augmented (station query)
 
 
-class CAPIFailedRequest():
+class CAPIFailedRequest(EDMCCAPIReturn):
     """CAPI failed query error class."""
 
-    def __init__(self, message, exception=None):
-        self.message = message
-        self.exception = exception
+    def __init__(
+            self, message: str,
+            querytime: int, retrying: bool = False, play_sound: bool = False, auto_update: bool = False,
+            exception=None
+    ):
+        super().__init__(querytime=querytime, retrying=retrying, play_sound=play_sound, auto_update=auto_update)
+        self.message: str = message  # User-friendly reason for failure.
+        self.exception: int = exception  # Exception that recipient should raise.
 
 
 class Session(object):
@@ -701,6 +720,7 @@ class Session(object):
 
             if r.url.startswith(FRONTIER_AUTH_SERVER):
                 logger.info('Redirected back to Auth Server')
+                self.dump(r)
                 raise CredentialsError('Redirected back to Auth Server')
 
             elif 500 <= r.status_code < 600:
@@ -814,49 +834,59 @@ class Session(object):
                     break
 
             logger.trace_if('capi.worker', f'Processing query: {query.endpoint}')
-            data: CAPIData
+            capi_data: CAPIData
             if query.endpoint == self._CAPI_PATH_STATION:
                 try:
-                    data = capi_station_queries()
+                    capi_data = capi_station_queries()
 
                 except Exception as e:
                     self.capi_response_queue.put(
                         (
                             CAPIFailedRequest(
                                 message=e.args,
-                                exception=e
+                                exception=e,
+                                querytime=query.querytime,
+                                play_sound=query.play_sound,
+                                auto_update=query.auto_update
                             ),
-                            query.querytime,
-                            query.play_sound,
-                            query.auto_update
                         )
                     )
 
                 else:
                     self.capi_response_queue.put(
-                        (data, query.querytime, query.play_sound, query.auto_update)
+                        EDMCCAPIResponse(
+                            capi_data=capi_data,
+                            querytime=query.querytime,
+                            play_sound=query.play_sound,
+                            auto_update=query.auto_update
+                        )
                     )
 
             else:
                 try:
-                    data = capi_single_query(self.FRONTIER_CAPI_PATH_PROFILE)
+                    capi_data = capi_single_query(self.FRONTIER_CAPI_PATH_PROFILE)
 
                 except Exception as e:
                     self.capi_response_queue.put(
                         (
                             CAPIFailedRequest(
                                 message=e.args,
-                                exception=e
+                                exception=e,
+                                querytime=query.querytime,
+                                play_sound=query.play_sound,
+                                auto_update=query.auto_update
                             ),
-                            query.querytime,
-                            query.play_sound,
-                            query.auto_update
                         )
                     )
 
                 else:
                     self.capi_response_queue.put(
-                        (data, query.querytime, query.play_sound, query.auto_update)
+                        EDMCCAPIResponse(
+                            capi_data=capi_data,
+                            querytime=query.querytime,
+                            play_sound=query.play_sound,
+                            auto_update=query.auto_update
+                        )
                     )
 
             self.tk_master.event_generate('<<CAPIResponse>>')
@@ -867,15 +897,17 @@ class Session(object):
         """Ask the CAPI query thread to finish."""
         self.capi_query_queue.put(None)
 
-    def query(self, endpoint: str, querytime: int, play_sound: bool = False, auto_update: bool = False) -> None:
+    def query(
+            self, endpoint: str,
+            querytime: int, retrying: bool = False, play_sound: bool = False, auto_update: bool = False
+    ) -> None:
         """
         Perform a query against the specified CAPI endpoint.
 
         :param querytime: When this query was initiated.
+        :param retrying: Whether this is a retry.
         :param play_sound: Whether the app should play a sound on error.
-        :param endpoint: The CAPI endpoint to query, might be a pseudo-value.
         :param auto_update: Whether this request was triggered automatically.
-        :return:
         """
         logger.trace_if('capi.query', f'Performing query for endpoint "{endpoint}"')
         if self.state == Session.STATE_INIT:
@@ -894,27 +926,40 @@ class Session(object):
         self.capi_query_queue.put(
             EDMCCAPIRequest(
                 endpoint=endpoint,
+                retrying=retrying,
                 querytime=querytime,
                 play_sound=play_sound,
                 auto_update=auto_update
             )
         )
 
-    def profile(self, querytime: int = int(time.time()), play_sound: bool = False, auto_update: bool = False) -> None:
+    def profile(
+            self,
+            querytime: int = int(time.time()), retrying: bool = False,
+            play_sound: bool = False, auto_update: bool = False
+    ) -> None:
         """
         Perform general CAPI /profile endpoint query.
 
         :param querytime: When this query was initiated.
+        :param retrying: Whether this is a retry.
         :param play_sound: Whether the app should play a sound on error.
         :param auto_update: Whether this request was triggered automatically.
         """
-        self.query(self.FRONTIER_CAPI_PATH_PROFILE, querytime=querytime, play_sound=play_sound, auto_update=auto_update)
+        self.query(
+            self.FRONTIER_CAPI_PATH_PROFILE, querytime=querytime, retrying=retrying,
+            play_sound=play_sound, auto_update=auto_update
+        )
 
-    def station(self, querytime: int, play_sound: bool = False, auto_update: bool = False) -> None:
+    def station(
+            self,
+            querytime: int, retrying: bool = False, play_sound: bool = False, auto_update: bool = False
+    ) -> None:
         """
         Perform CAPI quer(y|ies) for station data.
 
         :param querytime: When this query was initiated.
+        :param retrying: Whether this is a retry.
         :param play_sound: Whether the app should play a sound on error.
         :param auto_update: Whether this request was triggered automatically.
         """
@@ -923,6 +968,7 @@ class Session(object):
             EDMCCAPIRequest(
                 endpoint=self._CAPI_PATH_STATION,
                 querytime=querytime,
+                retrying=retrying,
                 play_sound=play_sound,
                 auto_update=auto_update
             )