Merge pull request #215 from IdentityPython/ft-time_handling_fixes

time handling fixes

Author: leifj

requirements.txt

@@ -4,7 +4,6 @@ cachetools
 gunicorn
 httplib2 >=0.7.7
 ipaddr
-iso8601 >=0.1.4
 lxml >=4.1.1
 mako
 nose

src/pyff/api.py

@@ -2,6 +2,7 @@
 import threading
 from datetime import datetime, timedelta
 from json import dumps
+from typing import Any, List, Mapping
 
 import pkg_resources
 import pyramid.httpexceptions as exc
@@ -21,8 +22,9 @@
 from .logs import get_log
 from .pipes import plumbing
 from .repo import MDRepository
+from .resource import Resource
 from .samlmd import entity_display_name
-from .utils import b2u, dumptree, duration2timedelta, hash_id, json_serializer
+from .utils import b2u, dumptree, duration2timedelta, hash_id, json_serializer, utc_now
 
 log = get_log(__name__)
 
@@ -387,7 +389,7 @@ def resources_handler(request):
     :return: a JSON representation of the set of resources currently loaded by the server
     """
 
-    def _info(r):
+    def _info(r: Resource) -> List[Mapping[str, Any]]:
         nfo = r.info
         nfo['Valid'] = r.is_valid()
         nfo['Parser'] = r.last_parser
@@ -407,7 +409,7 @@ def _info(r):
 
 def pipeline_handler(request):
     """
-    Implements the /api/resources endpoint
+    Implements the /api/pipeline endpoint
 
     :param request: the HTTP request
     :return: a JSON representation of the active pipeline
@@ -536,8 +538,7 @@ def mkapp(*args, **kwargs):
         ctx.add_route('request', '/*path', request_method='GET')
         ctx.add_view(request_handler, route_name='request')
 
-        start = datetime.utcnow() + timedelta(seconds=1)
-        log.debug(start)
+        start = utc_now() + timedelta(seconds=1)
         if config.update_frequency > 0:
             ctx.registry.scheduler.add_job(
                 call,

src/pyff/builtins.py

@@ -17,13 +17,12 @@
 import ipaddr
 import six
 import xmlsec
-from iso8601 import iso8601
 from lxml.etree import DocumentInvalid
 from six.moves.urllib_parse import quote_plus, urlparse
 
 from pyff.pipes import registry
 
-from .constants import NS, config
+from .constants import NS
 from .decorators import deprecated
 from .exceptions import MetadataException
 from .logs import get_log
@@ -42,12 +41,15 @@
     sort_entities,
 )
 from .utils import (
+    datetime2iso,
     dumptree,
     duration2timedelta,
     hash_id,
+    iso2datetime,
     root,
     safe_write,
     total_seconds,
+    utc_now,
     validate_document,
     with_tree,
     xslt_transform,
@@ -1503,7 +1505,7 @@ def finalize(req, *opts):
         if name:
             e.set('Name', name)
 
-    now = datetime.utcnow()
+    now = utc_now()
 
     mdid = req.args.get('ID', 'prefix _')
     if re.match('(\s)*prefix(\s)*', mdid):
@@ -1520,18 +1522,19 @@ def finalize(req, *opts):
         offset = duration2timedelta(valid_until)
         if offset is not None:
             dt = now + offset
-            e.set('validUntil', dt.strftime("%Y-%m-%dT%H:%M:%SZ"))
+            e.set('validUntil', datetime2iso(dt))
         elif valid_until is not None:
+            # TODO: if validUntil was not present, valid_until will be the string 'None' here - never the literal None
             try:
-                dt = iso8601.parse_date(valid_until)
-                dt = dt.replace(tzinfo=None)  # make dt "naive" (tz-unaware)
+                dt = iso2datetime(valid_until)
                 offset = dt - now
-                e.set('validUntil', dt.strftime("%Y-%m-%dT%H:%M:%SZ"))
+                e.set('validUntil', datetime2iso(dt))
             except ValueError as ex:
                 log.error("Unable to parse validUntil: %s (%s)" % (valid_until, ex))
 
-                # set a reasonable default: 50% of the validity
+        # set a reasonable default: 50% of the validity
         # we replace this below if we have cacheDuration set
+        # TODO: offset can be None here, if validUntil is not a valid duration or ISO date
         req.state['cache'] = int(total_seconds(offset) / 50)
 
     cache_duration = req.args.get('cacheDuration', e.get('cacheDuration', None))

src/pyff/parse.py

@@ -5,7 +5,7 @@
 
 from .constants import NS
 from .logs import get_log
-from .utils import find_matching_files, first_text, parse_xml, root, unicode_stream
+from .utils import find_matching_files, parse_xml, root, unicode_stream, utc_now
 
 __author__ = 'leifj'
 
@@ -66,7 +66,7 @@ def parse(self, resource, content):
 
         resource.never_expires = True
         resource.expire_time = None
-        resource.last_seen = datetime.now()
+        resource.last_seen = utc_now().replace(microsecond=0)
 
         return dict()
 
@@ -98,7 +98,7 @@ def parse(self, resource, content):
                     fp = fingerprints[0]
                 log.debug("XRD: {} verified by {}".format(link_href, fp))
                 resource.add_child(link_href, verify=fp)
-        resource.last_seen = datetime.now()
+        resource.last_seen = utc_now().replace(microsecond=0)
         resource.expire_time = None
         resource.never_expires = True
         return info

src/pyff/pipes.py

@@ -2,13 +2,17 @@
 Pipes and plumbing. Plumbing instances are sequences of pipes. Each pipe is called in order to load, select,
 transform, sign or output SAML metadata.
 """
+from __future__ import annotations
 
 import os
 import traceback
+from typing import Any, Dict, Optional
 
 import yaml
+from apscheduler.schedulers.background import BackgroundScheduler
 
 from .logs import get_log
+from .repo import MDRepository
 from .utils import PyffException, is_text, resource_string
 
 log = get_log(__name__)
@@ -202,7 +206,16 @@ class Request(object):
         """
 
         def __init__(
-            self, pl, md, t=None, name=None, args=None, state=None, store=None, scheduler=None, raise_exceptions=True
+            self,
+            pl: Plumbing,
+            md: MDRepository,
+            t=None,
+            name=None,
+            args=None,
+            state: Optional[Dict[str, Any]] = None,
+            store=None,
+            scheduler: Optional[BackgroundScheduler] = None,
+            raise_exceptions: bool = True,
         ):
             if not state:
                 state = dict()
@@ -313,7 +326,7 @@ def process(self, md, args=None, state=None, t=None, store=None, raise_exception
         ).process(self)
 
 
-def plumbing(fn):
+def plumbing(fn: str) -> Plumbing:
     """
     Create a new plumbing instance by parsing yaml from the filename.
 

src/pyff/resource.py

@@ -3,12 +3,14 @@
 An abstraction layer for metadata fetchers. Supports both synchronous and asynchronous fetchers with cache.
 
 """
+from __future__ import annotations
 
 import os
 from collections import deque
 from copy import deepcopy
 from datetime import datetime
 from threading import Condition, Lock
+from typing import Optional
 
 import requests
 
@@ -17,7 +19,7 @@
 from .fetch import make_fetcher
 from .logs import get_log
 from .parse import parse_resource
-from .utils import Watchable, hex_digest, img_to_data, non_blocking_lock, url_get
+from .utils import Watchable, hex_digest, img_to_data, non_blocking_lock, url_get, utc_now
 
 requests.packages.urllib3.disable_warnings()
 
@@ -133,9 +135,9 @@ def __init__(self, url=None, **kwargs):
         self.t = None
         self.type = "text/plain"
         self.etag = None
-        self.expire_time = None
-        self.never_expires = False
-        self.last_seen = None
+        self.expire_time: Optional[datetime] = None
+        self.never_expires: bool = False
+        self.last_seen: Optional[datetime] = None
         self.last_parser = None
         self._infos = deque(maxlen=config.info_buffer_size)
         self.children = deque()
@@ -220,13 +222,13 @@ def walk(self):
             for cn in c.walk():
                 yield cn
 
-    def is_expired(self):
+    def is_expired(self) -> bool:
         if self.never_expires:
             return False
-        now = datetime.now()
+        now = utc_now()
         return self.expire_time is not None and self.expire_time < now
 
-    def is_valid(self):
+    def is_valid(self) -> bool:
         return not self.is_expired() and self.last_seen is not None and self.last_parser is not None
 
     def add_info(self, info):
@@ -239,7 +241,7 @@ def _replace(self, r):
                 return
         raise ValueError("Resource {} not present - use add_child".format(r.url))
 
-    def add_child(self, url, **kwargs):
+    def add_child(self, url: str, **kwargs) -> Resource:
         opts = deepcopy(self.opts)
         if 'as' in opts:
             del opts['as']
@@ -301,7 +303,7 @@ def parse(self, getter):
             info.update(parse_info)
 
         if self.t is not None:
-            self.last_seen = datetime.now()
+            self.last_seen = utc_now().replace(microsecond=0)
             if self.post and isinstance(self.post, list):
                 for cb in self.post:
                     if self.t is not None:

src/pyff/samlmd.py

@@ -17,6 +17,7 @@
     Lambda,
     b2u,
     check_signature,
+    datetime2iso,
     dumptree,
     duration2timedelta,
     filter_lang,
@@ -34,6 +35,7 @@
     subdomains,
     unicode_stream,
     url2host,
+    utc_now,
     validate_document,
     xml_error,
 )
@@ -172,7 +174,10 @@ def parse(self, resource, content):
         )
 
         if expire_time_offset is not None:
-            expire_time = datetime.now() + expire_time_offset
+            now = utc_now()
+            now = now.replace(microsecond=0)
+
+            expire_time = now + expire_time_offset
             resource.expire_time = expire_time
             info['Expiration Time'] = str(expire_time)
 
@@ -217,13 +222,13 @@ def parse(self, resource, content):
             info['NextUpdate'] = next_update
             resource.expire_time = iso2datetime(next_update)
         elif config.respect_cache_duration:
-            now = datetime.utcnow()
+            now = utc_now()
             now = now.replace(microsecond=0)
             next_update = now + duration2timedelta(config.default_cache_duration)
             info['NextUpdate'] = next_update
             resource.expire_time = next_update
 
-        info['Expiration Time'] = str(resource.expire_time)
+        info['Expiration Time'] = 'None' if not resource.expire_time else resource.expire_time.isoformat()
         info['IssuerName'] = first_text(relt, "{%s}IssuerName" % NS['ser'])
         info['SchemeIdentifier'] = first_text(relt, "{%s}SchemeIdentifier" % NS['ser'])
         info['SchemeTerritory'] = first_text(relt, "{%s}SchemeTerritory" % NS['ser'])
@@ -266,7 +271,7 @@ def _update_entities(_t, **kwargs):
                         r.add_via(Lambda(_update_entities, **args))
 
         log.debug("Done parsing eIDAS MetadataServiceList")
-        resource.last_seen = datetime.now()
+        resource.last_seen = utc_now().replace(microsecond=0)
         resource.expire_time = None
         return info
 
@@ -280,10 +285,9 @@ def metadata_expiration(t):
         cache_duration = config.default_cache_duration
         valid_until = relt.get('validUntil', None)
         if valid_until is not None:
-            now = datetime.utcnow()
+            now = utc_now().replace(microsecond=0)
             vu = iso2datetime(valid_until)
-            now = now.replace(microsecond=0)
-            vu = vu.replace(microsecond=0, tzinfo=None)
+            vu = vu.replace(microsecond=0)
             return vu - now
         elif config.respect_cache_duration:
             cache_duration = relt.get('cacheDuration', config.default_cache_duration)
@@ -1037,8 +1041,7 @@ def set_pubinfo(e, publisher=None, creation_instant=None):
         raise MetadataException("At least publisher must be provided")
 
     if creation_instant is None:
-        now = datetime.utcnow()
-        creation_instant = now.strftime("%Y-%m-%dT%H:%M:%SZ")
+        creation_instant = datetime2iso(utc_now())
 
     ext = entity_extensions(e)
     pi = ext.find(".//{%s}PublicationInfo" % NS['mdrpi'])
@@ -1080,10 +1083,8 @@ def expiration(t):
         cache_duration = config.default_cache_duration
         valid_until = relt.get('validUntil', None)
         if valid_until is not None:
-            now = datetime.utcnow()
+            now = utc_now().replace(microsecond=0)
             vu = iso2datetime(valid_until)
-            now = now.replace(microsecond=0)
-            vu = vu.replace(microsecond=0, tzinfo=None)
             return vu - now
         elif config.respect_cache_duration:
             cache_duration = relt.get('cacheDuration', config.default_cache_duration)