Update to use Twisted and python 3.7 (#42)

* Reformat code using `black`

* Initial work on async version of Sygnal, with GCM support.

* Address some points mentioned during code review.

* Eliminate asyncio

* Deal with errors by reporting 500 or 502 as appropriate when things go wrong.

* Start writing tests (non-functional for now)

* Add GCM tests and do some clean-up and fixes.

* Start APNs implementation and miscellaneous clean-ups.

* Configure flake8 to match black and PEP8.

* Small cosmetic changes.

* Add docstring for Database.query

* Configuration improvements

* Add docstring for twisted_sleep

* Update tests for new configuration system

* Add APNs support and tests

* Add sample YAML config file

* Add service_identity dependency for proper certificate validation in Twisted.

* Complete APNs authentication support (oops)

* Black codebase again

* Remove obsolete TODOs

* Black again

* Add Prometheus support

* Fix startup issues and fix pushkin startup errors causing hanging

* Add Prometheus metrics for GCM and the Push Gateway API

* (Cosmetic) Minor tidying. Black.

* Working GCM demo! More fixes to come!

* Fix autoformat gone wrong in sample config.

* Collect more into OpenTracing and Prometheus

* Add docstrings

* Handle missing config file

* Add docstrings to tests

* Fixes tests

* Add Sentry integration

* Add Buildkite CI

* Fix flake8 code style/lint issues

* Check excess config keys in the new metrics sections.

* Clean up logging, particularly regarding exceptions

* Remove pushkey from OpenTracing as it is relatively sensitive
because it could be used to send a user notifications.

* Use request ID-derived notification IDs in APNs to reduce confusion
in the logs.

* Improve logging.

* Better describe some APNs Truncation tests.

* Add overlooked config field to sample configuration.

* Update .gitignore for PyCharm and Twisted Trial

* Install asyncio reactor because twisted.web.client requires a reactor
to be installed.

* Improve logging

* Make reactor a named argument to be more explicit.

* Update README

* Minor style tweaks

* Style format

* Install reactor only in main as otherwise it disrupts tests

* Update copyright in utils.py

* Remove DummyPushkin as it was only for development.

* Solve some minor TODOs and remove other obsolete/invalid TODOs.

* Remove obsolete Gunicorn config

* Add capability for access logging

* Fix tests

* Generate fresh UUIDs for APNs notifications as they are not just opaque
strings.

* APNs expects UUIDs in hex, not base64 as received.

* Add logging on successful deliveries

* Use Python's logging dict configuration for more flexibility.

* Remove obsolete `shutdown` methods.

Signed-off-by: reivilibre <[email protected]>

* Follow convention of explicitly inheriting from `object`.

Signed-off-by: reivilibre <[email protected]>

* Fold _setup into constructor as no need to separate.

Signed-off-by: reivilibre <[email protected]>

* Remove Pushkin `start` method and make `Sygnal.run` clearer.

Signed-off-by: reivilibre <[email protected]>

* Remove obsolete TODOs.

Signed-off-by: reivilibre <[email protected]>

* Style fixes

Signed-off-by: reivilibre <[email protected]>

* Remove `Database` in favour of Twisted's adbapi ConnectionPool.

Signed-off-by: reivilibre <[email protected]>

* Pull `dispatch_request` out of `dispatch_notification`.

Signed-off-by: reivilibre <[email protected]>

* Docstrings for `CanonicalRegIdStore`.

Signed-off-by: reivilibre <[email protected]>

.buildkite/pipeline.yml

@@ -0,0 +1,27 @@
+
+steps:
+  - command:
+      - "python -m pip install black"
+      - "black --check ."
+    label: "Check Code Formatting"
+    plugins:
+      - docker#v3.0.1:
+          image: "python:3.7"
+
+  - command:
+      - "python -m pip install flake8"
+      - "flake8 ."
+    label: "Check Code Style"
+    plugins:
+      - docker#v3.0.1:
+          image: "python:3.7"
+
+  - wait
+
+  - command:
+      - "python -m pip install -e ."
+      - "trial tests"
+    label: "Run unit tests"
+    plugins:
+      - docker#v3.0.1:
+          image: "python:3.7"

.gitignore

@@ -1,6 +1,9 @@
 *.pyc
 sygnal.conf
+sygnal.yaml
 gunicorn_config.py
 var/
 sygnal.pid
 sygnal.db
+/_trial_temp*
+/.idea

README.rst

@@ -1,90 +1,73 @@
 Introduction
 ============
 
-sygnal is a reference Push Gateway for Matrix (http://matrix.org/).
+Sygnal is a reference Push Gateway for `Matrix <https://matrix.org/>`_.
 
-See
-http://matrix.org/docs/spec/client_server/r0.2.0.html#id51 for a high level overview of how notifications work in Matrix.
+See https://matrix.org/docs/spec/client_server/r0.5.0#id134
+for a high level overview of how notifications work in Matrix.
 
-http://matrix.org/docs/spec/push_gateway/unstable.html#post-matrix-push-r0-notify
-describes the protocol that Matrix Home Servers use to send notifications to
-Push Gateways such as sygnal.
+https://matrix.org/docs/spec/push_gateway/r0.1.0
+describes the protocol that Matrix Home Servers use to send notifications to Push Gateways such as Sygnal.
 
 Setup
 =====
-sygnal is a plain WSGI app, although these instructions use gunicorn which
-will create a complete, standalone webserver.  When used with gunicorn,
-sygnal can use gunicorn's extra hook to perform a clean shutdown which tries as
-hard as possible to ensure no messages are lost.
+Sygnal is configured through a YAML configuration file.
+By default, this configuration file is assumed to be named ``sygnal.yaml`` and to be in the working directory.
+To change this, set the ``SYGNAL_CONF`` environment variable to the path to your configuration file.
+A sample configuration file is provided in this repository;
+see ``sygnal.yaml.sample``.
 
-There are two config files:
- * sygnal.conf (The app-specific config file)
- * gunicorn_config.py (gunicorn's config file)
+The `apps:` section is where you set up different apps that are to be handled.
+Each app should be given its own subsection, with the key of that subsection being the app's ``app_id``.
+Keys in this section take the form of the ``app_id``, as specified when setting up a Matrix pusher
+(see https://matrix.org/docs/spec/client_server/r0.5.0#post-matrix-client-r0-pushers-set).
 
-sygnal.conf contains configuration for sygnal itself. This includes the location
-and level of sygnal's log file. The [apps] section is where you set up different
-apps that are to be handled. Keys in this section take the form of the app_id
-and the name of the configuration key, joined by a single dot ('.'). The app_id
-is as specified when setting up a Matrix pusher (see
-http://matrix.org/docs/spec/client_server/r0.2.0.html#post-matrix-client-r0-pushers-set). So for example, the `type` for
-the App ID of `com.example.myapp.ios.prod` would be specified as follows::
-
-  com.example.myapp.ios.prod.type = foobar
-
-By default sygnal.conf is assumed to be in the working directory, but the path
-can be overriden by setting the `sygnal.conf` environment variable.
-
-The gunicorn sample config contains everything necessary to run sygnal from
-gunicorn. The shutdown hook handles clean shutdown. You can customise other
-aspects of this file as you wish to change, for example, the log location or the
-bind port.
-
-Note that sygnal uses gevent. You should therefore not change the worker class
-or the number of workers (which should be 1: in gevent, a single worker uses
-multiple greenlets to handle all the requests).
+See the sample configuration for examples.
 
 App Types
 ---------
 There are two supported App Types:
 
 apns
   This sends push notifications to iOS apps via the Apple Push Notification
-  Service (APNS). It expects the 'certfile' parameter to be a path relative to
-  sygnal's working directory of a PEM file containing the APNS certificate and
-  unencrypted private key.
+  Service (APNS).
 
-gcm
-  This sends messages via Google Cloud Messaging (GCM) and hence can be used
-  to deliver notifications to Android apps. It expects the 'apiKey' parameter
-  to contain the secret GCM key.
+  Expected configuration depends on which kind of authentication you wish to use:
 
-Running
-=======
-To run with gunicorn:
+  |
+
+  For certificate-based authentication:
+    It expects:
 
-gunicorn -c gunicorn_config.py sygnal:app
+    * the ``certfile`` parameter to be a path relative to
+      sygnal's working directory of a PEM file containing the APNS certificate and
+      unencrypted private key.
 
-You can customise the gunicorn_config.py to determine whether this daemonizes or runs in the foreground.
+  For token-based authentication:
+    It expects:
 
-Gunicorn maintains its own logging in addition to the app's, so the access_log
-and error_log contain gunicorn's accesses and gunicorn specific errors. The log
-file in sygnal.conf contains app level logging.
+    * the 'keyfile' parameter to be a path relative to Sygnal's working directory of a p8 file
+    * the 'key_id' parameter
+    * the 'team_id' parameter
+    * the 'topic' parameter
 
-Clean shutdown
-==============
-The code for APNS uses a grace period where it waits for errors to come down the
-socket before declaring it safe for the app to shut down (due to the design of
-APNS). Terminating using SIGTERM performs a clean shutdown::
+gcm
+  This sends messages via Google/Firebase Cloud Messaging (GCM/FCM) and hence can be used
+  to deliver notifications to Android apps. It expects the 'api_key' parameter
+  to contain the 'Server key', which can be acquired from Firebase Console at:
+  ``https://console.firebase.google.com/project/<PROJECT NAME>/settings/cloudmessaging/``
 
-    kill -TERM `cat sygnal.pid`
+Running
+=======
 
-Restarting sygnal using SIGHUP will handle this gracefully::
+``python -m sygnal.sygnal``
 
-    kill -HUP `cat sygnal.pid`
+Python 3.7 or higher is required.
 
 Log Rotation
 ============
-Gunicorn appends to files but does not use a rotating logger.
-Sygnal's app logging does the same. Gunicorn will re-open all log files
-(including the app's) when sent SIGUSR1.  The recommended configuration is
-therefore to use logrotate.
+Sygnal's logging appends to files but does not use a rotating logger.
+The recommended configuration is therefore to use ``logrotate``.
+The log file will be automatically reopened if the log file changes, for example
+due to ``logrotate``.
+

setup.cfg

@@ -1,3 +1,15 @@
+[flake8]
+# line length defaulted to by black
+max-line-length = 88
+
+# see https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes
+# for error codes. The ones we ignore are:
+#  W503: line break before binary operator
+#  W504: line break after binary operator
+#  E203: whitespace before ':' (which is contrary to pep8?)
+# (this is a subset of those ignored in Synapse)
+ignore=W503,W504,E203
+
 [isort]
 line_length = 80
 not_skip = __init__.py

setup.py

@@ -2,6 +2,7 @@
 
 # Copyright 2014 OpenMarket Ltd
 # Copyright 2017 Vector Creations Ltd
+# Copyright 2019 The Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -16,6 +17,7 @@
 # limitations under the License.
 
 import os
+
 from setuptools import setup, find_packages
 
 
@@ -26,18 +28,20 @@
 def read(fname):
     return open(os.path.join(os.path.dirname(__file__), fname)).read()
 
+
 setup(
     name="matrix-sygnal",
     version=read("VERSION").strip(),
     packages=find_packages(exclude=["tests", "tests.*"]),
     description="Reference Push Gateway for Matrix Notifications",
     install_requires=[
-        "flask>=1.0.2",
-        "gevent>=1.0.1",
-        "pushbaby>=0.0.9",
-        "grequests",
-        "six",
-        "prometheus_client>=0.7.0,<0.8"
+        "Twisted>=19.2.1",
+        "prometheus_client>=0.7.0,<0.8",
+        "aioapns>=1.7",
+        "pyyaml>=5.1.1",
+        "service_identity>=18.1.0",
+        "jaeger-client>=4.0.0",
+        "opentracing>=2.2.0",
     ],
     long_description=read("README.rst"),
 )