From c12c739c11af5e55750b4e9dd7cc438a350e4312 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 15 Sep 2020 14:15:07 +0100 Subject: [PATCH 1/7] companion.py: Full pass to 100% pass flake8 * docstrings added throughout. * .format() all now f-strings. * Type annotations added throughout. * White space tweaked. --- companion.py | 326 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 192 insertions(+), 134 deletions(-) diff --git a/companion.py b/companion.py index 947ec170..b6396827 100644 --- a/companion.py +++ b/companion.py @@ -1,31 +1,38 @@ -from builtins import str -from builtins import range -from builtins import object +""" +Handle use of Frontier's Companion API (CAPI) service. + +Deals with initiating authentication for, and use of, CAPI. +Some associated code is in protocol.py which creates and handles the edmc:// +protocol used for the callback. +""" + import base64 import csv -import requests -from typing import TYPE_CHECKING - -# TODO: see https://github.com/EDCD/EDMarketConnector/issues/569 -from http.cookiejar import LWPCookieJar # noqa: F401 - No longer needed but retained in case plugins use it -from email.utils import parsedate import hashlib +import logging import numbers import os -from os.path import join import random import time import urllib.parse import webbrowser +from builtins import object, range, str +from email.utils import parsedate +# TODO: see https://github.com/EDCD/EDMarketConnector/issues/569 +from http.cookiejar import LWPCookieJar # noqa: F401 - No longer needed but retained in case plugins use it +from os.path import join +from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Union + +import requests from config import appname, appversion, config from protocol import protocolhandler -import logging + logger = logging.getLogger(appname) if TYPE_CHECKING: - _ = lambda x: x # noqa # to make flake8 stop complaining that the hacked in _ method doesnt exist + _ = lambda x: x # noqa: E731 # to make flake8 stop complaining that the hacked in _ method doesnt exist holdoff = 60 # be nice @@ -35,83 +42,89 @@ auth_timeout = 30 # timeout for initial auth # Currently the "Elite Dangerous Market Connector (EDCD/Athanasius)" one in # Athanasius' Frontier account # Obtain from https://auth.frontierstore.net/client/signup -CLIENT_ID = os.getenv('CLIENT_ID') or 'fb88d428-9110-475f-a3d2-dc151c2b9c7a' +CLIENT_ID = os.getenv('CLIENT_ID') or 'fb88d428-9110-475f-a3d2-dc151c2b9c7a' SERVER_AUTH = 'https://auth.frontierstore.net' -URL_AUTH = '/auth' +URL_AUTH = '/auth' URL_TOKEN = '/token' -USER_AGENT = 'EDCD-{}-{}'.format(appname, appversion) +USER_AGENT = f'EDCD-{appname}-{appversion}' SERVER_LIVE = 'https://companion.orerve.net' SERVER_BETA = 'https://pts-companion.orerve.net' -URL_QUERY = '/profile' -URL_MARKET = '/market' -URL_SHIPYARD= '/shipyard' +URL_QUERY = '/profile' +URL_MARKET = '/market' +URL_SHIPYARD = '/shipyard' # Map values reported by the Companion interface to names displayed in-game # May be imported by plugins category_map = { - 'Narcotics' : 'Legal Drugs', - 'Slaves' : 'Slavery', - 'Waste ' : 'Waste', - 'NonMarketable' : False, # Don't appear in the in-game market so don't report + 'Narcotics': 'Legal Drugs', + 'Slaves': 'Slavery', + 'Waste ': 'Waste', + 'NonMarketable': False, # Don't appear in the in-game market so don't report } commodity_map = {} ship_map = { - 'adder' : 'Adder', - 'anaconda' : 'Anaconda', - 'asp' : 'Asp Explorer', - 'asp_scout' : 'Asp Scout', - 'belugaliner' : 'Beluga Liner', - 'cobramkiii' : 'Cobra MkIII', - 'cobramkiv' : 'Cobra MkIV', - 'clipper' : 'Panther Clipper', - 'cutter' : 'Imperial Cutter', - 'diamondback' : 'Diamondback Scout', - 'diamondbackxl' : 'Diamondback Explorer', - 'dolphin' : 'Dolphin', - 'eagle' : 'Eagle', - 'empire_courier' : 'Imperial Courier', - 'empire_eagle' : 'Imperial Eagle', - 'empire_fighter' : 'Imperial Fighter', - 'empire_trader' : 'Imperial Clipper', - 'federation_corvette' : 'Federal Corvette', - 'federation_dropship' : 'Federal Dropship', - 'federation_dropship_mkii' : 'Federal Assault Ship', - 'federation_gunship' : 'Federal Gunship', - 'federation_fighter' : 'F63 Condor', - 'ferdelance' : 'Fer-de-Lance', - 'hauler' : 'Hauler', - 'independant_trader' : 'Keelback', - 'independent_fighter' : 'Taipan Fighter', - 'krait_mkii' : 'Krait MkII', - 'krait_light' : 'Krait Phantom', - 'mamba' : 'Mamba', - 'orca' : 'Orca', - 'python' : 'Python', - 'scout' : 'Taipan Fighter', - 'sidewinder' : 'Sidewinder', - 'testbuggy' : 'Scarab', - 'type6' : 'Type-6 Transporter', - 'type7' : 'Type-7 Transporter', - 'type9' : 'Type-9 Heavy', - 'type9_military' : 'Type-10 Defender', - 'typex' : 'Alliance Chieftain', - 'typex_2' : 'Alliance Crusader', - 'typex_3' : 'Alliance Challenger', - 'viper' : 'Viper MkIII', - 'viper_mkiv' : 'Viper MkIV', - 'vulture' : 'Vulture', + 'adder': 'Adder', + 'anaconda': 'Anaconda', + 'asp': 'Asp Explorer', + 'asp_scout': 'Asp Scout', + 'belugaliner': 'Beluga Liner', + 'cobramkiii': 'Cobra MkIII', + 'cobramkiv': 'Cobra MkIV', + 'clipper': 'Panther Clipper', + 'cutter': 'Imperial Cutter', + 'diamondback': 'Diamondback Scout', + 'diamondbackxl': 'Diamondback Explorer', + 'dolphin': 'Dolphin', + 'eagle': 'Eagle', + 'empire_courier': 'Imperial Courier', + 'empire_eagle': 'Imperial Eagle', + 'empire_fighter': 'Imperial Fighter', + 'empire_trader': 'Imperial Clipper', + 'federation_corvette': 'Federal Corvette', + 'federation_dropship': 'Federal Dropship', + 'federation_dropship_mkii': 'Federal Assault Ship', + 'federation_gunship': 'Federal Gunship', + 'federation_fighter': 'F63 Condor', + 'ferdelance': 'Fer-de-Lance', + 'hauler': 'Hauler', + 'independant_trader': 'Keelback', + 'independent_fighter': 'Taipan Fighter', + 'krait_mkii': 'Krait MkII', + 'krait_light': 'Krait Phantom', + 'mamba': 'Mamba', + 'orca': 'Orca', + 'python': 'Python', + 'scout': 'Taipan Fighter', + 'sidewinder': 'Sidewinder', + 'testbuggy': 'Scarab', + 'type6': 'Type-6 Transporter', + 'type7': 'Type-7 Transporter', + 'type9': 'Type-9 Heavy', + 'type9_military': 'Type-10 Defender', + 'typex': 'Alliance Chieftain', + 'typex_2': 'Alliance Crusader', + 'typex_3': 'Alliance Challenger', + 'viper': 'Viper MkIII', + 'viper_mkiv': 'Viper MkIV', + 'vulture': 'Vulture', } -# Companion API sometimes returns an array as a json array, sometimes as a json object indexed by "int". -# This seems to depend on whether the there are 'gaps' in the Cmdr's data - i.e. whether the array is sparse. -# In practice these arrays aren't very sparse so just convert them to lists with any 'gaps' holding None. -def listify(thing): +def listify(thing: Union[List, Dict]) -> List: + """ + Convert actual JSON array or int-indexed dict into a Python list. + + Companion API sometimes returns an array as a json array, sometimes as + a json object indexed by "int". This seems to depend on whether the + there are 'gaps' in the Cmdr's data - i.e. whether the array is sparse. + In practice these arrays aren't very sparse so just convert them to + lists with any 'gaps' holding None. + """ if thing is None: return [] # data is not present @@ -132,10 +145,12 @@ def listify(thing): return retval else: - raise ValueError("expected an array or sparse array, got {!r}".format(thing)) + raise ValueError(f"expected an array or sparse array, got {thing!r}") class ServerError(Exception): + """Exception Class for CAPI ServerErrors.""" + def __init__(self, *args): # Raised when cannot contact the Companion API server self.args = args @@ -144,23 +159,34 @@ class ServerError(Exception): class ServerLagging(Exception): + """Exception Class for CAPI Server lagging. + + Raised when Companion API server is returning old data, e.g. when the + servers are too busy. + """ + def __init__(self, *args): - # Raised when Companion API server is returning old data, e.g. when the servers are too busy self.args = args if not args: self.args = (_('Error: Frontier server is lagging'),) class SKUError(Exception): + """Exception Class for CAPI SKU error. + + Raised when the Companion API server thinks that the user has not + purchased E:D i.e. doesn't have the correct 'SKU'. + """ + def __init__(self, *args): - # Raised when the Companion API server thinks that the user has not purchased E:D - # i.e. doesn't have the correct 'SKU' self.args = args if not args: self.args = (_('Error: Frontier server SKU problem'),) class CredentialsError(Exception): + """Exception Class for CAPI Credentials error.""" + def __init__(self, *args): self.args = args if not args: @@ -168,24 +194,35 @@ class CredentialsError(Exception): class CmdrError(Exception): + """Exception Class for CAPI Commander error. + + Raised when the user has multiple accounts and the username/password + setting is not for the account they're currently playing OR the user has + reset their Cmdr and the Companion API server is still returning data + for the old Cmdr. + """ + def __init__(self, *args): - # Raised when the user has multiple accounts and the username/password setting is not for - # the account they're currently playing OR the user has reset their Cmdr and the Companion API - # server is still returning data for the old Cmdr self.args = args if not args: self.args = (_('Error: Wrong Cmdr'),) class Auth(object): - def __init__(self, cmdr): + """Handles authentication with the Frontier CAPI service via oAuth2.""" + + def __init__(self, cmdr: str): self.cmdr = cmdr self.session = requests.Session() self.session.headers['User-Agent'] = USER_AGENT self.verifier = self.state = None - def refresh(self): - # Try refresh token. Returns new refresh token if successful, otherwise makes new authorization request. + def refresh(self) -> None: + """ + Attempt use of Refresh Token to get a valid Access Token. + + If the Refresh Token doesn't work, make a new authorization request. + """ logger.debug(f'Trying for "{self.cmdr}"') self.verifier = None @@ -219,7 +256,7 @@ class Auth(object): logger.error(f"Frontier CAPI Auth: Can't refresh token for \"{self.cmdr}\"") self.dump(r) - except Exception as e: + except Exception: logger.exception(f"Frontier CAPI Auth: Can't refresh token for \"{self.cmdr}\"") else: @@ -233,21 +270,17 @@ class Auth(object): self.state = self.base64_url_encode(s.to_bytes(32, byteorder='big')) # Won't work under IE: https://blogs.msdn.microsoft.com/ieinternals/2011/07/13/understanding-protocols/ logger.debug(f'Trying auth from scratch for Commander "{self.cmdr}"') + challenge = self.base64_url_encode(hashlib.sha256(self.verifier).digest()) webbrowser.open( - '{server_auth}{url_auth}?response_type=code&audience=frontier&scope=capi&client_id={client_id}&code_challenge={challenge}&code_challenge_method=S256&state={state}&redirect_uri={redirect}'.format( # noqa: E501 # I cant make this any shorter - server_auth=SERVER_AUTH, - url_auth=URL_AUTH, - client_id=CLIENT_ID, - challenge=self.base64_url_encode(hashlib.sha256(self.verifier).digest()), - state=self.state, - redirect=protocolhandler.redirect - ) + f'{SERVER_AUTH}{URL_AUTH}?response_type=code&audience=frontier&scope=capi&client_id={CLIENT_ID}&code_challenge={challenge}&code_challenge_method=S256&state={self.state}&redirect_uri={protocolhandler.redirect}' # noqa: E501 # I cant make this any shorter ) - def authorize(self, payload): + def authorize(self, payload: str) -> str: + """Handle oAuth authorization callback. + + :return: access token if successful, otherwise raises CredentialsError. + """ logger.debug('Checking oAuth authorization callback') - # Handle OAuth authorization code callback. - # Returns access token if successful, otherwise raises CredentialsError if '?' not in payload: logger.error(f'Frontier CAPI Auth: Malformed response (no "?" in payload)\n{payload}\n') raise CredentialsError('malformed payload') # Not well formed @@ -255,14 +288,14 @@ class Auth(object): data = urllib.parse.parse_qs(payload[(payload.index('?') + 1):]) if not self.state or not data.get('state') or data['state'][0] != self.state: logger.error(f'Frontier CAPI Auth: Unexpected response\n{payload}\n') - raise CredentialsError('Unexpected response from authorization {!r}'.format(payload)) # Unexpected reply + raise CredentialsError(f'Unexpected response from authorization {payload!r}') if not data.get('code'): logger.error(f'Frontier CAPI Auth: Negative response (no "code" in returned data)\n{payload}\n') error = next( (data[k] for k in ('error_description', 'error', 'message') if k in data), ('',) ) - raise CredentialsError('Error: {!r}'.format(error)[0]) + raise CredentialsError(f'Error: {error[0]!r}') try: logger.debug('Got code, posting it back...') @@ -299,10 +332,11 @@ class Auth(object): logger.error(f"Frontier CAPI Auth: Can't get token for \"{self.cmdr}\"") self.dump(r) error = next((data[k] for k in ('error_description', 'error', 'message') if k in data), ('',)) - raise CredentialsError('Error: {!r}'.format(error)[0]) + raise CredentialsError(f'Error: {error[0]!r}') @staticmethod - def invalidate(cmdr): + def invalidate(cmdr: str) -> None: + """Invalidate Refresh Token for specified Commander.""" logger.info(f'Frontier CAPI Auth: Invalidated token for "{cmdr}"') cmdrs = config.get('cmdrs') idx = cmdrs.index(cmdr) @@ -312,14 +346,18 @@ class Auth(object): config.set('fdev_apikeys', tokens) config.save() # Save settings now for use by command-line app - def dump(self, r): + def dump(self, r: requests.Response) -> None: + """Dump details of HTTP failure from oAuth attempt.""" logger.debug(f'Frontier CAPI Auth: {r.url} {r.status_code} {r.reason if r.reason else "None"} {r.text}') - def base64_url_encode(self, text): + def base64_url_encode(self, text: str) -> str: + """Base64 encode text for URL.""" return base64.urlsafe_b64encode(text).decode().replace('=', '') class Session(object): + """Methods for handling an oAuth2 session.""" + STATE_INIT, STATE_AUTH, STATE_OK = list(range(3)) def __init__(self): @@ -329,8 +367,12 @@ class Session(object): self.auth = None self.retrying = False # Avoid infinite loop when successful auth / unsuccessful query - def login(self, cmdr=None, is_beta=None): - # Returns True if login succeeded, False if re-authorization initiated. + def login(self, cmdr: str = None, is_beta: Union[None, bool] = None) -> bool: + """ + Attempt oAuth2 login. + + :return: True if login succeeded, False if re-authorization initiated. + """ if not CLIENT_ID: logger.error('CLIENT_ID is None') raise CredentialsError('cannot login without a valid Client ID') @@ -376,7 +418,8 @@ class Session(object): # Wait for callback # Callback from protocol handler - def auth_callback(self): + def auth_callback(self) -> None: + """Handle callback from edmc:// handler.""" logger.debug('Handling callback from edmc:// handler') if self.state != Session.STATE_AUTH: # Shouldn't be getting a callback @@ -394,14 +437,16 @@ class Session(object): self.auth = None raise # Bad thing happened - def start(self, access_token): + def start(self, access_token: str) -> None: + """Start an oAuth2 session.""" logger.debug('Starting session') self.session = requests.Session() - self.session.headers['Authorization'] = 'Bearer {}'.format(access_token) + self.session.headers['Authorization'] = f'Bearer {access_token}' self.session.headers['User-Agent'] = USER_AGENT self.state = Session.STATE_OK - def query(self, endpoint): + def query(self, endpoint: str) -> Mapping[str, Any]: + """Perform a query against the specified CAPI endpoint.""" logger.debug(f'Performing query for endpoint "{endpoint}"') if self.state == Session.STATE_INIT: if self.login(): @@ -432,7 +477,7 @@ class Session(object): # Server error. Typically 500 "Internal Server Error" if server is down logger.debug('500 status back from CAPI') self.dump(r) - raise ServerError('Received error {} from server'.format(r.status_code)) + raise ServerError(f'Received error {r.status_code} from server') try: r.raise_for_status() # Typically 403 "Forbidden" on token expiry @@ -467,36 +512,41 @@ class Session(object): return data - def profile(self): + def profile(self) -> Mapping[str, Any]: + """Perform general CAPI /profile endpoint query.""" return self.query(URL_QUERY) - def station(self): + def station(self) -> Union[Mapping[str, Any], None]: + """Perform CAPI /profile endpoint query for station data.""" data = self.query(URL_QUERY) - if data['commander'].get('docked'): - services = data['lastStarport'].get('services', {}) + if not data['commander'].get('docked'): + return None - last_starport_name = data['lastStarport']['name'] - last_starport_id = int(data['lastStarport']['id']) + services = data['lastStarport'].get('services', {}) - if services.get('commodities'): - marketdata = self.query(URL_MARKET) - if (last_starport_name != marketdata['name'] or last_starport_id != int(marketdata['id'])): - raise ServerLagging() + last_starport_name = data['lastStarport']['name'] + last_starport_id = int(data['lastStarport']['id']) - else: - data['lastStarport'].update(marketdata) + if services.get('commodities'): + marketdata = self.query(URL_MARKET) + if (last_starport_name != marketdata['name'] or last_starport_id != int(marketdata['id'])): + raise ServerLagging() - if services.get('outfitting') or services.get('shipyard'): - shipdata = self.query(URL_SHIPYARD) - if (last_starport_name != shipdata['name'] or last_starport_id != int(shipdata['id'])): - raise ServerLagging() + else: + data['lastStarport'].update(marketdata) - else: - data['lastStarport'].update(shipdata) + if services.get('outfitting') or services.get('shipyard'): + shipdata = self.query(URL_SHIPYARD) + if (last_starport_name != shipdata['name'] or last_starport_id != int(shipdata['id'])): + raise ServerLagging() + + else: + data['lastStarport'].update(shipdata) return data - def close(self): + def close(self) -> None: + """Close CAPI authorization session.""" self.state = Session.STATE_INIT if self.session: try: @@ -507,18 +557,25 @@ class Session(object): self.session = None - def invalidate(self): + def invalidate(self) -> None: + """Invalidate oAuth2 credentials.""" logger.debug('Forcing a full re-authentication') # Force a full re-authentication self.close() Auth.invalidate(self.credentials['cmdr']) - def dump(self, r): + def dump(self, r: requests.Response) -> None: + """Log, as error, status of requests.Response from CAPI request.""" logger.error(f'Frontier CAPI Auth: {r.url} {r.status_code} {r.reason and r.reason or "None"} {r.text}') -# Returns a shallow copy of the received data suitable for export to older tools -# English commodity names and anomalies fixed up -def fixup(data): + +def fixup(data: Mapping[str, Any]) -> Mapping[str, Any]: # noqa: C901, CCR001 # Can't be usefully simplified + """ + Fix up commodity names to English & miscellaneous anomalies fixes. + + :return: a shallow copy of the received data suitable for export to + older tools. + """ if not commodity_map: # Lazily populate for f in ('commodity.csv', 'rare_commodity.csv'): @@ -589,9 +646,10 @@ def fixup(data): return datacopy -# Return a subset of the received data describing the current ship -def ship(data): - def filter_ship(d): +def ship(data: Mapping[str, Any]) -> Mapping[str, Any]: + """Construct a subset of the received data describing the current ship.""" + def filter_ship(d: Mapping[str, Any]) -> Mapping[str, Any]: + """Filter provided ship data.""" filtered = {} for k, v in d.items(): if v == []: @@ -619,8 +677,8 @@ def ship(data): return filter_ship(data['ship']) -# Ship name suitable for writing to a file -def ship_file_name(ship_name, ship_type): +def ship_file_name(ship_name: str, ship_type: str) -> str: + """Return a ship name suitable for a filename.""" name = str(ship_name or ship_map.get(ship_type.lower(), ship_type)).strip() if name.endswith('.'): name = name[:-1] From da2a582c460ee219c40561de92250ab8a1867708 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 15 Sep 2020 14:35:37 +0100 Subject: [PATCH 2/7] companion.py: Use custom type for CAPI Data. This way if we get more stringent about it (could be a class) we don't need to edit all the references. --- companion.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/companion.py b/companion.py index b6396827..74b3d909 100644 --- a/companion.py +++ b/companion.py @@ -21,7 +21,7 @@ from email.utils import parsedate # TODO: see https://github.com/EDCD/EDMarketConnector/issues/569 from http.cookiejar import LWPCookieJar # noqa: F401 - No longer needed but retained in case plugins use it from os.path import join -from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Union +from typing import TYPE_CHECKING, Any, Dict, List, Mapping, NewType, Union import requests @@ -35,6 +35,9 @@ if TYPE_CHECKING: _ = lambda x: x # noqa: E731 # to make flake8 stop complaining that the hacked in _ method doesnt exist +# Define custom type for the dicts that hold CAPI data +CAPIData = NewType('CAPIData', Mapping[str, Any]) + holdoff = 60 # be nice timeout = 10 # requests timeout auth_timeout = 30 # timeout for initial auth @@ -445,7 +448,7 @@ class Session(object): self.session.headers['User-Agent'] = USER_AGENT self.state = Session.STATE_OK - def query(self, endpoint: str) -> Mapping[str, Any]: + def query(self, endpoint: str) -> CAPIData: """Perform a query against the specified CAPI endpoint.""" logger.debug(f'Performing query for endpoint "{endpoint}"') if self.state == Session.STATE_INIT: @@ -512,11 +515,11 @@ class Session(object): return data - def profile(self) -> Mapping[str, Any]: + def profile(self) -> CAPIData: """Perform general CAPI /profile endpoint query.""" return self.query(URL_QUERY) - def station(self) -> Union[Mapping[str, Any], None]: + def station(self) -> Union[CAPIData, None]: """Perform CAPI /profile endpoint query for station data.""" data = self.query(URL_QUERY) if not data['commander'].get('docked'): @@ -569,7 +572,7 @@ class Session(object): logger.error(f'Frontier CAPI Auth: {r.url} {r.status_code} {r.reason and r.reason or "None"} {r.text}') -def fixup(data: Mapping[str, Any]) -> Mapping[str, Any]: # noqa: C901, CCR001 # Can't be usefully simplified +def fixup(data: CAPIData) -> CAPIData: # noqa: C901, CCR001 # Can't be usefully simplified """ Fix up commodity names to English & miscellaneous anomalies fixes. @@ -646,9 +649,9 @@ def fixup(data: Mapping[str, Any]) -> Mapping[str, Any]: # noqa: C901, CCR001 # return datacopy -def ship(data: Mapping[str, Any]) -> Mapping[str, Any]: +def ship(data: CAPIData) -> CAPIData: """Construct a subset of the received data describing the current ship.""" - def filter_ship(d: Mapping[str, Any]) -> Mapping[str, Any]: + def filter_ship(d: CAPIData) -> CAPIData: """Filter provided ship data.""" filtered = {} for k, v in d.items(): From 457535392312c2e0264220db3f9c15a162c88e78 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 15 Sep 2020 15:00:22 +0100 Subject: [PATCH 3/7] companion.py: CAPIData type & PyCharm-prompted cleanup. * CAPI Data really is just a Dict. Define a custom type so it could easily be a class in future with minimal edits. * Auth.refresh() *can* also return a str (Access Token). * Catch specific (but still quite loose) exceptions in Auth.refresh(). * Set self.server in Session.__init__. * Remove some extraneous () on conditionals. --- companion.py | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/companion.py b/companion.py index 74b3d909..e357bf95 100644 --- a/companion.py +++ b/companion.py @@ -13,6 +13,7 @@ import logging import numbers import os import random +import requests import time import urllib.parse import webbrowser @@ -21,22 +22,19 @@ from email.utils import parsedate # TODO: see https://github.com/EDCD/EDMarketConnector/issues/569 from http.cookiejar import LWPCookieJar # noqa: F401 - No longer needed but retained in case plugins use it from os.path import join -from typing import TYPE_CHECKING, Any, Dict, List, Mapping, NewType, Union - -import requests +from typing import TYPE_CHECKING, Dict, List, NewType, Union from config import appname, appversion, config from protocol import protocolhandler logger = logging.getLogger(appname) - if TYPE_CHECKING: _ = lambda x: x # noqa: E731 # to make flake8 stop complaining that the hacked in _ method doesnt exist # Define custom type for the dicts that hold CAPI data -CAPIData = NewType('CAPIData', Mapping[str, Any]) +CAPIData = NewType('CAPIData', Dict) holdoff = 60 # be nice timeout = 10 # requests timeout @@ -220,11 +218,13 @@ class Auth(object): self.session.headers['User-Agent'] = USER_AGENT self.verifier = self.state = None - def refresh(self) -> None: + def refresh(self) -> Union[str, None]: """ Attempt use of Refresh Token to get a valid Access Token. If the Refresh Token doesn't work, make a new authorization request. + + :return: Access Token if retrieved, else None. """ logger.debug(f'Trying for "{self.cmdr}"') @@ -239,14 +239,14 @@ class Auth(object): tokens = tokens + [''] * (len(cmdrs) - len(tokens)) if tokens[idx]: logger.debug('We have a refresh token for that idx') - try: - data = { - 'grant_type': 'refresh_token', - 'client_id': CLIENT_ID, - 'refresh_token': tokens[idx], - } + data = { + 'grant_type': 'refresh_token', + 'client_id': CLIENT_ID, + 'refresh_token': tokens[idx], + } - logger.debug('Attempting refresh with Frontier...') + logger.debug('Attempting refresh with Frontier...') + try: r = self.session.post(SERVER_AUTH + URL_TOKEN, data=data, timeout=auth_timeout) if r.status_code == requests.codes.ok: data = r.json() @@ -259,7 +259,7 @@ class Auth(object): logger.error(f"Frontier CAPI Auth: Can't refresh token for \"{self.cmdr}\"") self.dump(r) - except Exception: + except (ValueError, requests.RequestException, ): logger.exception(f"Frontier CAPI Auth: Can't refresh token for \"{self.cmdr}\"") else: @@ -300,9 +300,9 @@ class Auth(object): ) raise CredentialsError(f'Error: {error[0]!r}') + r = None try: logger.debug('Got code, posting it back...') - r = None data = { 'grant_type': 'authorization_code', 'client_id': CLIENT_ID, @@ -365,6 +365,7 @@ class Session(object): def __init__(self): self.state = Session.STATE_INIT + self.server = None self.credentials = None self.session = None self.auth = None @@ -532,7 +533,7 @@ class Session(object): if services.get('commodities'): marketdata = self.query(URL_MARKET) - if (last_starport_name != marketdata['name'] or last_starport_id != int(marketdata['id'])): + if last_starport_name != marketdata['name'] or last_starport_id != int(marketdata['id']): raise ServerLagging() else: @@ -540,7 +541,7 @@ class Session(object): if services.get('outfitting') or services.get('shipyard'): shipdata = self.query(URL_SHIPYARD) - if (last_starport_name != shipdata['name'] or last_starport_id != int(shipdata['id'])): + if last_starport_name != shipdata['name'] or last_starport_id != int(shipdata['id']): raise ServerLagging() else: @@ -646,7 +647,7 @@ def fixup(data: CAPIData) -> CAPIData: # noqa: C901, CCR001 # Can't be usefully datacopy = data.copy() datacopy['lastStarport'] = data['lastStarport'].copy() datacopy['lastStarport']['commodities'] = commodities - return datacopy + return CAPIData(datacopy) def ship(data: CAPIData) -> CAPIData: From 55fcfbeb17937cc4fdc86334b530f18c6bb1d4d3 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 15 Sep 2020 15:33:00 +0100 Subject: [PATCH 4/7] companion.py: Further PyCharm-prompted cleanup. * Correct use of error[0] to just error. * We don't want methods not referencing self to be static in this case, so annotate them. * Take suggestion of "if v == []:" equivalent to "if not v:". * Preserve CAPIData typing throughout filter_ship(). --- companion.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/companion.py b/companion.py index e357bf95..6b742526 100644 --- a/companion.py +++ b/companion.py @@ -298,7 +298,7 @@ class Auth(object): error = next( (data[k] for k in ('error_description', 'error', 'message') if k in data), ('',) ) - raise CredentialsError(f'Error: {error[0]!r}') + raise CredentialsError(f'Error: {error!r}') r = None try: @@ -335,7 +335,7 @@ class Auth(object): logger.error(f"Frontier CAPI Auth: Can't get token for \"{self.cmdr}\"") self.dump(r) error = next((data[k] for k in ('error_description', 'error', 'message') if k in data), ('',)) - raise CredentialsError(f'Error: {error[0]!r}') + raise CredentialsError(f'Error: {error!r}') @staticmethod def invalidate(cmdr: str) -> None: @@ -349,11 +349,13 @@ class Auth(object): config.set('fdev_apikeys', tokens) config.save() # Save settings now for use by command-line app + # noinspection PyMethodMayBeStatic def dump(self, r: requests.Response) -> None: """Dump details of HTTP failure from oAuth attempt.""" logger.debug(f'Frontier CAPI Auth: {r.url} {r.status_code} {r.reason if r.reason else "None"} {r.text}') - def base64_url_encode(self, text: str) -> str: + # noinspection PyMethodMayBeStatic + def base64_url_encode(self, text: bytes) -> str: """Base64 encode text for URL.""" return base64.urlsafe_b64encode(text).decode().replace('=', '') @@ -568,6 +570,7 @@ class Session(object): self.close() Auth.invalidate(self.credentials['cmdr']) + # noinspection PyMethodMayBeStatic def dump(self, r: requests.Response) -> None: """Log, as error, status of requests.Response from CAPI request.""" logger.error(f'Frontier CAPI Auth: {r.url} {r.status_code} {r.reason and r.reason or "None"} {r.text}') @@ -654,9 +657,9 @@ def ship(data: CAPIData) -> CAPIData: """Construct a subset of the received data describing the current ship.""" def filter_ship(d: CAPIData) -> CAPIData: """Filter provided ship data.""" - filtered = {} + filtered: CAPIData = CAPIData({}) for k, v in d.items(): - if v == []: + if not v: pass # just skip empty fields for brevity elif k in ('alive', 'cargo', 'cockpitBreached', 'health', 'oxygenRemaining', From c311957efffaa9e4610628654377a408609cabfa Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 15 Sep 2020 15:49:28 +0100 Subject: [PATCH 5/7] companion.py: `import requests` does belong down there. --- companion.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/companion.py b/companion.py index 6b742526..b01fc7a0 100644 --- a/companion.py +++ b/companion.py @@ -13,7 +13,6 @@ import logging import numbers import os import random -import requests import time import urllib.parse import webbrowser @@ -24,6 +23,8 @@ from http.cookiejar import LWPCookieJar # noqa: F401 - No longer needed but ret from os.path import join from typing import TYPE_CHECKING, Dict, List, NewType, Union +import requests + from config import appname, appversion, config from protocol import protocolhandler From ed52528718202f859db22bf29c1f821a1c512600 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 15 Sep 2020 16:15:13 +0100 Subject: [PATCH 6/7] companion.py: Slight tweaks from running mypy NB: Using `mypy --follow-imports skip` for now to limit how much it checks each time. --- companion.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/companion.py b/companion.py index b01fc7a0..2d4ba73c 100644 --- a/companion.py +++ b/companion.py @@ -21,7 +21,7 @@ from email.utils import parsedate # TODO: see https://github.com/EDCD/EDMarketConnector/issues/569 from http.cookiejar import LWPCookieJar # noqa: F401 - No longer needed but retained in case plugins use it from os.path import join -from typing import TYPE_CHECKING, Dict, List, NewType, Union +from typing import TYPE_CHECKING, Any, Dict, List, NewType, Union import requests @@ -67,7 +67,7 @@ category_map = { 'NonMarketable': False, # Don't appear in the in-game market so don't report } -commodity_map = {} +commodity_map: Dict = {} ship_map = { 'adder': 'Adder', @@ -134,7 +134,7 @@ def listify(thing: Union[List, Dict]) -> List: return list(thing) # array is not sparse elif isinstance(thing, dict): - retval = [] + retval: List[Any] = [] for k, v in thing.items(): idx = int(k) @@ -214,10 +214,11 @@ class Auth(object): """Handles authentication with the Frontier CAPI service via oAuth2.""" def __init__(self, cmdr: str): - self.cmdr = cmdr + self.cmdr: str = cmdr self.session = requests.Session() self.session.headers['User-Agent'] = USER_AGENT - self.verifier = self.state = None + self.verifier: Union[bytes, None] = None + self.state: Union[str, None] = None def refresh(self) -> Union[str, None]: """ @@ -279,6 +280,8 @@ class Auth(object): f'{SERVER_AUTH}{URL_AUTH}?response_type=code&audience=frontier&scope=capi&client_id={CLIENT_ID}&code_challenge={challenge}&code_challenge_method=S256&state={self.state}&redirect_uri={protocolhandler.redirect}' # noqa: E501 # I cant make this any shorter ) + return None + def authorize(self, payload: str) -> str: """Handle oAuth authorization callback. @@ -304,7 +307,7 @@ class Auth(object): r = None try: logger.debug('Got code, posting it back...') - data = { + request_data = { 'grant_type': 'authorization_code', 'client_id': CLIENT_ID, 'code_verifier': self.verifier, @@ -312,7 +315,7 @@ class Auth(object): 'redirect_uri': protocolhandler.redirect, } - r = self.session.post(SERVER_AUTH + URL_TOKEN, data=data, timeout=auth_timeout) + r = self.session.post(SERVER_AUTH + URL_TOKEN, data=request_data, timeout=auth_timeout) data = r.json() if r.status_code == requests.codes.ok: logger.info(f'Frontier CAPI Auth: New token for \"{self.cmdr}\"') @@ -324,7 +327,7 @@ class Auth(object): config.set('fdev_apikeys', tokens) config.save() # Save settings now for use by command-line app - return data.get('access_token') + return str(data.get('access_token')) except Exception as e: logger.exception(f"Frontier CAPI Auth: Can't get token for \"{self.cmdr}\"") @@ -515,7 +518,7 @@ class Session(object): self.retrying = False if 'timestamp' not in data: logger.debug('timestamp not in data, adding from response headers') - data['timestamp'] = time.strftime('%Y-%m-%dT%H:%M:%SZ', parsedate(r.headers['Date'])) + data['timestamp'] = time.strftime('%Y-%m-%dT%H:%M:%SZ', parsedate(r.headers['Date'])) # type: ignore return data From 14d1a0ad5efb151f267b8ffebc6664f08f482a67 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Tue, 15 Sep 2020 17:30:23 +0100 Subject: [PATCH 7/7] companion.py: Re-format next(...) to be more obvious. --- companion.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/companion.py b/companion.py index 2d4ba73c..3ed92e7b 100644 --- a/companion.py +++ b/companion.py @@ -300,7 +300,8 @@ class Auth(object): if not data.get('code'): logger.error(f'Frontier CAPI Auth: Negative response (no "code" in returned data)\n{payload}\n') error = next( - (data[k] for k in ('error_description', 'error', 'message') if k in data), ('',) + (data[k] for k in ('error_description', 'error', 'message') if k in data), + '' ) raise CredentialsError(f'Error: {error!r}') @@ -338,7 +339,10 @@ class Auth(object): logger.error(f"Frontier CAPI Auth: Can't get token for \"{self.cmdr}\"") self.dump(r) - error = next((data[k] for k in ('error_description', 'error', 'message') if k in data), ('',)) + error = next( + (data[k] for k in ('error_description', 'error', 'message') if k in data), + '' + ) raise CredentialsError(f'Error: {error!r}') @staticmethod