Merge lp:~chipaca/u1db/split_out_bad_globbage into lp:u1db

[John Lenton]

Split the «user globbed wrong» error from the «user used the wrong number of arguments» error when querying indexes.

[lp:~chipaca/u1db/split_out_bad_globbage]

310. By John Lenton on 2012-05-23

fix up docstrings as per issues reported by thisfred and suggestions from sil

Unmerged revisions

=== modified file 'include/u1db/u1db.h'
--- include/u1db/u1db.h	2012-05-21 15:01:18 +0000
+++ include/u1db/u1db.h	2012-05-23 12:00:25 +0000
@@ -67,6 +67,7 @@
 #define U1DB_INVALID_FIELD_SPECIFIER -16
 #define U1DB_DUPLICATE_INDEX_NAME -17
 #define U1DB_INDEX_DOES_NOT_EXIST -18
+#define U1DB_INVALID_GLOBBING -19
 #define U1DB_INTERNAL_ERROR -999
 
 // Used by put_doc_if_newer

=== modified file 'src/u1db_query.c'
--- src/u1db_query.c	2012-05-21 16:29:25 +0000
+++ src/u1db_query.c	2012-05-23 12:00:25 +0000
@@ -725,7 +725,7 @@
             wildcard[i] = 2;
             if (have_wildcard) {
                 //globs not allowed after another wildcard
-                status = U1DB_INVALID_VALUE_FOR_INDEX;
+                status = U1DB_INVALID_GLOBBING;
                 goto finish;
             }
             have_wildcard = 1;
@@ -734,7 +734,7 @@
             wildcard[i] = 0;
             if (have_wildcard) {
                 // Can't have a non-wildcard after a wildcard
-                status = U1DB_INVALID_VALUE_FOR_INDEX;
+                status = U1DB_INVALID_GLOBBING;
                 goto finish;
             }
             add_to_buf(&cur, &buf_size, " AND d%d.value = ?", i);

=== modified file 'u1db/backends/inmemory.py'
--- u1db/backends/inmemory.py	2012-05-21 15:01:18 +0000
+++ u1db/backends/inmemory.py	2012-05-23 12:00:25 +0000
@@ -302,14 +302,14 @@
                     # We have an 'x*' style wildcard
                     if is_wildcard:
                         # We were already in wildcard mode, so this is invalid
-                        raise errors.InvalidValueForIndex()
+                        raise errors.InvalidGlobbing
                     last = idx + 1
                 is_wildcard = True
             else:
                 if is_wildcard:
                     # We were in wildcard mode, we can't follow that with
                     # non-wildcard
-                    raise errors.InvalidValueForIndex()
+                    raise errors.InvalidGlobbing
                 last = idx + 1
         if not is_wildcard:
             return -1

=== modified file 'u1db/backends/sqlite_backend.py'
--- u1db/backends/sqlite_backend.py	2012-05-21 16:29:25 +0000
+++ u1db/backends/sqlite_backend.py	2012-05-23 12:00:25 +0000
@@ -592,13 +592,13 @@
                         if is_wildcard:
                             # We can't have a partial wildcard following
                             # another wildcard
-                            raise errors.InvalidValueForIndex()
+                            raise errors.InvalidGlobbing
                         where.append(like_where[idx])
                         args.append(self._transform_glob(value))
                     is_wildcard = True
                 else:
                     if is_wildcard:
-                        raise errors.InvalidValueForIndex()
+                        raise errors.InvalidGlobbing
                     where.append(exact_where[idx])
                     args.append(value)
             statement = ("SELECT d.doc_id, d.doc_rev, d.content"

=== modified file 'u1db/commandline/client.py'
--- u1db/commandline/client.py	2012-05-21 17:32:38 +0000
+++ u1db/commandline/client.py	2012-05-23 12:00:25 +0000
@@ -18,6 +18,7 @@
 
 import argparse
 import os
+import simplejson
 import sys
 
 from u1db import (
@@ -322,9 +323,86 @@
             return
         return 1
 
-
 client_commands.register(CmdGetIndexKeys)
 
 
+class CmdGetFromIndex(OneDbCmd):
+    """Find documents by searching an index"""
+
+    name = "get-from-index"
+    argv = None
+
+    @classmethod
+    def _populate_subparser(cls, parser):
+        parser.add_argument('database', help='The local database to query',
+                            metavar='database-path')
+        parser.add_argument('index', help='the name of the index')
+        parser.add_argument('values', metavar="value",
+                            help='the value to look up (one per index column)',
+                            nargs="+")
+
+    def run(self, database, index, values):
+        try:
+            db = self._open(database, create=False)
+            docs = db.get_from_index(index, [values])
+        except errors.DatabaseDoesNotExist:
+            self.stderr.write("Database does not exist.\n")
+        except errors.IndexDoesNotExist:
+            self.stderr.write("Index does not exist.\n")
+        except errors.InvalidValueForIndex:
+            index_def = db._get_index_definition(index)
+            msg = ["Invalid query;"]
+            len_diff = len(index_def) - len(values)
+            if len_diff:
+                msg.append("index %r requires %d query expression%s"
+                           % (index, len(index_def),
+                              "s" if len(index_def) > 1 else ""))
+                if len(values):
+                    msg[-1] += ", not %d" % len(values)
+                if len_diff > 0:
+                    msg[-1] += ".\nPerhaps you meant:"
+                    argv = self.argv if self.argv is not None else sys.argv
+                    msg.extend(argv[:2])
+                    msg.append(repr(database))
+                    msg.extend(map(repr, values))
+                    msg.extend(["'*'" for i in range(len_diff)])
+            else:
+                # can't happen (HAH)
+                msg.append("not sure how to help you (read the docs?)")
+            self.stderr.write(" ".join(msg))
+            self.stderr.write(".\n")
+        except errors.InvalidGlobbing:
+            # XXX this needs to actually help the user
+            argv = self.argv if self.argv is not None else sys.argv
+            fixed = []
+            for (i, v) in enumerate(values):
+                fixed.append(v)
+                if v.endswith('*'):
+                    break
+            # values has at least one element, so i is defined
+            fixed.extend('*'*(len(values)-i-1))
+            self.stderr.write("Invalid query; you cannot do a partial search"
+                              " twice in one query.\nPerhaps you meant: "
+                              "%s %s %r %s.\n"
+                              % (argv[0], argv[1], database,
+                                 " ".join(map(repr, fixed))))
+
+        else:
+            self.stdout.write("[")
+            for i, doc in enumerate(docs):
+                if i:
+                    self.stdout.write(",")
+                self.stdout.write(simplejson.dumps(dict(
+                            id=doc.doc_id,
+                            rev=doc.rev,
+                            content=simplejson.loads(doc.content)),
+                                                   indent=4))
+            self.stdout.write("]\n")
+            return
+        return 1
+
+client_commands.register(CmdGetFromIndex)
+
+
 def main(args):
     return client_commands.run_argv(args, sys.stdin, sys.stdout, sys.stderr)

=== modified file 'u1db/errors.py'
--- u1db/errors.py	2012-05-21 15:01:18 +0000
+++ u1db/errors.py	2012-05-23 12:00:25 +0000
@@ -44,10 +44,11 @@
 
 
 class InvalidValueForIndex(U1DBError):
-    """The values supplied does not match the index definition.
-
-    Can also be raised if wildcard matches are not strictly at the tail of the
-    request.
+    """The values supplied does not match the index definition."""
+
+
+class InvalidGlobbing(U1DBError):
+    """Raised if wildcard matches are not strictly at the tail of the request.
     """
 
 

=== modified file 'u1db/tests/c_backend_wrapper.pyx'
--- u1db/tests/c_backend_wrapper.pyx	2012-05-21 18:28:05 +0000
+++ u1db/tests/c_backend_wrapper.pyx	2012-05-23 12:00:25 +0000
@@ -116,6 +116,7 @@
     int U1DB_NOT_IMPLEMENTED
     int U1DB_INVALID_JSON
     int U1DB_INVALID_VALUE_FOR_INDEX
+    int U1DB_INVALID_GLOBBING
     int U1DB_BROKEN_SYNC_STREAM
     int U1DB_DUPLICATE_INDEX_NAME
     int U1DB_INDEX_DOES_NOT_EXIST
@@ -560,6 +561,8 @@
                                   % (context,))
     if status == U1DB_INVALID_VALUE_FOR_INDEX:
         raise errors.InvalidValueForIndex()
+    if status == U1DB_INVALID_GLOBBING:
+        raise errors.InvalidGlobbing()
     if status == U1DB_INTERNAL_ERROR:
         raise errors.U1DBError("internal error")
     if status == U1DB_BROKEN_SYNC_STREAM:

=== modified file 'u1db/tests/commandline/test_client.py'
--- u1db/tests/commandline/test_client.py	2012-05-21 17:32:38 +0000
+++ u1db/tests/commandline/test_client.py	2012-05-23 12:00:25 +0000
@@ -17,6 +17,7 @@
 import cStringIO
 import os
 import sys
+import simplejson
 import subprocess
 
 from u1db import (
@@ -145,6 +146,13 @@
         self.assertEqual('db', args.database)
         self.assertEqual('index', args.index)
 
+    def test_get_from_index(self):
+        args = self.parse_args(['get-from-index', 'db', 'index', 'foo'])
+        self.assertEqual(client.CmdGetFromIndex, args.subcommand)
+        self.assertEqual('db', args.database)
+        self.assertEqual('index', args.index)
+        self.assertEqual(['foo'], args.values)
+
 
 class TestCaseWithDB(tests.TestCase):
     """These next tests are meant to have one class per Command.
@@ -472,6 +480,90 @@
         self.assertEqual(cmd.stderr.getvalue(), 'Index does not exist.\n')
 
 
+class TestCmdGetFromIndex(TestCaseWithDB):
+
+    def test_get_from_index(self):
+        self.db.create_index("index", ["key"])
+        doc1 = self.db.create_doc(tests.simple_doc)
+        doc2 = self.db.create_doc(tests.nested_doc)
+        cmd = self.make_command(client.CmdGetFromIndex)
+        retval = cmd.run(self.db_path, "index", ["value"])
+        self.assertEqual(retval, None)
+        self.assertEqual(sorted(simplejson.loads(cmd.stdout.getvalue())),
+                         sorted([dict(id=doc1.doc_id,
+                                      rev=doc1.rev,
+                                      content=simplejson.loads(doc1.content)),
+                                 dict(id=doc2.doc_id,
+                                      rev=doc2.rev,
+                                      content=simplejson.loads(doc2.content)),
+                                 ]))
+        self.assertEqual(cmd.stderr.getvalue(), '')
+
+    def test_get_from_index_empty(self):
+        self.db.create_index("index", ["key"])
+        cmd = self.make_command(client.CmdGetFromIndex)
+        retval = cmd.run(self.db_path, "index", ["value"])
+        self.assertEqual(retval, None)
+        self.assertEqual(cmd.stdout.getvalue(), '[]\n')
+        self.assertEqual(cmd.stderr.getvalue(), '')
+
+    def test_get_from_index_no_db(self):
+        cmd = self.make_command(client.CmdGetFromIndex)
+        retval = cmd.run(self.db_path + "__DOES_NOT_EXIST", "foo", [])
+        self.assertEqual(retval, 1)
+        self.assertEqual(cmd.stdout.getvalue(), '')
+        self.assertEqual(cmd.stderr.getvalue(), 'Database does not exist.\n')
+
+    def test_get_from_index_no_index(self):
+        cmd = self.make_command(client.CmdGetFromIndex)
+        retval = cmd.run(self.db_path, "foo", [])
+        self.assertEqual(retval, 1)
+        self.assertEqual(cmd.stdout.getvalue(), '')
+        self.assertEqual(cmd.stderr.getvalue(), 'Index does not exist.\n')
+
+    def test_get_from_index_two_expr_instead_of_one(self):
+        self.db.create_index("index", ["key1"])
+        cmd = self.make_command(client.CmdGetFromIndex)
+        retval = cmd.run(self.db_path, "index", ["value1", "value2"])
+        self.assertEqual(retval, 1)
+        self.assertEqual(cmd.stdout.getvalue(), '')
+        self.assertEqual(cmd.stderr.getvalue(), "Invalid query; index"
+                         " 'index' requires 1 query expression, not 2.\n")
+
+    def test_get_from_index_three_expr_instead_of_two(self):
+        self.db.create_index("index", ["key1", "key2"])
+        cmd = self.make_command(client.CmdGetFromIndex)
+        retval = cmd.run(self.db_path, "index", ["value1", "value2", "value3"])
+        self.assertEqual(retval, 1)
+        self.assertEqual(cmd.stdout.getvalue(), '')
+        self.assertEqual(cmd.stderr.getvalue(), "Invalid query; index"
+                         " 'index' requires 2 query expressions, not 3.\n")
+
+    def test_get_from_index_one_expr_instead_of_two(self):
+        self.db.create_index("index", ["key1", "key2"])
+        cmd = self.make_command(client.CmdGetFromIndex)
+        cmd.argv = ["XX", "YY"]
+        retval = cmd.run(self.db_path, "index", ["value1"])
+        self.assertEqual(retval, 1)
+        self.assertEqual(cmd.stdout.getvalue(), '')
+        self.assertEqual(cmd.stderr.getvalue(), "Invalid query; index"
+                         " 'index' requires 2 query expressions, not 1.\n"
+                         "Perhaps you meant: XX YY %r 'value1' '*'.\n"
+                         % self.db_path)
+
+    def test_get_from_index_cant_bad_glob(self):
+        self.db.create_index("index", ["key1", "key2"])
+        cmd = self.make_command(client.CmdGetFromIndex)
+        cmd.argv = ["XX", "YY"]
+        retval = cmd.run(self.db_path, "index", ["value1*", "value2"])
+        self.assertEqual(retval, 1)
+        self.assertEqual(cmd.stdout.getvalue(), '')
+        self.assertEqual(cmd.stderr.getvalue(), "Invalid query; you"
+                         " cannot do a partial search twice in one query.\n"
+                         "Perhaps you meant: XX YY %r 'value1*' '*'.\n"
+                         % self.db_path)
+
+
 class RunMainHelper(object):
 
     def run_main(self, args, stdin=None):

=== modified file 'u1db/tests/test_backends.py'
--- u1db/tests/test_backends.py	2012-05-21 16:29:25 +0000
+++ u1db/tests/test_backends.py	2012-05-23 12:00:25 +0000
@@ -769,12 +769,12 @@
 
     def test_get_from_index_illegal_wildcard_order(self):
         self.db.create_index('test-idx', ['k1', 'k2'])
-        self.assertRaises(errors.InvalidValueForIndex,
+        self.assertRaises(errors.InvalidGlobbing,
             self.db.get_from_index, 'test-idx', [('*', 'v2')])
 
     def test_get_from_index_illegal_glob_after_wildcard(self):
         self.db.create_index('test-idx', ['k1', 'k2'])
-        self.assertRaises(errors.InvalidValueForIndex,
+        self.assertRaises(errors.InvalidGlobbing,
             self.db.get_from_index, 'test-idx', [('*', 'v*')])
 
     def test_get_all_from_index(self):
@@ -891,13 +891,13 @@
 
     def test_get_from_index_illegal_glob_before_value(self):
         self.db.create_index('test-idx', ['k1', 'k2'])
-        self.assertRaises(errors.InvalidValueForIndex,
+        self.assertRaises(errors.InvalidGlobbing,
             self.db.get_from_index, 'test-idx', [('v*', 'v2')])
 
     def test_get_from_index_illegal_glob_after_glob(self):
         self.db.create_index('test-idx', ['k1', 'k2'])
-        self.assertRaises(errors.InvalidValueForIndex,
-            self.db.get_from_index, 'test-idx', [('*', 'v*')])
+        self.assertRaises(errors.InvalidGlobbing,
+            self.db.get_from_index, 'test-idx', [('v*', 'v*')])
 
     def test_get_from_index_with_sql_wildcards(self):
         self.db.create_index('test-idx', ['key'])

=== modified file 'u1db/tests/test_inmemory.py'
--- u1db/tests/test_inmemory.py	2012-04-27 14:22:47 +0000
+++ u1db/tests/test_inmemory.py	2012-05-23 12:00:25 +0000
@@ -124,5 +124,5 @@
             idx._find_non_wildcards, ('a', 'b'))
         self.assertRaises(errors.InvalidValueForIndex,
             idx._find_non_wildcards, ('a', 'b', 'c', 'd'))
-        self.assertRaises(errors.InvalidValueForIndex,
+        self.assertRaises(errors.InvalidGlobbing,
             idx._find_non_wildcards, ('*', 'b', 'c'))