Ignore '' path segments, which is important for get_mapping et al.

Factor out _join_path to make testing easier.

pyelasticsearch/client.py

@@ -161,6 +161,15 @@ def _to_query(cls, obj):
         raise TypeError("_to_query() doesn't know how to represent %r in an ES"
                         " query string." % obj)
 
+    def _join_path(self, path_components):
+        """Smush together the (string) path components, ignoring empty ones."""
+        path = '/'.join(quote_plus(str(p), '') for p in path_components if
+                        len(str(p)))
+
+        if not path.startswith('/'):
+            path = '/' + path
+        return path
+
     def send_request(self,
                      method,
                      path_components,
@@ -186,22 +195,13 @@ def send_request(self,
             ``None``
         :arg encode_body: Whether to encode the body of the request as JSON
         """
-        def join_path(path_components):
-            """Smush together the path components, ignoring empty ones."""
-            path = '/'.join(quote_plus(str(p), '') for p in path_components)
-
-            if not path.startswith('/'):
-                path = '/' + path
-            return path
-
-        path = join_path(path_components)
+        path = self._join_path(path_components)
         if query_params:
             path = '?'.join(
                 [path, urlencode(dict((k, self._to_query(v)) for k, v in
                                       query_params.iteritems()))])
 
-        kwargs = ({'data': self._encode_json(body) if encode_body else body}
-                   if body else {})
+        request_body = self._encode_json(body) if encode_body else body
         req_method = getattr(self.session, method.lower())
 
         # We do our own retrying rather than using urllib3's; we want to retry
@@ -212,10 +212,13 @@ def join_path(path_components):
             url = server_url + path
             self.logger.debug(
                 "Making a request equivalent to this: curl -X%s '%s' -d '%s'" %
-                (method, url, kwargs.get('data', {})))
+                (method, url, request_body))
 
             try:
-                resp = req_method(url, timeout=self.timeout, **kwargs)
+                resp = req_method(
+                    url,
+                    timeout=self.timeout,
+                    **({'data': request_body} if body else {}))
             except (ConnectionError, Timeout):
                 self.servers.mark_dead(server_url)
                 self.logger.info('%s marked as dead for %s seconds.',
@@ -359,16 +362,19 @@ def delete(self, index, doc_type, id, query_params=None):
 
         :arg index: The name of the index from which to delete
         :arg doc_type: The type of the document to delete
-        :arg id: The ID of the document to delete
+        :arg id: The (string or int) ID of the document to delete
 
         See `ES's delete API`_ for more detail.
 
         .. _`ES's delete API`:
             http://www.elasticsearch.org/guide/reference/api/delete.html
         """
+        # id should never be None, and it's not particular dangerous
+        # (equivalent to deleting a doc with ID "None", but it's almost
+        # certainly not what the caller meant:
         if id is None or id == '':
             raise ValueError('No ID specified. To delete all documents in '
-                'an index, use delete_all().')
+                             'an index, use delete_all().')
         return self.send_request('DELETE', [index, doc_type, id],
                                  query_params=query_params)
 

pyelasticsearch/tests.py

@@ -254,6 +254,21 @@ def testUpdate(self):
                   'lang': 'python'},
                   query_params={})
 
+    def test_empty_path_segments(self):
+        """'' segments passed to ``_join_path`` should be omitted."""
+        # Call _join_path like get_mapping might if called with no params:
+        self.assertEqual(self.conn._join_path(['', '', '_mapping']),
+                         '/_mapping')
+
+    def test_0_path_segments(self):
+        """
+        ``0`` segments passed to ``_join_path`` should be included.
+
+        This is so doc IDs that are 0 work.
+        """
+        self.assertEqual(self.conn._join_path([0, '_mapping']),
+                         '/0/_mapping')
+
 
 class SearchTestCase(ElasticSearchTestCase):
     def setUp(self):