Add a package listing API to cipd.

appengine/chrome_infra_packages/cipd/impl.py

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Implementation of package repository service.
 
 Package and PackageInstance
 ---------------------------
 
 Definitions:
@@ skipping 65 matching lines @@
 ACL changes are applied atomically (e.g. multiple ACLs can be changed all at
 once or none at all), but checks are still only eventually consistent (for
 performance and code simplicity).
 
 TODO(vadimsh): Add fine grain ACL for tags. Tags that are set by Buildbot
 builders should not be allowed to set by other WRITERs.
 """
 
 import collections
 import hashlib
+import itertools
 import json
 import logging
 import re
 import webapp2
 
 from google.appengine import runtime
 from google.appengine.api import datastore_errors
 from google.appengine.ext import ndb
 
 from components import auth
@@ skipping 62 matching lines @@
     """Returns Package entity if it exists.
 
     Args:
       package_name: name of the package, e.g. 'infra/tools/cipd'.
 
     Returns:
       Package or None.
     """
     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.
 
     Args:
       package_name: name of the package, e.g. 'infra/tools/cipd'.
       instance_id: identifier of the package instance (SHA1 of package file).
       processor_name: name of the processor to retrieve results of.
 
     Returns:
       ProcessingResult entity or None.
@@ skipping 601 matching lines @@
         processors=payload['processors'])
 
 
 def get_backend_routes():  # pragma: no cover
   """Returns a list of webapp2.Route to add to backend WSGI app."""
   return [
     webapp2.Route(
         r'/internal/taskqueue/cipd-process/<instance_id:.+>',
         ProcessTaskQueueHandler),
   ]