refactor configuration parsing

* use a single default configuration, share/isso.conf
* try to use config.new in some tests which are decoupled

A few tests now depend on `isso.dist` to show that they (or the used
objects) have too much dependencies and need to be rewritten.

isso/__init__.py

@@ -62,8 +62,8 @@ from werkzeug.contrib.profiler import ProfilerMiddleware
 local = Local()
 local_manager = LocalManager([local])
 
-from isso import db, migrate, wsgi, ext, views
+from isso import config, db, migrate, wsgi, ext, views
-from isso.core import ThreadedMixin, ProcessMixin, uWSGIMixin, Config
+from isso.core import ThreadedMixin, ProcessMixin, uWSGIMixin
 from isso.wsgi import origin, urlsplit
 from isso.utils import http, JSONRequest, html, hash
 from isso.views import comments
@@ -215,7 +215,7 @@ def main():
     serve = subparser.add_parser("run", help="run server")
 
     args = parser.parse_args()
-    conf = Config.load(args.conf)
+    conf = config.load(join(dist.location, "share", "isso.conf"), args.conf)
 
     if args.command == "import":
         conf.set("guard", "enabled", "off")

isso/config.py

@@ -0,0 +1,153 @@
+# -*- encoding: utf-8 -*-
+
+from __future__ import unicode_literals
+
+import re
+import logging
+import datetime
+
+from email.utils import parseaddr, formataddr
+from configparser import ConfigParser
+
+from isso.compat import text_type as str
+
+logger = logging.getLogger("isso")
+
+
+# Python 2.6 compatibility
+def total_seconds(td):
+    return (td.microseconds + (td.seconds + td.days * 24 * 3600) * 10**6) / 10**6
+
+
+def timedelta(string):
+    """
+    Parse :param string: into :class:`datetime.timedelta`, you can use any
+    (logical) combination of Nw, Nd, Nh and Nm, e.g. `1h30m` for 1 hour, 30
+    minutes or `3w` for 3 weeks.
+
+    Raises a ValueError if the input is invalid/unparseable.
+
+    >>> print(timedelta("3w"))
+    21 days, 0:00:00
+    >>> print(timedelta("3w 12h 57m"))
+    21 days, 12:57:00
+    >>> print(timedelta("1h30m37s"))
+    1:30:37
+    >>> print(timedelta("1asdf3w"))
+    Traceback (most recent call last):
+        ...
+    ValueError: invalid human-readable timedelta
+    """
+
+    keys = ["weeks", "days", "hours", "minutes", "seconds"]
+    regex = "".join(["((?P<%s>\d+)%s ?)?" % (k, k[0]) for k in keys])
+    kwargs = {}
+    for k, v in re.match(regex, string).groupdict(default="0").items():
+        kwargs[k] = int(v)
+
+    rv = datetime.timedelta(**kwargs)
+    if rv == datetime.timedelta():
+        raise ValueError("invalid human-readable timedelta")
+
+    return datetime.timedelta(**kwargs)
+
+
+class Section(object):
+    """A wrapper around :class:`IssoParser` that returns a partial configuration
+    section object.
+
+    >>> conf = new({"foo": {"bar": "spam"}})
+    >>> section = conf.section("foo")
+    >>> conf.get("foo", "bar") == section.get("bar")
+    True
+    """
+
+    def __init__(self, conf, section):
+        self.conf = conf
+        self.section = section
+
+    def get(self, key):
+        return self.conf.get(self.section, key)
+
+    def getint(self, key):
+        return self.conf.getint(self.section, key)
+
+    def getlist(self, key):
+        return self.conf.getlist(self.section, key)
+
+    def getiter(self, key):
+        return self.conf.getiter(self.section, key)
+
+    def getboolean(self, key):
+        return self.conf.getboolean(self.section, key)
+
+
+class IssoParser(ConfigParser):
+    """Parse INI-style configuration with some modifications for Isso.
+
+        * parse human-readable timedelta such as "15m" as "15 minutes"
+        * handle indented lines as "lists"
+    """
+
+    def getint(self, section, key):
+        try:
+            delta = timedelta(self.get(section, key))
+        except ValueError:
+            return super(IssoParser, self).getint(section, key)
+        else:
+            try:
+                return int(delta.total_seconds())
+            except AttributeError:
+                return int(total_seconds(delta))
+
+    def getlist(self, section, key):
+        return list(map(str.strip, self.get(section, key).split(',')))
+
+    def getiter(self, section, key):
+        for item in map(str.strip, self.get(section, key).split('\n')):
+            if item:
+                yield item
+
+    def section(self, section):
+        return Section(self, section)
+
+
+def new(options=None):
+
+    cp = IssoParser(allow_no_value=True)
+
+    if options:
+        cp.read_dict(options)
+
+    return cp
+
+
+def load(default, user=None):
+
+    # return set of (section, option)
+    setify = lambda cp: set((section, option) for section in cp.sections()
+                            for option in cp.options(section))
+
+    parser = new()
+    parser.read(default)
+
+    a = setify(parser)
+
+    if user:
+        parser.read(user)
+
+    for item in setify(parser).difference(a):
+        logger.warn("no such option: [%s] %s", *item)
+        if item in (("server", "host"), ("server", "port")):
+            logger.warn("use `listen = http://$host:$port` instead")
+        if item == ("smtp", "ssl"):
+            logger.warn("use `security = none | starttls | ssl` instead")
+        if item == ("general", "session-key"):
+            logger.info("Your `session-key` has been stored in the "
+                        "database itself, this option is now unused")
+
+    if not parseaddr(parser.get("smtp", "from"))[0]:
+        parser.set("smtp", "from",
+                   formataddr(("Ich schrei sonst!", parser.get("smtp", "from"))))
+
+    return parser

isso/core.py

@@ -2,15 +2,11 @@
 
 from __future__ import print_function
 
-import io
 import time
 import logging
 import threading
 import multiprocessing
 
-from email.utils import parseaddr, formataddr
-from configparser import ConfigParser
-
 try:
     import uwsgi
 except ImportError:
@@ -23,138 +19,11 @@ if PY2K:
 else:
     import _thread as thread
 
-from isso.utils import parse
-from isso.compat import text_type as str
-
 from werkzeug.contrib.cache import NullCache, SimpleCache
 
 logger = logging.getLogger("isso")
 
 
-class Section:
-
-    def __init__(self, conf, section):
-        self.conf = conf
-        self.section = section
-
-    def get(self, key):
-        return self.conf.get(self.section, key)
-
-    def getint(self, key):
-        return self.conf.getint(self.section, key)
-
-    def getlist(self, key):
-        return self.conf.getlist(self.section, key)
-
-    def getiter(self, key):
-        return self.conf.getiter(self.section, key)
-
-    def getboolean(self, key):
-        return self.conf.getboolean(self.section, key)
-
-
-class IssoParser(ConfigParser):
-    """
-    Extended :class:`ConfigParser` to parse human-readable timedeltas
-    into seconds and handles multiple values per key.
-
-    """
-
-    @classmethod
-    def _total_seconds(cls, td):
-        return (td.microseconds + (td.seconds + td.days * 24 * 3600) * 10**6) / 10**6
-
-    def getint(self, section, key):
-        try:
-            delta = parse.timedelta(self.get(section, key))
-        except ValueError:
-            return super(IssoParser, self).getint(section, key)
-        else:
-            try:
-                return int(delta.total_seconds())
-            except AttributeError:
-                return int(IssoParser._total_seconds(delta))
-
-    def getlist(self, section, key):
-        return list(map(str.strip, self.get(section, key).split(',')))
-
-    def getiter(self, section, key):
-        for item in map(str.strip, self.get(section, key).split('\n')):
-            if item:
-                yield item
-
-    def section(self, section):
-        return Section(self, section)
-
-
-class Config:
-
-    default = [
-        "[general]",
-        "name = ",
-        "dbpath = /tmp/isso.db",
-        "host = ",
-        "max-age = 15m",
-        "notify = stdout",
-        "[moderation]",
-        "enabled = false",
-        "purge-after = 30d",
-        "[server]",
-        "listen = http://localhost:8080/",
-        "reload = off", "profile = off",
-        "[smtp]",
-        "username = ", "password = ",
-        "host = localhost", "port = 587", "security = starttls",
-        "to = ", "from = ",
-        "timeout = 10",
-        "[guard]",
-        "enabled = true",
-        "ratelimit = 2",
-        "direct-reply = 3",
-        "reply-to-self = false",
-        "[markup]",
-        "options = strikethrough, autolink",
-        "allowed-elements = ",
-        "allowed-attributes = ",
-        "[hash]",
-        "algorithm = pbkdf2",
-        "salt = Eech7co8Ohloopo9Ol6baimi"
-    ]
-
-    @classmethod
-    def load(cls, configfile):
-
-        # return set of (section, option)
-        setify = lambda cp: set((section, option) for section in cp.sections()
-                                for option in cp.options(section))
-
-        rv = IssoParser(allow_no_value=True)
-        rv.read_file(io.StringIO(u'\n'.join(Config.default)))
-
-        a = setify(rv)
-
-        if configfile:
-            rv.read(configfile)
-
-        diff = setify(rv).difference(a)
-
-        if diff:
-            for item in diff:
-                logger.warn("no such option: [%s] %s", *item)
-                if item in (("server", "host"), ("server", "port")):
-                    logger.warn("use `listen = http://$host:$port` instead")
-                if item == ("smtp", "ssl"):
-                    logger.warn("use `security = none | starttls | ssl` instead")
-                if item == ("general", "session-key"):
-                    logger.info("Your `session-key` has been stored in the "
-                                "database itself, this option is now unused")
-
-        if not parseaddr(rv.get("smtp", "from"))[0]:
-            rv.set("smtp", "from", formataddr((u"Ich schrei sonst!", rv.get("smtp", "from"))))
-
-        return rv
-
-
 class Cache:
     """Wrapper around werkzeug's cache class, to make it compatible to
     uWSGI's cache framework.

isso/dispatch.py

@@ -9,8 +9,7 @@ from glob import glob
 from werkzeug.wsgi import DispatcherMiddleware
 from werkzeug.wrappers import Response
 
-from isso import make_app, wsgi
+from isso import dist, make_app, wsgi, config
-from isso.core import Config
 
 logger = logging.getLogger("isso")
 
@@ -21,11 +20,13 @@ class Dispatcher(DispatcherMiddleware):
     a relative URI, e.g. /foo.example and /other.bar.
     """
 
+    default = os.path.join(dist.location, "share", "isso.conf")
+
     def __init__(self, *confs):
 
         self.isso = {}
 
-        for i, conf in enumerate(map(Config.load, confs)):
+        for i, conf in enumerate(map(config.load(Dispatcher.default, conf))):
 
             if not conf.get("general", "name"):
                 logger.warn("unable to dispatch %r, no 'name' set", confs[i])

isso/run.py

@@ -3,7 +3,10 @@
 import os
 
 from isso import make_app
-from isso.core import Config
+from isso import dist, config
 
-application = make_app(Config.load(os.environ.get('ISSO_SETTINGS')),
+application = make_app(
-                       multiprocessing=True)
+    config.load(
+        os.path.join(dist.location, "share", "isso.conf"),
+        os.environ.get('ISSO_SETTINGS')),
+    multiprocessing=True)

isso/tests/fixtures.py

@@ -4,6 +4,7 @@ import json
 
 from werkzeug.test import Client
 
+
 class FakeIP(object):
 
     def __init__(self, app, ip):
@@ -35,5 +36,6 @@ class Dummy:
     def __exit__(self, exc_type, exc_val, exc_tb):
         pass
 
+
 curl = lambda method, host, path: Dummy()
 loads = lambda data: json.loads(data.decode('utf-8'))

isso/tests/test_comments.py

@@ -18,7 +18,7 @@ except ImportError:
 
 from werkzeug.wrappers import Response
 
-from isso import Isso, core
+from isso import Isso, core, config, dist
 from isso.utils import http
 from isso.views import comments
 
@@ -32,7 +32,7 @@ class TestComments(unittest.TestCase):
 
     def setUp(self):
         fd, self.path = tempfile.mkstemp()
-        conf = core.Config.load(None)
+        conf = config.load(os.path.join(dist.location, "share", "isso.conf"))
         conf.set("general", "dbpath", self.path)
         conf.set("guard", "enabled", "off")
         conf.set("hash", "algorithm", "none")
@@ -380,7 +380,7 @@ class TestModeratedComments(unittest.TestCase):
 
     def setUp(self):
         fd, self.path = tempfile.mkstemp()
-        conf = core.Config.load(None)
+        conf = config.load(os.path.join(dist.location, "share", "isso.conf"))
         conf.set("general", "dbpath", self.path)
         conf.set("moderation", "enabled", "true")
         conf.set("guard", "enabled", "off")
@@ -412,7 +412,7 @@ class TestPurgeComments(unittest.TestCase):
 
     def setUp(self):
         fd, self.path = tempfile.mkstemp()
-        conf = core.Config.load(None)
+        conf = config.load(os.path.join(dist.location, "share", "isso.conf"))
         conf.set("general", "dbpath", self.path)
         conf.set("moderation", "enabled", "true")
         conf.set("guard", "enabled", "off")

isso/tests/test_config.py

@@ -7,14 +7,14 @@ except ImportError:
 
 import io
 
-from isso import core
+from isso import config
 
 
 class TestConfig(unittest.TestCase):
 
     def test_parser(self):
 
-        parser = core.IssoParser(allow_no_value=True)
+        parser = config.IssoParser(allow_no_value=True)
         parser.read_file(io.StringIO(u"""
             [foo]
             bar = 1h

isso/tests/test_db.py

@@ -1,3 +1,4 @@
+# -*- encoding: utf-8 -*-
 
 try:
     import unittest2 as unittest
@@ -8,8 +9,8 @@ import os
 import sqlite3
 import tempfile
 
+from isso import config
 from isso.db import SQLite3
-from isso.core import Config
 
 from isso.compat import iteritems
 
@@ -24,14 +25,25 @@ class TestDBMigration(unittest.TestCase):
 
     def test_defaults(self):
 
-        db = SQLite3(self.path, Config.load(None))
+        conf = config.new({
+            "general": {
+                "dbpath": "/dev/null",
+                "max-age": "1h"
+            }
+        })
+        db = SQLite3(self.path, conf)
 
         self.assertEqual(db.version, SQLite3.MAX_VERSION)
         self.assertTrue(db.preferences.get("session-key", "").isalnum())
 
     def test_session_key_migration(self):
 
-        conf = Config.load(None)
+        conf = config.new({
+            "general": {
+                "dbpath": "/dev/null",
+                "max-age": "1h"
+            }
+        })
         conf.set("general", "session-key", "supersecretkey")
 
         with sqlite3.connect(self.path) as con:
@@ -88,7 +100,12 @@ class TestDBMigration(unittest.TestCase):
                             "   tid, parent, created)"
                             "VALUEs (?, ?, ?)", (id, parent, id))
 
-        conf = Config.load(None)
+        conf = config.new({
+            "general": {
+                "dbpath": "/dev/null",
+                "max-age": "1h"
+            }
+        })
         SQLite3(self.path, conf)
 
         flattened = [

isso/tests/test_guard.py

@@ -7,6 +7,7 @@ try:
 except ImportError:
     import unittest
 
+import os
 import json
 import tempfile
 
@@ -14,7 +15,7 @@ from werkzeug import __version__
 from werkzeug.test import Client
 from werkzeug.wrappers import Response
 
-from isso import Isso, core
+from isso import Isso, config, core, dist
 from isso.utils import http
 
 from fixtures import curl, FakeIP
@@ -36,7 +37,7 @@ class TestGuard(unittest.TestCase):
 
     def makeClient(self, ip, ratelimit=2, direct_reply=3, self_reply=False):
 
-        conf = core.Config.load(None)
+        conf = config.load(os.path.join(dist.location, "share", "isso.conf"))
         conf.set("general", "dbpath", self.path)
         conf.set("hash", "algorithm", "none")
         conf.set("guard", "enabled", "true")

isso/tests/test_html.py

@@ -1,3 +1,4 @@
+# -*- encoding: utf-8 -*-
 
 try:
     import unittest2 as unittest
@@ -5,7 +6,7 @@ except ImportError:
     import unittest
 
 
-from isso.core import Config
+from isso import config
 from isso.utils import html
 
 
@@ -54,7 +55,13 @@ class TestHTML(unittest.TestCase):
             self.assertEqual(html.sanitize(sanitizer, input), expected)
 
     def test_render(self):
-        conf = Config.load(None).section("markup")
+        conf = config.new({
-        renderer = html.Markup(conf).render
+            "markup": {
+                "options": "autolink",
+                "allowed-elements": "",
+                "allowed-attributes": ""
+            }
+        })
+        renderer = html.Markup(conf.section("markup")).render
         self.assertEqual(renderer("http://example.org/ and sms:+1234567890"),
                          '<p><a href="http://example.org/">http://example.org/</a> and sms:+1234567890</p>')

isso/tests/test_migration.py

@@ -10,11 +10,18 @@ except ImportError:
 import tempfile
 from os.path import join, dirname
 
-from isso.core import Config
+from isso import config
 
 from isso.db import SQLite3
 from isso.migrate import Disqus, WordPress, autodetect
 
+conf = config.new({
+    "general": {
+        "dbpath": "/dev/null",
+        "max-age": "1h"
+    }
+})
+
 
 class TestMigration(unittest.TestCase):
 
@@ -23,7 +30,7 @@ class TestMigration(unittest.TestCase):
         xml = join(dirname(__file__), "disqus.xml")
         xxx = tempfile.NamedTemporaryFile()
 
-        db = SQLite3(xxx.name, Config.load(None))
+        db = SQLite3(xxx.name, conf)
         Disqus(db, xml).migrate()
 
         self.assertEqual(len(db.execute("SELECT id FROM comments").fetchall()), 2)
@@ -45,7 +52,7 @@ class TestMigration(unittest.TestCase):
         xml = join(dirname(__file__), "wordpress.xml")
         xxx = tempfile.NamedTemporaryFile()
 
-        db = SQLite3(xxx.name, Config.load(None))
+        db = SQLite3(xxx.name, conf)
         WordPress(db, xml).migrate()
 
         self.assertEqual(db.threads["/2014/test/"]["title"], "Hello, World…")

isso/tests/test_utils_hash.py

@@ -7,8 +7,9 @@ try:
 except ImportError:
     import unittest
 
+from isso import config
+
 from isso.compat import PY2K, string_types
-from isso.core import Config
 from isso.utils.hash import Hash, PBKDF2, new
 
 
@@ -46,15 +47,15 @@ class TestPBKDF2(unittest.TestCase):
 
 class TestCreate(unittest.TestCase):
 
-    def test_default(self):
-        conf = Config.load(None)
-        self.assertIsInstance(new(conf.section("hash")), PBKDF2)
-
     def test_custom(self):
 
         def _new(val):
-            conf = Config.load(None)
+            conf = config.new({
-            conf.set("hash", "algorithm", val)
+                "hash": {
+                    "algorithm": val,
+                    "salt": ""
+                }
+            })
             return new(conf.section("hash"))
 
         sha1 = _new("sha1")

isso/tests/test_vote.py

@@ -1,6 +1,7 @@
 
 from __future__ import unicode_literals
 
+import os
 import json
 import tempfile
 
@@ -11,7 +12,7 @@ except ImportError:
 
 from werkzeug.wrappers import Response
 
-from isso import Isso, core
+from isso import Isso, core, config, dist
 from isso.utils import http
 
 from fixtures import curl, loads, FakeIP, JSONClient
@@ -25,7 +26,7 @@ class TestVote(unittest.TestCase):
 
     def makeClient(self, ip):
 
-        conf = core.Config.load(None)
+        conf = config.load(os.path.join(dist.location, "share", "isso.conf"))
         conf.set("general", "dbpath", self.path)
         conf.set("guard", "enabled", "off")
         conf.set("hash", "algorithm", "none")

isso/utils/parse.py

@@ -1,11 +1,8 @@
 
 from __future__ import print_function
 
-import datetime
 from itertools import chain
 
-import re
-
 
 try:
     from urllib import unquote
@@ -21,37 +18,6 @@ if PY2K:  # http://bugs.python.org/issue12984
     NamedNodeMap.__contains__ = lambda self, key: self.has_key(key)
 
 
-def timedelta(value):
-    """
-    Parse :param value: into :class:`datetime.timedelta`, you can use any (logical)
-    combination of Nw, Nd, Nh and Nm, e.g. `1h30m` for 1 hour, 30 minutes or `3w` for
-    3 weeks. Raises a ValueError if the input is invalid/unparseable.
-
-    >>> print(timedelta("3w"))
-    21 days, 0:00:00
-    >>> print(timedelta("3w 12h 57m"))
-    21 days, 12:57:00
-    >>> print(timedelta("1h30m37s"))
-    1:30:37
-    >>> print(timedelta("1asdf3w"))
-    Traceback (most recent call last):
-        ...
-    ValueError: invalid human-readable timedelta
-    """
-
-    keys = ["weeks", "days", "hours", "minutes", "seconds"]
-    regex = "".join(["((?P<%s>\d+)%s ?)?" % (k, k[0]) for k in keys])
-    kwargs = {}
-    for k, v in re.match(regex, value).groupdict(default="0").items():
-        kwargs[k] = int(v)
-
-    rv = datetime.timedelta(**kwargs)
-    if rv == datetime.timedelta():
-        raise ValueError("invalid human-readable timedelta")
-    return datetime.timedelta(**kwargs)
-
-
-
 def thread(data, default=u"Untitled.", id=None):
     """
     Extract <h1> title from web page. The title is *probably* the text node,

isso/wsgi.py

@@ -56,8 +56,8 @@ def urlsplit(name):
 
     rv = urlparse(name)
     if rv.scheme == 'https' and rv.port is None:
-        return (rv.netloc, 443, True)
+        return rv.netloc, 443, True
-    return (rv.netloc.rsplit(':')[0], rv.port or 80, rv.scheme == 'https')
+    return rv.netloc.rsplit(':')[0], rv.port or 80, rv.scheme == 'https'
 
 
 def urljoin(netloc, port, ssl):
@@ -120,7 +120,7 @@ class CORSMiddleware(object):
 
     methods = ("HEAD", "GET", "POST", "PUT", "DELETE")
 
-    def __init__(self, app, origin, allowed=[], exposed=[]):
+    def __init__(self, app, origin, allowed=None, exposed=None):
         self.app = app
         self.origin = origin
         self.allowed = allowed

share/isso.conf

@@ -18,10 +18,13 @@ name =
 # your website, you have to "whitelist" your website(s).
 #
 # I recommend the first value to be a non-SSL website that is used as fallback
-# if Firefox users (and only those) supress their HTTP referer completely.
+# if Firefox users (and only those) supress their HTTP referer completely:
+#
+# host =
+#     http://example.tld/
+#     https://example.tld/
+#
 host =
-    http://example.tld/
-    https://example.tld/
 
 # time range that allows users to edit/remove their own comments.
 # It supports years, weeks, days, hours, minutes, seconds.
@@ -38,7 +41,7 @@ max-age = 15m
 # smtp
 #     Send notifications via SMTP on new comments with activation (if
 #     moderated) and deletion links.
-notify =
+notify = stdout
 
 
 [moderation]