Merge ~andreserl/maas:2.2_lp1507712 into maas:2.2

[Andres Rodriguez]

Backport dd962958f73efb95216044f93de9aea3aace01e8 from master.

LP: #1507712 - cli: maas logout causes KeyError for other profiles

There is a race where when profiles are being deleted, if this happens
between the __iter__ and __getitem__ (of a different invocation of the
cli) which are two different SQL queries, there is an inconsistent view.

So instead use a cache with a consistent view of the data. This
guarantees correct -- if stale behavior in the cli.

Note that this is acceptable because when running two commands at the
same time ordering is unknown and the cache only lives for the lifetime
of a single command.

[Andres Rodriguez (andreserl)]

selfie as it is a backport!

review: Approve

diff --git a/src/maascli/config.py b/src/maascli/config.py
index e3145dd..129e944 100644
--- a/src/maascli/config.py
+++ b/src/maascli/config.py
@@ -24,23 +24,41 @@ class ProfileConfig:
 
     def __init__(self, database):
         self.database = database
+        self.cache = {}
         with self.cursor() as cursor:
             cursor.execute(
                 "CREATE TABLE IF NOT EXISTS profiles "
                 "(id INTEGER PRIMARY KEY,"
                 " name TEXT NOT NULL UNIQUE,"
                 " data BLOB)")
+        self.__fill_cache()
 
     def cursor(self):
         return closing(self.database.cursor())
 
+    def __fill_cache(self):
+        """Touch each entry in the database to fill the cache. This cache is
+        needed to enforce a consistent view. Without it, the list of items can
+        be out of sync with the items actually in the database leading to
+        KeyErrors when traversing the profiles.
+        """
+        for name in self:
+            try:
+                self[name]
+            except KeyError:
+                pass
+
     def __iter__(self):
+        if self.cache:
+            return (name for name in self.cache)
         with self.cursor() as cursor:
             results = cursor.execute(
                 "SELECT name FROM profiles").fetchall()
         return (name for (name,) in results)
 
     def __getitem__(self, name):
+        if name in self.cache:
+            return self.cache[name]
         with self.cursor() as cursor:
             data = cursor.execute(
                 "SELECT data FROM profiles"
@@ -48,19 +66,26 @@ class ProfileConfig:
         if data is None:
             raise KeyError(name)
         else:
-            return json.loads(data[0])
+            info = json.loads(data[0])
+            self.cache[name] = info
+            return info
 
     def __setitem__(self, name, data):
         with self.cursor() as cursor:
             cursor.execute(
                 "INSERT OR REPLACE INTO profiles (name, data) "
                 "VALUES (?, ?)", (name, json.dumps(data)))
+        self.cache[name] = data
 
     def __delitem__(self, name):
         with self.cursor() as cursor:
             cursor.execute(
                 "DELETE FROM profiles"
                 " WHERE name = ?", (name,))
+        try:
+            del self.cache[name]
+        except KeyError:
+            pass
 
     @classmethod
     def create_database(cls, dbpath):
diff --git a/src/maascli/tests/test_config.py b/src/maascli/tests/test_config.py
index 5500a91..d83211c 100644
--- a/src/maascli/tests/test_config.py
+++ b/src/maascli/tests/test_config.py
@@ -9,7 +9,10 @@ import contextlib
 from contextlib import contextmanager
 import os.path
 import sqlite3
-from unittest.mock import call
+from unittest.mock import (
+    call,
+    patch,
+)
 
 from maascli import (
     api,
@@ -59,6 +62,14 @@ class TestProfileConfig(MAASTestCase):
         self.assertEqual({"alice"}, set(config))
         self.assertEqual({"def": 456}, config["alice"])
 
+    def test_cache(self):
+        database = sqlite3.connect(":memory:")
+        config = api.ProfileConfig(database)
+        config["alice"] = {"abc": 123}
+        with patch.object(config, 'cursor') as cursor:
+            self.assertEqual({"abc": 123}, config["alice"])
+            cursor.assert_not_called()
+
     def test_getting_profile(self):
         database = sqlite3.connect(":memory:")
         config = api.ProfileConfig(database)