From b59aa745560608c8503421bd9542c99fc1c571b5 Mon Sep 17 00:00:00 2001
From: Kegan Dougal <kegan@matrix.org>
Date: Tue, 31 Mar 2015 11:35:45 +0100
Subject: [PATCH] Fix tests and missing returns on deferreds.

---
 synapse/appservice/__init__.py   |  2 +-
 synapse/storage/appservice.py    | 18 ++++++++------
 tests/storage/test_appservice.py | 41 ++++++++++++++++++++++----------
 3 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index c60db16b7..a8108c1ef 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -95,7 +95,7 @@ class ApplicationService(object):
         #   rooms: [ {regex: "[A-z]+.*", exclusive: true}, ...],
         # }
         if not namespaces:
-            return None
+            namespaces = {}
 
         for ns in ApplicationService.NS_LIST:
             if ns not in namespaces:
diff --git a/synapse/storage/appservice.py b/synapse/storage/appservice.py
index fe9372a7c..a520a859d 100644
--- a/synapse/storage/appservice.py
+++ b/synapse/storage/appservice.py
@@ -37,7 +37,7 @@ class ApplicationServiceStore(SQLBaseStore):
         )
 
     def get_app_services(self):
-        defer.succeed(self.services_cache)
+        return defer.succeed(self.services_cache)
 
     def get_app_service_by_user_id(self, user_id):
         """Retrieve an application service from their user ID.
@@ -54,9 +54,8 @@ class ApplicationServiceStore(SQLBaseStore):
         """
         for service in self.services_cache:
             if service.sender == user_id:
-                defer.succeed(service)
-                return
-        defer.succeed(None)
+                return defer.succeed(service)
+        return defer.succeed(None)
 
     def get_app_service_by_token(self, token):
         """Get the application service with the given appservice token.
@@ -69,7 +68,7 @@ class ApplicationServiceStore(SQLBaseStore):
         for service in self.services_cache:
             if service.token == token:
                 return defer.succeed(service)
-        defer.succeed(None)
+        return defer.succeed(None)
 
     def get_app_service_rooms(self, service):
         """Get a list of RoomsForUser for this application service.
@@ -237,11 +236,16 @@ class ApplicationServiceStore(SQLBaseStore):
 
     def _populate_appservice_cache(self, config_files):
         """Populates a cache of Application Services from the config files."""
+        if not isinstance(config_files, list):
+            logger.warning(
+                "Expected %s to be a list of AS config files.", config_files
+            )
+            return
+
         for config_file in config_files:
             try:
                 with open(config_file, 'r') as f:
-                    as_info = yaml.load(f)
-                    appservice = self._load_appservice(as_info)
+                    appservice = self._load_appservice(yaml.load(f))
                     logger.info("Loaded application service: %s", appservice)
                     self.services_cache.append(appservice)
             except Exception as e:
diff --git a/tests/storage/test_appservice.py b/tests/storage/test_appservice.py
index 82bfea15a..b856438fd 100644
--- a/tests/storage/test_appservice.py
+++ b/tests/storage/test_appservice.py
@@ -22,6 +22,8 @@ from synapse.storage.appservice import (
 )
 
 import json
+import os
+import yaml
 from mock import Mock
 from tests.utils import SQLiteMemoryDbPool, MockClock
 
@@ -30,25 +32,40 @@ class ApplicationServiceStoreTestCase(unittest.TestCase):
 
     @defer.inlineCallbacks
     def setUp(self):
+        self.as_yaml_files = []
         db_pool = SQLiteMemoryDbPool()
         yield db_pool.prepare()
         hs = HomeServer(
-            "test", db_pool=db_pool, clock=MockClock(), config=Mock()
+            "test", db_pool=db_pool, clock=MockClock(),
+            config=Mock(
+                app_service_config_files=self.as_yaml_files
+            )
         )
+
         self.as_token = "token1"
-        db_pool.runQuery(
-            "INSERT INTO application_services(token) VALUES(?)",
-            (self.as_token,)
-        )
-        db_pool.runQuery(
-            "INSERT INTO application_services(token) VALUES(?)", ("token2",)
-        )
-        db_pool.runQuery(
-            "INSERT INTO application_services(token) VALUES(?)", ("token3",)
-        )
+        self.as_url = "some_url"
+        self._add_appservice(self.as_token, self.as_url, "some_hs_token", "bob")
+        self._add_appservice("token2", "some_url", "some_hs_token", "bob")
+        self._add_appservice("token3", "some_url", "some_hs_token", "bob")
         # must be done after inserts
         self.store = ApplicationServiceStore(hs)
 
+    def tearDown(self):
+        # TODO: suboptimal that we need to create files for tests!
+        for f in self.as_yaml_files:
+            try:
+                os.remove(f)
+            except:
+                pass
+
+    def _add_appservice(self, as_token, url, hs_token, sender):
+        as_yaml = dict(url=url, as_token=as_token, hs_token=hs_token,
+                       sender=sender, namespaces={})
+        # use the token as the filename
+        with open(as_token, 'w') as outfile:
+            outfile.write(yaml.dump(as_yaml))
+            self.as_yaml_files.append(as_token)
+
     @defer.inlineCallbacks
     def test_retrieve_unknown_service_token(self):
         service = yield self.store.get_app_service_by_token("invalid_token")
@@ -60,7 +77,7 @@ class ApplicationServiceStoreTestCase(unittest.TestCase):
             self.as_token
         )
         self.assertEquals(stored_service.token, self.as_token)
-        self.assertEquals(stored_service.url, None)
+        self.assertEquals(stored_service.url, self.as_url)
         self.assertEquals(
             stored_service.namespaces[ApplicationService.NS_ALIASES],
             []