Reviewers: mp+238457_code.launchpad.net,

Message:
Please take a look.

Description:
Add 'start' parameter to API search.

The start is the zero-based index into the search results.  It can be
used
in conjunction with limit.

https://code.launchpad.net/~bac/charmworld/bug-1379397/+merge/238457

(do not edit description out of merge proposal)


Please review this at https://codereview.appspot.com/158010043/

Affected files (+85, -7 lines):
   A [revision details]
   M charmworld/search.py
   M charmworld/tests/test_search.py
   M charmworld/views/api/__init__.py


Index: [revision details]
=== added file '[revision details]'
--- [revision details]	2012-01-01 00:00:00 +0000
+++ [revision details]	2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140905151929-pbmcu2i5i3s9vgkx
+New revision: <email address hidden>

Index: charmworld/search.py
=== modified file 'charmworld/search.py'
--- charmworld/search.py	2014-07-15 19:09:59 +0000
+++ charmworld/search.py	2014-10-15 14:45:53 +0000
@@ -63,6 +63,10 @@
      """Raised when a negative limit is specified."""


+class InvalidStart(Exception):
+    """Raised when an invalid 'start' is specified."""
+
+
  class IncompatibleMapping(Exception):
      """Raised when desired mapping is not compatible with active  
mapping."""

@@ -615,7 +619,7 @@
      def api_search(self, text='', filters=None, type_=None,
                     limit=None, valid_only=True, sort=None,
                     autocomplete=False, doctype=CHARM,
-                   min_score=None):
+                   min_score=None, start=None):
          """Search charms and bundles (as used by the API).

          :param text: A string to search for -- will be analyzed (i.e. split
@@ -641,12 +645,18 @@
              overridden by use of 'charm:' or 'bundle:' query string prefix.
          :param min_score: Only return results with a score of at least
              min_score.
+        :param start: Return the 'limit' number of items from the start
+            index, e.g. items[start:start+limit].  Start is zero-based.
          return: A list of {doctype=<charm|bundle>, data=entity}.
          """
          if limit is not None:
              if limit < 0:
                  raise NegativeLimit()

+        if start is not None:
+            if start < 0:
+                raise InvalidStart()
+
          text, doctype, all_requested = extract_doctype_from_search_terms(
              text, doctype)

@@ -679,11 +689,17 @@
              else:
                  limit = min(limit, self.default_search_limit)

+        if start is not None and limit is not None:
+            limit = limit + start
+
          try:
              hits = self._unlimited_search(dsl, limit)
          except ElasticHttpError:
              hits = []

+        if start is not None:
+            hits = hits[start:]
+
          return [
              dict(doctype=hit['_type'], data=hit['_source']['data'])
              for hit in hits]


Index: charmworld/tests/test_search.py
=== modified file 'charmworld/tests/test_search.py'
--- charmworld/tests/test_search.py	2014-07-15 19:09:59 +0000
+++ charmworld/tests/test_search.py	2014-10-15 14:45:53 +0000
@@ -35,6 +35,7 @@
      IndexMissing,
      IndexNotReady,
      InvalidCharmType,
+    InvalidStart,
      update,
      NegativeLimit,
      reindex,
@@ -801,6 +802,47 @@
          result = self.index_client.api_search('charms', {}, None)
          self.assertEqual(22, len(result))

+    def test_charms_start(self):
+        """Start provides the starting offset for results."""
+        result = self.index_client.api_search(
+            '', {}, None, limit=6, min_score=0, start=0)
+        self.assertEqual(0, len(result))
+        # Make ten charms
+        for i in range(10):
+            self.makeCharm()
+        # Starting at 0, all charms are returned.
+        result = self.index_client.api_search(
+            '', {}, None, min_score=0, start=0)
+        self.assertEqual(10, len(result))
+        # Starting at 1, nine are returned.
+        result = self.index_client.api_search(
+            '', {}, None, min_score=0, start=1)
+        self.assertEqual(9, len(result))
+        # Starting at 9, one is returned.
+        result = self.index_client.api_search(
+            '', {}, None, min_score=0, start=9)
+        self.assertEqual(1, len(result))
+        # Test in combination with limit.
+        # Starting at 0, the limit number are returned.
+        result = self.index_client.api_search(
+            '', {}, None, limit=5, min_score=0, start=0)
+        self.assertEqual(5, len(result))
+        # Starting at 3, the limit number are still returned.
+        result = self.index_client.api_search(
+            '', {}, None, limit=5, min_score=0, start=3)
+        self.assertEqual(5, len(result))
+        # If we start beyond 'limit' point, not all are returned.
+        result = self.index_client.api_search(
+            '', {}, None, limit=5, min_score=0, start=6)
+        self.assertEqual(4, len(result))
+        # Start is zero-based, so asking for start=10 returns no results.
+        result = self.index_client.api_search(
+            '', {}, None, limit=1, min_score=0, start=10)
+        self.assertEqual(0, len(result))
+        with self.assertRaises(InvalidStart):
+            result = self.index_client.api_search(
+                '', {}, None, start=-1, min_score=0)
+
      def test_api_search_with_store_error(self):
          """api_search() does not return charms withs errors by default."""
          self.makeCharm(name='evil-charm', charm_error=True)


Index: charmworld/views/api/__init__.py
=== modified file 'charmworld/views/api/__init__.py'
--- charmworld/views/api/__init__.py	2014-05-06 20:49:19 +0000
+++ charmworld/views/api/__init__.py	2014-10-15 14:45:53 +0000
@@ -37,6 +37,7 @@
      BUNDLE,
      CHARM,
      InvalidCharmType,
+    InvalidStart,
      NegativeLimit,
  )
  from charmworld.utils import (
@@ -716,7 +717,7 @@
      def _search(self, limit=None, name=None, series=None, owner=None,
                  provides=None, requires=None, type_=None, provider=None,
                  scope=None, categories=None, text=None, autocomplete=False,
-                doctype=None, min_score=None):
+                doctype=None, min_score=None, start=None):
          """Search for charms and bundles matching parameters.

          :limit: A query limit.  (The max number of results.)  Dispatched  
as a
@@ -724,6 +725,11 @@
          :min_score: The minimum score for filtering.  Dispatched as a
             list of numeric characters representing a float, e.g. ['1.1'].
          """
+
+        def _check_scalar(value):
+            if value is not None and len(value) > 1:
+                raise ValueError()
+
          autocomplete = autocomplete == ['true']
          params = dict((key, value) for key, value in locals().items()
                        if key in ('series', 'owner', 'name', 'categories'))
@@ -734,16 +740,24 @@
          params['i_provides'] = provides
          params['i_requires'] = requires
          filters = dict(item for item in params.items() if item[1] is not  
None)
+        for name in ('limit', 'start'):
+            try:
+                _check_scalar(locals()[name])
+            except ValueError:
+                return json_response(
+                    406,
+                    {
+                        'type': 'multiple_values',
+                        'parameter': name,
+                    })
          if limit is not None:
-            if len(limit) > 1:
-                return json_response(406, {
-                                     'type': 'multiple_values',
-                                     'parameter': 'limit'})
              limit = int(limit[0])
+        if start is not None:
+            start = int(start[0])
          try:
              results = self.request.index_client.api_search(
                  text[0], filters, type_, limit, autocomplete=autocomplete,
-                doctype=doctype, min_score=min_score)
+                doctype=doctype, min_score=min_score, start=start)
          except InvalidCharmType as e:
              return json_response(406, {
                                   'type': 'unsupported_value',
@@ -753,6 +767,10 @@
              return json_response(406, {
                                   'type': 'negative_value',
                                   'parameter': 'limit'})
+        except InvalidStart:
+            return json_response(406, {
+                                 'type': 'invalid_value',
+                                 'parameter': 'start'})
          return {'result': self._item_results(results)}

      def _item_results(self, items):