Merge lp:~milo/linaro-patchmetrics/bug1155533 into lp:linaro-patchmetrics

[Milo Casagrande]

Fixed gerrit query string as per new upstream release.

Android gerrit instance has been updated to a new release, and they sligthly
change the query parameters necessary to retrieve results.

In particular, the default format returned with the query is json, and there
is no need to specify that (we already force that in the header).

Doing that, we are now using a full gerrit query, made of documented query
parameters.

In the changes I hade to introduce a new gerrit query param, and remove usage
of the unsupported ones.

[James Tunnicliffe (dooferlad)]

Looks good to me.

review: Approve

=== modified file 'apps/patchmetrics/gerrit.py'
--- apps/patchmetrics/gerrit.py	2012-07-02 07:37:08 +0000
+++ apps/patchmetrics/gerrit.py	2013-03-27 15:04:32 +0000
@@ -13,9 +13,8 @@
     GERRIT_ID_KEY,
     GERRIT_NUMBER_KEY,
     GERRIT_PROJECT_KEY,
-    GERRIT_QUERY_FORMAT,
+    GERRIT_QUERY_LIMIT,
     GERRIT_QUERY_OWNER,
-    GERRIT_QUERY_PG_SIZE,
     GERRIT_QUERY_RESUME_SORTKEY,
     GERRIT_SANITIZABLE_VALUE,
     GERRIT_STATUS_KEY,
@@ -58,34 +57,30 @@
                           'JSON_COMPACT' for JSON format without spaces, also
                           possible to specify 'JSON'.
     """
-    params = {GERRIT_QUERY_FORMAT: return_format,
-              GERRIT_QUERY_PG_SIZE: page_size}
     query_params = {GERRIT_QUERY_OWNER: email,
+                    GERRIT_QUERY_LIMIT: page_size,
                     GERRIT_QUERY_RESUME_SORTKEY: resume_key}
-    return rest_api_request(ROOT_REST_URL, CHANGES_QUERY_PATH, params,
-                            query_params)
-
-
-def rest_api_request(url, path, params, query_params):
+    return rest_api_request(ROOT_REST_URL, CHANGES_QUERY_PATH, query_params)
+
+
+def rest_api_request(url, path, query_params):
     """Calls the gerrit REST interface and return the JSON response.
 
     :param url: the root URL of the web service.
     :param path: the path of the REST interface, after the URL.
-    :param params: the parameters for the query, can be a dictionary or tuple.
     :param query_params: the parameters for the gerrit query interface, must
                          be a dictionary.
     """
-    request_url = create_request_query_url(url, path, params, query_params)
+    request_url = create_request_query_url(url, path, query_params)
     return gerrit_request(request_url)
 
 
-def create_request_query_url(url, path, params, query_params):
+def create_request_query_url(url, path, query_params):
     """Creates the full query URL from the various elements.
 
     :param url: the root URL of the web service, without path to the REST
                 interface.
     :param path: the path of the REST interface, after the URL.
-    :param params: the parameters for the query, can be a dictionary or tuple.
     :param query_params: the parameters for the gerrit query interface, must
                          be a dictionary.
     """
@@ -93,9 +88,8 @@
         url += '/'
     if not path.endswith('/'):
         path += '/'
+
     request_url = url + path + QUERY_STRING_START
-
-    request_url += urllib.urlencode(params)
     request_url += create_gerrit_rest_query(query_params)
 
     return request_url
@@ -115,18 +109,13 @@
                         type(query_params).__name__)
 
     params_list = []
-    # Where we will store the query.
     gerrit_query = ''
     for key, value in query_params.iteritems():
-        # We cannot have None values here, we just skip them.
-        if value is not None:
-            # We need string, and remove white chars.
-            value = str(value).strip()
-            # Some emails need to be quoted, and only double quotes are
-            # accepted by gerrit.
-            if key == GERRIT_QUERY_OWNER:
-                value = ("\"%s\"" % value)
-            # We need strings here, and we remove white spaces.
+        # We cannot have None values here.
+        if value:
+            # We need string, remove white chars and quote values.
+            # gerrit accepts single or double quotes and curly brackets.
+            value = ("\"%s\"" % str(value).strip())
             params_list.append(":".join((key, urllib.quote(value))))
     # The resulting query string, without separators or prefixes, should be in
     # the form of:
@@ -134,8 +123,8 @@
     # This is a normal query, more complex queries need to be thought of, since
     # we can express also OR, AND and NOT operators. By default it is an AND.
     if params_list:
-        gerrit_query = QUERY_STRING_SEP + GERRIT_QUERY_PREFIX + \
-        GERRIT_QUERY_SEPARATOR.join(params_list)
+        gerrit_query = (GERRIT_QUERY_PREFIX +
+            GERRIT_QUERY_SEPARATOR.join(params_list))
 
     return gerrit_query
 

=== modified file 'apps/patchmetrics/gerrit_values.py'
--- apps/patchmetrics/gerrit_values.py	2012-06-27 08:25:59 +0000
+++ apps/patchmetrics/gerrit_values.py	2013-03-27 15:04:32 +0000
@@ -26,6 +26,9 @@
 
 # Gerrit query parameters
 GERRIT_QUERY_OWNER = 'owner'
+GERRIT_QUERY_RESUME_SORTKEY = 'resume_sortkey'
+GERRIT_QUERY_LIMIT = 'limit'
+
+# Deprecated keys, or not anymore in the docs.
 GERRIT_QUERY_FORMAT = 'format'
 GERRIT_QUERY_PG_SIZE = 'n'
-GERRIT_QUERY_RESUME_SORTKEY = 'resume_sortkey'