refactor code to make typechecking happy, mostly adding safe guards

Author: fredrikt

src/pyff/api.py

@@ -178,7 +178,7 @@ def _d(x: Optional[str], do_split: bool = True) -> Tuple[Optional[str], Optional
 
         return x, None
 
-    log.debug(request)
+    log.debug(f'Processing request: {request}')
 
     if request.matchdict is None:
         raise exc.exception_response(400)
@@ -190,18 +190,18 @@ def _d(x: Optional[str], do_split: bool = True) -> Tuple[Optional[str], Optional
             pass
 
     entry = request.matchdict.get('entry', 'request')
-    path = list(request.matchdict.get('path', []))
+    path_elem = list(request.matchdict.get('path', []))
     match = request.params.get('q', request.params.get('query', None))
 
     # Enable matching on scope.
     match = match.split('@').pop() if match and not match.endswith('@') else match
     log.debug("match={}".format(match))
 
-    if 0 == len(path):
-        path = ['entities']
+    if not path_elem:
+        path_elem = ['entities']
 
-    alias = path.pop(0)
-    path = '/'.join(path)
+    alias = path_elem.pop(0)
+    path = '/'.join(path_elem)
 
     # Ugly workaround bc WSGI drops double-slashes.
     path = path.replace(':/', '://')
@@ -234,23 +234,31 @@ def _d(x: Optional[str], do_split: bool = True) -> Tuple[Optional[str], Optional
     accept = str(request.accept).split(',')[0]
     valid_accept = accept and not ('application/*' in accept or 'text/*' in accept or '*/*' in accept)
 
-    path_no_extension, extension = _d(path, True)
-    accept_from_extension = _ctypes.get(extension, accept)
+    new_path: Optional[str] = path
+    path_no_extension, extension = _d(new_path, True)
+    accept_from_extension = accept
+    if extension:
+        accept_from_extension = _ctypes.get(extension, accept)
 
     if policy == 'extension':
-        path = path_no_extension
+        new_path = path_no_extension
         if not valid_accept:
             accept = accept_from_extension
     elif policy == 'adaptive':
         if not valid_accept:
-            path = path_no_extension
+            new_path = path_no_extension
             accept = accept_from_extension
 
-    if pfx and path:
-        q = "{%s}%s" % (pfx, path)
-        path = "/%s/%s" % (alias, path)
+    if not accept:
+        log.warning('Could not determine accepted response type')
+        raise exc.exception_response(400)
+
+    q: Optional[str]
+    if pfx and new_path:
+        q = f'{{{pfx}}}{new_path}'
+        new_path = f'/{alias}/{new_path}'
     else:
-        q = path
+        q = new_path
 
     try:
         accepter = MediaAccept(accept)
@@ -262,18 +270,19 @@ def _d(x: Optional[str], do_split: bool = True) -> Tuple[Optional[str], Optional
                 'url': request.current_route_url(),
                 'select': q,
                 'match': match.lower() if match else match,
-                'path': path,
+                'path': new_path,
                 'stats': {},
             }
 
             r = p.process(request.registry.md, state=state, raise_exceptions=True, scheduler=request.registry.scheduler)
-            log.debug(r)
+            log.debug(f'Plumbing process result: {r}')
             if r is None:
                 r = []
 
             response = Response()
-            response.headers.update(state.get('headers', {}))
-            ctype = state.get('headers').get('Content-Type', None)
+            _headers = state.get('headers', {})
+            response.headers.update(_headers)
+            ctype = _headers.get('Content-Type', None)
             if not ctype:
                 r, t = _fmt(r, accepter)
                 ctype = t
@@ -288,13 +297,13 @@ def _d(x: Optional[str], do_split: bool = True) -> Tuple[Optional[str], Optional
         import traceback
 
         log.debug(traceback.format_exc())
-        log.warning(ex)
+        log.warning(f'Exception from processing pipeline: {ex}')
         raise exc.exception_response(409)
     except BaseException as ex:
         import traceback
 
         log.debug(traceback.format_exc())
-        log.error(ex)
+        log.error(f'Exception from processing pipeline: {ex}')
         raise exc.exception_response(500)
 
     if request.method == 'GET':
@@ -332,7 +341,7 @@ def webfinger_handler(request: Request) -> Response:
      "subject": "http://reep.refeds.org:8080"
     }
 
-    Depending on which version of pyFF your're running and the configuration you
+    Depending on which version of pyFF you're running and the configuration you
     may also see downstream metadata listed using the 'role' attribute to the link
     elements.
     """
@@ -344,7 +353,7 @@ def webfinger_handler(request: Request) -> Response:
         resource = request.host_url
 
     jrd: Dict[str, Any] = dict()
-    dt = datetime.now() + duration2timedelta("PT1H")
+    dt = datetime.now() + timedelta(hours=1)
     jrd['expires'] = dt.isoformat()
     jrd['subject'] = request.host_url
     links: List[Dict[str, Any]] = list()
@@ -509,7 +518,9 @@ def mkapp(*args: Any, **kwargs: Any) -> Any:
         for mn in config.modules:
             importlib.import_module(mn)
 
-        pipeline = args or None
+        pipeline = None
+        if args:
+            pipeline = list(args)
         if pipeline is None and config.pipeline:
             pipeline = [config.pipeline]
 

src/pyff/logs.py

@@ -3,7 +3,7 @@
 import logging
 import os
 import syslog
-from typing import Any
+from typing import Any, Optional
 
 import six
 
@@ -65,7 +65,7 @@ def get_log(name: str) -> PyFFLogger:
 log = get_log('pyff')
 
 
-def log_config_file(ini: str) -> None:
+def log_config_file(ini: Optional[str]) -> None:
     if ini is not None:
         import logging.config
 
@@ -76,7 +76,7 @@ def log_config_file(ini: str) -> None:
         logging.config.fileConfig(ini)
 
 
-log_config_file(os.getenv('PYFF_LOGGING', None))
+log_config_file(os.getenv('PYFF_LOGGING'))
 
 # http://www.aminus.org/blogs/index.php/2008/07/03/writing-high-efficiency-large-python-sys-1?blog=2
 # blog post explicitly gives permission for use

src/pyff/pipes.py

@@ -108,7 +108,7 @@ def _n(_d: str) -> Tuple[str, List[str]]:
 
     name = None
     args = None
-    opts = []
+    opts: List[str] = []
     if is_text(d):
         name, opts = _n(d)
     elif hasattr(d, '__iter__') and not type(d) is dict:
@@ -172,7 +172,7 @@ def __call__(self, *args: Any, **kwargs: Any) -> Any:
             return self.plumbing.process(self.req.md, store=self.store, state=state, t=t)
         except Exception as ex:
             log.debug(traceback.format_exc())
-            log.error(ex)
+            log.error(f'Got an exception executing the plumbing process: {ex}')
             raise ex
 
 
@@ -327,7 +327,7 @@ def iprocess(self, req: Plumbing.Request) -> ElementTree:
                     break
             except BaseException as ex:
                 log.debug(traceback.format_exc())
-                log.error(ex)
+                log.error(f'Got exception when loading/executing pipe: {ex}')
                 req.exception = ex
                 if req.raise_exceptions:
                     raise ex

src/pyff/tools.py

@@ -14,7 +14,7 @@
 from xmldiff.main import diff_trees
 
 from pyff.constants import config, parse_options
-from pyff.resource import Resource
+from pyff.resource import Resource, ResourceOpts
 from pyff.samlmd import diff, iter_entities
 from pyff.store import MemoryStore
 
@@ -31,12 +31,12 @@ def difftool():
 
     try:
         rm = Resource()
-        r1 = Resource(args[0])
-        r2 = Resource(args[1])
         rm.add(r1)
         rm.add(r2)
         store = MemoryStore()
         rm.reload(store=store)
+        r1 = Resource(url=args[0], opts=ResourceOpts())
+        r2 = Resource(url=args[1], opts=ResourceOpts())
         status = 0
 
         if r1.t.get('Name') != r2.t.get('Name'):

src/pyff/utils.py

@@ -25,7 +25,7 @@
 from itertools import chain
 from threading import local
 from time import gmtime, strftime
-from typing import Any, BinaryIO, Callable, List, Optional, Sequence, Set, Tuple, Union
+from typing import Any, BinaryIO, Callable, Dict, List, Optional, Sequence, Set, Tuple, Union
 
 import pkg_resources
 import requests
@@ -390,17 +390,20 @@ def duration2timedelta(period: str) -> Optional[timedelta]:
     return delta
 
 
-def _lang(elt, default_lang):
+def _lang(elt: Element, default_lang: Optional[str]) -> Optional[str]:
     return elt.get("{http://www.w3.org/XML/1998/namespace}lang", default_lang)
 
 
-def lang_dict(elts, getter=lambda e: e, default_lang=None):
+def lang_dict(elts: Sequence[Element], getter=lambda e: e, default_lang: Optional[str] = None) -> Dict[str, Callable]:
     if default_lang is None:
         default_lang = config.langs[0]
 
     r = dict()
     for e in elts:
-        r[_lang(e, default_lang)] = getter(e)
+        _l = _lang(e, default_lang)
+        if not _l:
+            raise ValueError('Could not get lang from element, and no default provided')
+        r[_l] = getter(e)
     return r
 
 
@@ -422,6 +425,9 @@ def filter_lang(elts: Any, langs: Optional[Sequence[str]] = None) -> List[Elemen
     if len(elts) == 0:
         return []
 
+    if not langs:
+        raise RuntimeError('Configuration is missing langs')
+
     dflt = langs[0]
     lst = [find_lang(elts, l, dflt) for l in langs]
     if len(lst) > 0:
@@ -773,7 +779,7 @@ def img_to_data(data: bytes, content_type: str) -> Optional[str]:
                 assert data64
                 mime_type = "image/png"
         except BaseException as ex:
-            log.warning(ex)
+            log.warning(f'Exception when making Image: {ex}')
             log.debug(traceback.format_exc())
 
     if data64 is None or len(data64) == 0: