Add a package listing API to cipd.

appengine/chrome_infra_packages/cipd/impl.py

Index: appengine/chrome_infra_packages/cipd/impl.py
diff --git a/appengine/chrome_infra_packages/cipd/impl.py b/appengine/chrome_infra_packages/cipd/impl.py
index 451a0377cef1445f1698575efb196aa621fb0940..dbb9695287e4fd46a95e63474e14998bad4f3021 100644
--- a/appengine/chrome_infra_packages/cipd/impl.py
+++ b/appengine/chrome_infra_packages/cipd/impl.py
@@ -83,6 +83,7 @@ builders should not be allowed to set by other WRITERs.
 
 import collections
 import hashlib
+import itertools
 import json
 import logging
 import re
@@ -165,6 +166,83 @@ class RepoService(object):
     """
     return package_key(package_name).get()
 
+  @staticmethod
+  def _is_in_directory(directory, path, recursive):
+    """Tests is the path is under the given directory.

[Vadim Sh., 2015/06/23 00:08:34]

typo: if

[estaab, 2015/06/23 02:26:13]

Done.

+
+    This assumes directory is a prefix of path.
+
+    Args:
+      directory: String, directory the path should fall under.

[Vadim Sh., 2015/06/23 00:08:35]

nit: start arg desc with lower case letter

[estaab, 2015/06/23 02:26:13]

Done.

+      path: String, full path to test.
+      recursive: Whether the path can be in a subdirectory.
+
+    Returns:
+      True if the path is under the directory.
+    """
+    start = len(directory)
+    logging.info('processing %s in %s', path, directory)

[Vadim Sh., 2015/06/23 00:08:34]

remove logging if it doesn't provide any additional information. Once this code
is fully tested and stable, logs here would not be useful.

On GAE logs are quota-controlled non-infinite resource.

[estaab, 2015/06/23 02:26:13]

Sorry, also not supposed to be in the CL.

+
+    # The directory itself or anything shorter is not a match.
+    if len(path) <= start:
+      logging.info('shorter.')
+      return False
+
+    # The root doesn't begin with slash so only check non-root searches.
+    if start:
+      if path[start] != '/':
+        logging.info('next isnt /.')
+        return False
+      start += 1
+
+    # A subdirectory was found and we're not looking for recursive matches.
+    if not recursive and '/' in path[start:]:
+      logging.info('next after is /.')
+      return False
+    return True
+
+  def list_packages(self, dir_path, recursive):
+    """Returns lists of package names and directory names with the given prefix.
+
+    Args:
+      dir_path: string directory from which to list packages.
+      recursive: boolean whether to list contents of subdirectories.
+
+    Returns:
+      [package name, ...], [directory name, ...]
+    """
+    query = Package.query()
+
+    # Normalize directory to simplify matching logic later.
+    dir_path = dir_path.rstrip('/')
+
+    # Only apply the filtering if a prefix was given. The empty string isn't a
+    # valid key and will result in an exception.
+    if dir_path:
+      query = query.filter(
+          # Prefix match using the operators available to us. Packages can only
+          # contain lowercase ascii, numbers, and '/' so '\uffff' will always
+          # be larger.
+          ndb.AND(Package.key >= ndb.Key(Package, dir_path),
+                  Package.key <= ndb.Key(Package, dir_path + u'\uffff')))
+    pkgs = []
+    dirs = set()
+    for key in query.iter(keys_only=True):

[Vadim Sh., 2015/06/23 00:08:35]

non-ancestor queries are eventually consistent: the query can occasionally lie
and return entities that do not match filters. I doubt it happens for this
particular query, but it's not hard to filter out false hits with
pkg.startswith(dir_path) check.

From https://cloud.google.com/appengine/docs/python/datastore/queries:
"""
In an eventually consistent query, the indexes used to gather the results are
also accessed with eventual consistency. Consequently, such queries may
sometimes return entities that no longer match the original query criteria.
"""

[estaab, 2015/06/23 02:26:13]

Good to know, done. I couldn't think of an easy way to test this but it's
probably not too important. The no cover pragma is ugly, though.

+      pkg = key.string_id()
+      pkgs.append(pkg)
+
+      # Add in directories derived from full package path if it's not an exact
+      # match.
+      if '/' in pkg and len(dir_path) != len(pkg):

[Vadim Sh., 2015/06/23 00:08:35]

I don't understand len(dir_path) != len(pkg) condition. What does it guard from?

[estaab, 2015/06/23 02:26:13]

Removed, this was from some older logic.

+        parts = pkg.split('/')
+        dirs.update('/'.join(parts[:n]) for n in xrange(1, len(parts)))
+        logging.info(str(['/'.join(parts[:n]) for n in xrange(1, len(parts))]))

[Vadim Sh., 2015/06/23 00:08:34]

remove logging, it's probably not helpful in released code

[estaab, 2015/06/23 02:26:13]

Done, also a mistake.

+
+    dirs = [d for d in dirs if self._is_in_directory(dir_path, d, recursive)]
+    pkgs = [p for p in pkgs if self._is_in_directory(dir_path, p, recursive)
+            or len(dir_path) == len(p)]
+    return pkgs, dirs
+
   def get_processing_result(self, package_name, instance_id, processor_name):
     """Returns results of some asynchronous processor or None if not ready.