Patchwork [v9,RFC] scmutil: add a simple key-value file helper

login
register
mail settings
Submitter Kostia Balytskyi
Date Jan. 19, 2017, 11:55 a.m.
Message ID <19a449c91ef14e691cf1.1484826927@devvm1416.lla2.facebook.com>
Download mbox | patch
Permalink /patch/18249/
State Deferred
Headers show

Comments

Kostia Balytskyi - Jan. 19, 2017, 11:55 a.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1484824655 28800
#      Thu Jan 19 03:17:35 2017 -0800
# Node ID 19a449c91ef14e691cf1347748473e0094fedc86
# Parent  9f264adbe75bfae8551dc0e6e0fce8d43fc7b43a
scmutil: add a simple key-value file helper

The purpose of the added class is to serve purposes like save files of shelve
or state files of shelve, rebase and histedit. Keys of these files can be
alphanumeric and start with letters, while values must not contain newlines.
Keys which start with an uppercase letter are required, while other keys
are optional.

In light of Mercurial's reluctancy to use Python's json module, this tries
to provide a reasonable alternative for a non-nested named data.
Comparing to current approach of storing state in plain text files, where
semantic meaning of lines of text is only determined by their oreder,
simple key-value file allows for reordering lines and thus helps handle
optional values.

Initial use-case I see for this is obs-shelve's shelve files. Later we
can possibly migrate state files to this approach.

The test is in a new file beause I did not figure out where to put it
within existing test suite. If you give me a better idea, I will gladly
follow it.
Sean Farley - Jan. 19, 2017, 7:26 p.m.
Kostia Balytskyi <ikostia@fb.com> writes:

> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1484824655 28800
> #      Thu Jan 19 03:17:35 2017 -0800
> # Node ID 19a449c91ef14e691cf1347748473e0094fedc86
> # Parent  9f264adbe75bfae8551dc0e6e0fce8d43fc7b43a
> scmutil: add a simple key-value file helper
>
> The purpose of the added class is to serve purposes like save files of shelve
> or state files of shelve, rebase and histedit. Keys of these files can be
> alphanumeric and start with letters, while values must not contain newlines.
> Keys which start with an uppercase letter are required, while other keys
> are optional.
>
> In light of Mercurial's reluctancy to use Python's json module, this tries
> to provide a reasonable alternative for a non-nested named data.
> Comparing to current approach of storing state in plain text files, where
> semantic meaning of lines of text is only determined by their oreder,
> simple key-value file allows for reordering lines and thus helps handle
> optional values.
>
> Initial use-case I see for this is obs-shelve's shelve files. Later we
> can possibly migrate state files to this approach.
>
> The test is in a new file beause I did not figure out where to put it
> within existing test suite. If you give me a better idea, I will gladly
> follow it.

Let's hold off until the freeze is over.

Patch

diff --git a/mercurial/error.py b/mercurial/error.py
--- a/mercurial/error.py
+++ b/mercurial/error.py
@@ -246,3 +246,6 @@  class UnsupportedBundleSpecification(Exc
 
 class CorruptedState(Exception):
     """error raised when a command is not able to read its state from file"""
+
+class MissingRequiredKey(Exception):
+    """error raised when simple key-value file misses a required key"""
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -1571,3 +1571,51 @@  class checkambigatclosing(closewrapbase)
     def close(self):
         self._origfh.close()
         self._checkambig()
+
+class simplekeyvaluefile(object):
+    """A simple file with key=value lines
+
+    Keys must be alphanumerics and start with a letter, values must not
+    contain '\n' characters"""
+
+    # if KEYS is non-empty, read values are validated against it:
+    # each key is a tuple (keyname, required)
+    KEYS = []
+
+    def __init__(self, vfs, path):
+        self.vfs = vfs
+        self.path = path
+
+    def validate(self, d):
+        for key, req in self.KEYS:
+            if req and key not in d:
+                e = "missing a required key: '%s'" % key
+                raise error.MissingRequiredKey(e)
+
+    def read(self):
+        lines = self.vfs.readlines(self.path)
+        try:
+            d = dict(line[:-1].split('=', 1) for line in lines if line)
+        except ValueError as e:
+            raise error.CorruptedState(str(e))
+        self.validate(d)
+        return d
+
+    def write(self, data):
+        """Write key=>value mapping to a file
+        data is a dict. Keys must be alphanumerical and start with a letter.
+        Values must not contain newline characters."""
+        lines = []
+        for k, v in data.items():
+            if not k[0].isalpha():
+                e = "keys must start with a letter in a key-value file"
+                raise error.ProgrammingError(e)
+            if not k.isalnum():
+                e = "invalid key name in a simple key-value file"
+                raise error.ProgrammingError(e)
+            if '\n' in v:
+                e = "invalid value in a simple key-value file"
+                raise error.ProgrammingError(e)
+            lines.append("%s=%s\n" % (k, v))
+        with self.vfs(self.path, mode='wb', atomictemp=True) as fp:
+            fp.write(''.join(lines))
diff --git a/tests/test-simplekeyvaluefile.py b/tests/test-simplekeyvaluefile.py
new file mode 100644
--- /dev/null
+++ b/tests/test-simplekeyvaluefile.py
@@ -0,0 +1,87 @@ 
+from __future__ import absolute_import
+
+import unittest
+import silenttestrunner
+
+from mercurial import (
+    error,
+    scmutil,
+)
+
+contents = {}
+
+class fileobj(object):
+    def __init__(self, name):
+        self.name = name
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *args, **kwargs):
+        pass
+
+    def write(self, text):
+        contents[self.name] = text
+
+    def read(self):
+        return contents[self.name]
+
+class mockvfs(object):
+    def read(self, path):
+        return fileobj(path).read()
+
+    def readlines(self, path):
+        return fileobj(path).read().split('\n')
+
+    def __call__(self, path, mode, atomictemp):
+        return fileobj(path)
+
+class testsimplekeyvaluefile(unittest.TestCase):
+    def setUp(self):
+        self.vfs = mockvfs()
+
+    def testbasicwriting(self):
+        d = {'key1': 'value1', 'Key2': 'value2'}
+        scmutil.simplekeyvaluefile(self.vfs, 'kvfile').write(d)
+        self.assertEqual(sorted(self.vfs.read('kvfile').split('\n')),
+                         ['', 'Key2=value2', 'key1=value1'])
+
+    def testinvalidkeys(self):
+        d = {'0key1': 'value1', 'Key2': 'value2'}
+        with self.assertRaisesRegexp(error.ProgrammingError,
+                                     "keys must start with a letter.*"):
+            scmutil.simplekeyvaluefile(self.vfs, 'kvfile').write(d)
+        d = {'key1@': 'value1', 'Key2': 'value2'}
+        with self.assertRaisesRegexp(error.ProgrammingError, "invalid key.*"):
+            scmutil.simplekeyvaluefile(self.vfs, 'kvfile').write(d)
+
+    def testinvalidvalues(self):
+        d = {'key1': 'value1', 'Key2': 'value2\n'}
+        with self.assertRaisesRegexp(error.ProgrammingError, "invalid val.*"):
+            scmutil.simplekeyvaluefile(self.vfs, 'kvfile').write(d)
+
+    def testrequiredkeys(self):
+        d = {'key1': 'value1', 'Key2': 'value2'}
+        scmutil.simplekeyvaluefile(self.vfs, 'allkeyshere').write(d)
+
+        class kvf(scmutil.simplekeyvaluefile):
+            KEYS = [('key3', False), ('Key2', True)]
+        self.assertEqual(sorted(kvf(self.vfs, 'allkeyshere').read().items()),
+                         [('Key2', 'value'), ('key1', 'value')])
+
+        d = {'key1': 'value1', 'Key3': 'value2'}
+        scmutil.simplekeyvaluefile(self.vfs, 'missingkeys').write(d)
+
+        class kvf(scmutil.simplekeyvaluefile):
+            KEYS = [('key3', False), ('Key2', True)]
+        with self.assertRaisesRegexp(error.MissingRequiredKey, "missing a.*"):
+            kvf(self.vfs, 'missingkeys').read()
+
+    def testcorruptedfile(self):
+        contents['badfile'] = 'ababagalamaga\n'
+        with self.assertRaisesRegexp(error.CorruptedState,
+                                     "dictionary.*element.*"):
+            scmutil.simplekeyvaluefile(self.vfs, 'badfile').read()
+
+if __name__ == "__main__":
+    silenttestrunner.main(__name__)