Merge lp:~lifeless/launchpad/bug-727020 into lp:launchpad

[Robert Collins]

[r=stevenk][bug=727020] Eager load most fields for /api/1.0//products. OfficialBugTags are a bit hard for now.

More eager loading. How boring.

[Steve Kowalik (stevenk)]

review: Approve

=== modified file 'lib/lp/bugs/browser/bugalsoaffects.py'
--- lib/lp/bugs/browser/bugalsoaffects.py	2011-03-09 22:07:20 +0000
+++ lib/lp/bugs/browser/bugalsoaffects.py	2011-03-22 08:11:06 +0000
@@ -767,10 +767,10 @@
             # Use a local import as we don't want removeSecurityProxy used
             # anywhere else.
             from zope.security.proxy import removeSecurityProxy
-            name_matches = getUtility(IProductSet).search(
-                self.request.form.get('field.name'))
-            products = bugtracker.products.intersect(
-                removeSecurityProxy(name_matches))
+            name_matches = removeSecurityProxy(
+                getUtility(IProductSet).search_sqlobject(
+                self.request.form.get('field.name')))
+            products = bugtracker.products.intersect(name_matches)
             self.existing_products = list(
                 products[:self.MAX_PRODUCTS_TO_DISPLAY])
         else:

=== modified file 'lib/lp/code/model/branchcollection.py'
--- lib/lp/code/model/branchcollection.py	2011-03-21 03:50:10 +0000
+++ lib/lp/code/model/branchcollection.py	2011-03-22 08:11:06 +0000
@@ -183,6 +183,8 @@
                     cache._associatedProductSeries = []
                 if not safe_hasattr(cache, '_associatedSuiteSourcePackages'):
                     cache._associatedSuiteSourcePackages = []
+                if not safe_hasattr(cache, 'code_import'):
+                    cache.code_import = None
             # associatedProductSeries
             # Imported here to avoid circular import.
             from lp.registry.model.productseries import ProductSeries
@@ -206,10 +208,6 @@
             # need for validity etc in the /branches API call.
             load_related(Person, rows,
                 ['ownerID', 'registrantID', 'reviewerID'])
-            # Cache all branches as having no code imports to prevent fruitless
-            # lookups on the ones we don't find.
-            for cache in caches.values():
-                cache.code_import = None
             for code_import in IStore(CodeImport).find(
                 CodeImport, CodeImport.branchID.is_in(branch_ids)):
                 cache = caches[code_import.branchID]

=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py	2011-02-24 15:30:54 +0000
+++ lib/lp/registry/interfaces/product.py	2011-03-22 08:11:06 +0000
@@ -938,14 +938,20 @@
     @operation_parameters(text=TextLine(title=_("Search text")))
     @operation_returns_collection_of(IProduct)
     @export_read_operation()
-    def search(text=None, soyuz=None,
-               rosetta=None, malone=None,
-               bazaar=None):
+    def search(text=None):
         """Search through the Registry database for products that match the
         query terms. text is a piece of text in the title / summary /
-        description fields of product. soyuz, bazaar, malone etc are
-        hints as to whether the search should be limited to products
-        that are active in those Launchpad applications."""
+        description fields of product.
+
+        This call eager loads data appropriate for web API; caution may be
+        needed for other callers.
+        """
+
+    def search_sqlobject(text):
+        """A compatible sqlobject search for bugalsoaffects.py.
+
+        DO NOT ADD USES.
+        """
 
     @operation_returns_collection_of(IProduct)
     @call_with(quantity=None)

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2011-03-03 16:49:09 +0000
+++ lib/lp/registry/model/product.py	2011-03-22 08:11:06 +0000
@@ -78,6 +78,7 @@
     IStoreSelector,
     MAIN_STORE,
     )
+from canonical.lazr.utils import safe_hasattr
 from lp.answers.interfaces.faqtarget import IFAQTarget
 from lp.answers.interfaces.questioncollection import (
     QUESTION_STATUS_DEFAULT_SEARCH,
@@ -165,6 +166,7 @@
 from lp.registry.model.productseries import ProductSeries
 from lp.registry.model.series import ACTIVE_STATUSES
 from lp.registry.model.sourcepackagename import SourcePackageName
+from lp.services.database import bulk
 from lp.services.propertycache import (
     cachedproperty,
     get_property_cache,
@@ -459,7 +461,6 @@
     bug_reporting_guidelines = StringCol(default=None)
     bug_reported_acknowledgement = StringCol(default=None)
     enable_bugfiling_duplicate_search = BoolCol(notNull=True, default=True)
-    _cached_licenses = None
 
     def _validate_active(self, attr, value):
         # Validate deactivation.
@@ -639,11 +640,6 @@
         self.license_reviewed = False
         self.license_approved = False
 
-    def __storm_invalidated__(self):
-        """Clear cached non-storm attributes when the transaction ends."""
-        super(Product, self).__storm_invalidated__()
-        self._cached_licenses = None
-
     def _get_answers_usage(self):
         if self._answers_usage != ServiceUsage.UNKNOWN:
             # If someone has set something with the enum, use it.
@@ -704,14 +700,16 @@
         _set_translations_usage,
         doc="Indicates if the product uses the translations service.")
 
+    @cachedproperty
+    def _cached_licenses(self):
+        """Get the licenses as a tuple."""
+        product_licenses = ProductLicense.selectBy(
+            product=self, orderBy='license')
+        return tuple(
+            product_license.license
+            for product_license in product_licenses)
+
     def _getLicenses(self):
-        """Get the licenses as a tuple."""
-        if self._cached_licenses is None:
-            product_licenses = ProductLicense.selectBy(
-                product=self, orderBy='license')
-            self._cached_licenses = tuple(
-                product_license.license
-                for product_license in product_licenses)
         return self._cached_licenses
 
     def _setLicenses(self, licenses, reset_license_reviewed=True):
@@ -746,7 +744,7 @@
 
         for license in licenses.difference(old_licenses):
             ProductLicense(product=self, license=license)
-        self._cached_licenses = tuple(sorted(licenses))
+        get_property_cache(self)._cached_licenses = tuple(sorted(licenses))
 
     licenses = property(_getLicenses, _setLicenses)
 
@@ -1575,38 +1573,57 @@
                     Product.datecreated, Product.displayname)
         return result
 
-    def search(self, text=None, soyuz=None,
-               rosetta=None, malone=None,
-               bazaar=None,
-               show_inactive=False):
+    def search(self, text=None):
         """See lp.registry.interfaces.product.IProductSet."""
-        # XXX: kiko 2006-03-22: The soyuz argument is unused.
-        clauseTables = set()
-        clauseTables.add('Product')
-        queries = []
+        # Circular...
+        from lp.registry.model.projectgroup import ProjectGroup
+        conditions = []
+        conditions = [Product.active]
         if text:
-            queries.append("Product.fti @@ ftq(%s) " % sqlvalues(text))
-        if rosetta:
-            clauseTables.add('POTemplate')
-            clauseTables.add('ProductRelease')
-            clauseTables.add('ProductSeries')
-            queries.append("POTemplate.productrelease=ProductRelease.id")
-            queries.append("ProductRelease.productseries=ProductSeries.id")
-            queries.append("ProductSeries.product=product.id")
-        if malone:
-            clauseTables.add('BugTask')
-            queries.append('BugTask.product=Product.id')
-        if bazaar:
-            clauseTables.add('ProductSeries')
-            queries.append('(ProductSeries.branch IS NOT NULL)')
-        if 'ProductSeries' in clauseTables:
-            queries.append('ProductSeries.product=Product.id')
-        if not show_inactive:
-            queries.append('Product.active IS TRUE')
-        query = " AND ".join(queries)
-        return Product.select(query, distinct=True,
-                              prejoins=["_owner"],
-                              clauseTables=clauseTables)
+            conditions.append(
+                SQL("Product.fti @@ ftq(%s) " % sqlvalues(text)))
+        result = IStore(Product).find(Product, *conditions)
+        def eager_load(rows):
+            product_ids = set(obj.id for obj in rows)
+            if not product_ids:
+                return
+            products = dict((product.id, product) for product in rows)
+            caches = dict((product.id, get_property_cache(product))
+                for product in rows)
+            for cache in caches.values():
+                if not safe_hasattr(cache, 'commercial_subscription'):
+                    cache.commercial_subscription = None
+                if not safe_hasattr(cache, '_cached_licenses'):
+                    cache._cached_licenses = []
+            for subscription in IStore(CommercialSubscription).find(
+                CommercialSubscription, 
+                CommercialSubscription.productID.is_in(product_ids)):
+                cache = caches[subscription.productID]
+                cache.commercial_subscription = subscription
+            for license in IStore(ProductLicense).find(
+                ProductLicense,
+                ProductLicense.productID.is_in(product_ids)):
+                cache = caches[license.productID]
+                cache._cached_licenses.append(license.license)
+            for cache in caches.values():
+                cache._cached_licenses = tuple(sorted(cache._cached_licenses))
+            bulk.load_related(ProjectGroup, products.values(), ['projectID'])
+            bulk.load_related(ProductSeries, products.values(),
+                ['development_focusID'])
+            # Only need the objects for canonical_url, no need for validity.
+            bulk.load_related(Person, products.values(),
+                ['_ownerID', 'registrantID', 'bug_supervisorID', 'driverID',
+                 'security_contactID'])
+        return DecoratedResultSet(result, pre_iter_hook=eager_load)
+
+    def search_sqlobject(self, text):
+        """See `IProductSet`"""
+        clauseTables = ['Product']
+        queries = ["Product.fti @@ ftq(%s) " % sqlvalues(text)]
+        queries.append('Product.active IS TRUE')
+        query = "Product.active IS TRUE AND Product.fti @@ ftq(%s)" \
+            % sqlvalues(text)
+        return Product.select(query)
 
     def getTranslatables(self):
         """See `IProductSet`"""