Merge lp:~rharding/charmworld/total-stats into lp:~juju-jitsu/charmworld/trunk

[Richard Harding]

Add support for a total downloads attribute.

Add support for a total downloads attribute.

- Ingest pulls the total downloads as well as the last 30 days
- Model and tests are updated to deal with the new attribute
- A migration is put into place to init the new attribute = to the last 30 days count until next ingest runs
- Sort is updated to use the new attribute for determining most popular vs the old 30 day count.

[lp:~rharding/charmworld/total-stats]

291. By Richard Harding on 2013-07-01

Fix lint errors

[Curtis Hovey (sinzui)]

Thank you. I think abentley must reconcile the new attr with this forthcoming branch that documents DISTRO_CHARM_DATA in charmworld/views/tests/test_charms.py. This is good to land now.

review: Approve (code)

=== modified file 'charmworld/charmstore.py'
--- charmworld/charmstore.py	2013-04-22 14:55:43 +0000
+++ charmworld/charmstore.py	2013-07-01 16:01:33 +0000
@@ -46,7 +46,7 @@
         return date.strftime('%Y-%m-%d')
 
     @classmethod
-    def get_stats_range_request(cls, charm, start, end):
+    def get_stats_range_request(cls, charm, start=None, end=None):
         """Produce a Request for retrieving download counts in a range.
 
         :param charm: The charm to get information about.
@@ -56,13 +56,19 @@
         elements = ['charm-bundle', charm['series'], charm['name']]
         if charm['store_url'].startswith('cs:~'):
             elements.append(charm['owner'])
-        return Request(
-            'GET', '%s/%s' % (cls.STAT_URL, ':'.join(elements)), params={
+        params = {
+            'format': 'json',
+        }
+        if (start and end):
+            params.update({
                 'start': cls.date_str(start),
                 'end': cls.date_str(end),
                 'by': 'day',
-                'format': 'json',
-            }
+            })
+
+        return Request(
+            'GET', '%s/%s' % (cls.STAT_URL, ':'.join(elements)),
+            params=params
         )
 
     def get_charm_info(self, address):
@@ -72,7 +78,7 @@
         data['address'] = address
         return data
 
-    def get_download_counts(self, charm, start, end):
+    def get_download_counts(self, charm, start=None, end=None):
         """Retrieve download counts for each day in the specified range.
 
         :param charm: The charm to get information about.

=== modified file 'charmworld/jobs/ingest.py'
--- charmworld/jobs/ingest.py	2013-06-27 14:17:46 +0000
+++ charmworld/jobs/ingest.py	2013-07-01 16:01:33 +0000
@@ -336,7 +336,7 @@
             charm['test_results'][p] = result_id
         except:
             log.exception("Unknown error while processing %s %s",
-                               charm['branch_spec'], p)
+                          charm['branch_spec'], p)
 
 
 def scan_artifacts(provider, charm, result):
@@ -602,6 +602,8 @@
 def update_download_count(store, charm):
     count = store.count_downloads_in_days(charm, 30, date.today())
     charm['downloads_in_past_30_days'] = count
+    count = store.get_download_counts(charm)
+    charm['downloads'] = count[0][0] if count else 0
 
 
 def update_date_created(charm, log):

=== modified file 'charmworld/jobs/tests/test_cstat.py'
--- charmworld/jobs/tests/test_cstat.py	2013-06-17 15:02:59 +0000
+++ charmworld/jobs/tests/test_cstat.py	2013-07-01 16:01:33 +0000
@@ -32,15 +32,20 @@
             @staticmethod
             def send(request):
                 response = Response()
-                response._content = '[["2010-12-24", 4], ["2010-12-25", 1]]'
+                if ('start' in request.url):
+                    response._content = '[["2010-12-24", 4], ["2010-12-25", 1]]'
+                else:
+                    response._content = '[[25]]'
                 return response
 
         store.session = FakeSession
         update_counts(CharmSource(self.db, index_client), store, log)
         db_charm = self.db.charms.find_one({'_id': charm_id})
         self.assertEqual(5, db_charm['downloads_in_past_30_days'])
+        self.assertEqual(25, db_charm['downloads'])
         index_charm = index_client.get(charm_id)
         self.assertEqual(5, index_charm['downloads_in_past_30_days'])
+        self.assertEqual(25, index_charm['downloads'])
 
     def test_update_no_index(self):
         charm_id, charm = factory.makeCharm(self.db)

=== modified file 'charmworld/models.py'
--- charmworld/models.py	2013-06-27 09:25:59 +0000
+++ charmworld/models.py	2013-07-01 16:01:33 +0000
@@ -213,6 +213,7 @@
         'files': {},
 
         # Provided by the charm store job.
+        'downloads': 0,
         'downloads_in_past_30_days': 0,
         'store_data': {},
         'store_url': '',
@@ -415,6 +416,13 @@
         return self._representation['store_data']
 
     @property
+    def downloads(self):
+        """The number of times the charm was downloaded.
+
+        The number comes from the store. It does not equate to deploys."""
+        return self._representation['downloads']
+
+    @property
     def downloads_in_past_30_days(self):
         """The number of times the charm was downloaded in the past 30 days.
 

=== modified file 'charmworld/search.py'
--- charmworld/search.py	2013-06-27 09:25:59 +0000
+++ charmworld/search.py	2013-07-01 16:01:33 +0000
@@ -287,9 +287,8 @@
         :param valid_charms_only: If True, only charms that were successfully
             parsed are returned.
         :param sort: If None, charms are sorted by score.  If 'downloaded',
-            charms are in order of most to least downloaded in the past 30
-            days.  If 'new', charms are in order of most-recently to
-            least-recently created.
+            charms are in order of most to least downloaded.  If 'new', charms
+            are in order of most-recently to least-recently created.
         """
         if filters is None:
             filters = {}
@@ -298,7 +297,7 @@
         dsl = self._get_filtered(dsl, filters, type_, valid_charms_only)
         if sort == 'downloaded':
             dsl['sort'] = [{
-                'downloads_in_past_30_days': {'order': 'desc'},
+                'downloads': {'order': 'desc'},
             }]
         elif sort == 'new':
             dsl['sort'] = [{

=== modified file 'charmworld/testing/factory.py'
--- charmworld/testing/factory.py	2013-06-21 20:45:40 +0000
+++ charmworld/testing/factory.py	2013-07-01 16:01:33 +0000
@@ -141,9 +141,10 @@
                    summary=None, revno=12, maintainer=None,
                    commit_message='maintainer', provides=None, requires=None,
                    options=None, files=None, charm_error=False,
-                   date_created=None, downloads_in_past_30_days=0,
-                   is_featured=False, promulgated=False, categories=None,
-                   branch_deleted=False, proof=None, payload=None):
+                   date_created=None, downloads=0,
+                   downloads_in_past_30_days=0, is_featured=False,
+                   promulgated=False, categories=None, branch_deleted=False,
+                   proof=None, payload=None):
     """Return the json of a charm."""
     if not description:
         description = """byobu-class provides remote terminal access through
@@ -253,6 +254,7 @@
             'options': options,
         },
         'files': files,
+        'downloads': downloads,
         'downloads_in_past_30_days': downloads_in_past_30_days,
         'date_created': date_created.isoformat(),
     })

=== modified file 'charmworld/tests/test_charmstore.py'
--- charmworld/tests/test_charmstore.py	2013-04-10 23:01:10 +0000
+++ charmworld/tests/test_charmstore.py	2013-07-01 16:01:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013 Canonical Ltd.  This software is licensed under the
+
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -48,6 +48,21 @@
             'https://store.juju.ubuntu.com/stats/counter/charm-bundle:foo:bar',
             url)
 
+    def test_get_stats_range_request_wo_dates(self):
+        """The start and end dates are optional
+
+        It will fetch total counts without the start and end dates.
+
+        """
+        charm = factory.get_charm_json(series='foo', name='bar', owner='baz')
+        cs = CharmStore()
+        req = cs.get_stats_range_request(charm)
+        self.assertEqual('GET', req.method)
+        self.assertStoreURL('/charm-bundle:foo:bar:baz', req.url)
+        self.assertEqual({
+            'format': 'json'
+        }, req.params)
+
     def test_get_download_counts(self):
         charm = factory.get_charm_json(series='foo', name='bar', owner='baz')
         start = date(2011, 01, 02)

=== modified file 'charmworld/tests/test_models.py'
--- charmworld/tests/test_models.py	2013-06-27 15:16:03 +0000
+++ charmworld/tests/test_models.py	2013-07-01 16:01:33 +0000
@@ -192,6 +192,15 @@
         charm = Charm({})
         self.assertEquals({}, charm.files)
 
+    def test_downloads(self):
+        """The downloads property is an int."""
+        charm_data = {'downloads': 30}
+        charm = Charm(charm_data)
+        self.assertEqual(30, charm.downloads)
+        # The default is zero.
+        charm = Charm({})
+        self.assertEquals(0, charm.downloads)
+
     def test_downloads_in_past_30_days(self):
         # The downloads_in_past_30_days property is an int.
         charm_data = {'downloads_in_past_30_days': 30}

=== modified file 'charmworld/tests/test_search.py'
--- charmworld/tests/test_search.py	2013-06-27 15:32:30 +0000
+++ charmworld/tests/test_search.py	2013-07-01 16:01:33 +0000
@@ -164,7 +164,7 @@
         self.index_client.index_charm(charm)
         for key, query in items:
             # Skip ints and lists.
-            if key in ('revision', 'downloads_in_past_30_days'):
+            if key in ('revision', 'downloads_in_past_30_days', 'downloads'):
                 continue
             elif key in ('hooks', 'i_requires', 'i_provides', 'categories'):
                 query = query[0]
@@ -433,9 +433,9 @@
 
     def test_search_charm_sort_most_downloaded(self):
         """Specifying 'downloaded' sorts results appropriately."""
-        self.makeCharm(name='popular1', downloads_in_past_30_days=3)
-        self.makeCharm(name='popular2', downloads_in_past_30_days=2)
-        self.makeCharm(name='popular3', downloads_in_past_30_days=1)
+        self.makeCharm(name='popular1', downloads=3)
+        self.makeCharm(name='popular2', downloads=2)
+        self.makeCharm(name='popular3', downloads=1)
         order123 = ['popular1', 'popular2', 'popular3']
         result = self.index_client.api_search()
         self.assertNotEqual(order123, [r['name'] for r in result])

=== modified file 'charmworld/views/api.py'
--- charmworld/views/api.py	2013-06-18 19:40:55 +0000
+++ charmworld/views/api.py	2013-07-01 16:01:33 +0000
@@ -169,6 +169,7 @@
             'name': 'name',
             'description': 'description',
             'owner': 'owner',
+            'downloads': 'downloads',
             'downloads_in_past_30_days': 'downloads_in_past_30_days',
             'distro_series': 'series',
             'revision': 'revision',
@@ -226,6 +227,7 @@
             'name': charm.name,
             'has_icon': icon_key in charm.files,
             'categories': charm.categories,
+            'downloads': charm.downloads,
             'downloads_in_past_30_days': charm.downloads_in_past_30_days,
             'commits_in_past_30_days': len(charm.changes_since(since)),
             'weight': weight,

=== modified file 'charmworld/views/tests/test_api.py'
--- charmworld/views/tests/test_api.py	2013-06-18 19:40:55 +0000
+++ charmworld/views/tests/test_api.py	2013-07-01 16:01:33 +0000
@@ -276,7 +276,7 @@
         for num in range(15):
             dt = datetime.utcfromtimestamp(0)
             last_popular_charm = self.makeCharm(
-                downloads_in_past_30_days=num, name='pop%d' % num,
+                downloads=num, name='pop%d' % num,
                 date_created=dt)[1]
         for num in range(15):
             dt = datetime.utcfromtimestamp(0)
@@ -451,6 +451,7 @@
                 },
                 'files': ['README', 'hooks/install'],
                 'tested_providers': {},
+                'downloads': 0,
                 'downloads_in_past_30_days': 0,
                 'is_subordinate': False,
             },
@@ -507,6 +508,7 @@
                 'options': self.options2,
                 'files': ['README.md', 'hooks/config-changed'],
                 'tested_providers': {},
+                'downloads': 0,
                 'downloads_in_past_30_days': 0,
                 'is_subordinate': True,
             },
@@ -628,9 +630,12 @@
     def test_format_charm_respects_download_count(self):
         charm = factory.get_charm_json()
         formatted = self.api_class._format_charm(Charm(charm))
+        self.assertEqual(0, formatted['downloads'])
         self.assertEqual(0, formatted['downloads_in_past_30_days'])
+        charm['downloads'] = 30
         charm['downloads_in_past_30_days'] = 20
         formatted = self.api_class._format_charm(Charm(charm))
+        self.assertEqual(30, formatted['downloads'])
         self.assertEqual(20, formatted['downloads_in_past_30_days'])
 
     def test_format_charm_tested_providers(self):
@@ -954,16 +959,20 @@
         self.assertIn('charm', result)
 
     def test_format_related(self):
-        charm = factory.get_charm_json(downloads_in_past_30_days=57)
+        charm = factory.get_charm_json(
+            downloads=100,
+            downloads_in_past_30_days=57)
         formatted = self.api_class._format_related(charm, 0)
-        self.assertItemsEqual(['id', 'name', 'categories', 'weight',
-                               'has_icon', 'downloads_in_past_30_days',
-                               'commits_in_past_30_days', 'is_approved'],
-                              formatted.keys())
+        self.assertItemsEqual([
+            'id', 'name', 'categories', 'weight', 'has_icon', 'downloads',
+            'downloads_in_past_30_days', 'commits_in_past_30_days',
+            'is_approved'
+        ], formatted.keys())
         self.assertEqual(
             self.api_class._get_api_id(Charm(charm)), formatted['id'])
         self.assertEqual(charm['name'], formatted['name'])
         self.assertEqual(charm['categories'], formatted['categories'])
+        self.assertEqual(charm['downloads'], 100)
         self.assertEqual(charm['downloads_in_past_30_days'], 57)
 
     def test_format_related_is_approved(self):

=== modified file 'charmworld/views/tests/test_charms.py'
--- charmworld/views/tests/test_charms.py	2013-06-25 08:59:50 +0000
+++ charmworld/views/tests/test_charms.py	2013-07-01 16:01:33 +0000
@@ -248,9 +248,9 @@
         # Charms with errors are not returned by interface().
         providing = Charm(factory.makeCharm(
             self.db, provides={'foo': {'interface': 'httpX'}})[1])
-        requiring = Charm(factory.makeCharm(
-            self.db, requires={'bar': {'interface': 'httpX'}},
-            charm_error=True)[1])
+        Charm(factory.makeCharm(
+              self.db, requires={'bar': {'interface': 'httpX'}},
+              charm_error=True)[1])
         request = self.getRequest()
         request.matchdict = {'interface': 'httpX'}
         response = interface(request)

=== modified file 'charmworld/views/tests/test_search.py'
--- charmworld/views/tests/test_search.py	2013-06-25 13:14:10 +0000
+++ charmworld/views/tests/test_search.py	2013-07-01 16:01:33 +0000
@@ -11,7 +11,7 @@
 from charmworld.testing import (
     factory,
     ViewTestBase,
-    )
+)
 from charmworld.views import search
 
 

=== modified file 'docs/api.rst'
--- docs/api.rst	2013-06-17 19:26:16 +0000
+++ docs/api.rst	2013-07-01 16:01:33 +0000
@@ -69,9 +69,9 @@
 The ``result`` is a mapping of ``new``, ``featured`` and ``popular`` charms.
 ``new`` is a list of the 10 most recently-created charms (according to their
 Launchpad branches), from most to least-recently created.  ``popular`` is a
-list of the 10 most-downloaded charms in the past 30 days, from most to
-least-downloaded.  ``featured`` is a list of all charms that have been manually
-selected as ``featured``, in unspecified order.
+list of the 10 most-downloaded charms., from most to least-downloaded.
+``featured`` is a list of all charms that have been manually selected as
+``featured``, in unspecified order.
 
 Retrieving an individual charm
 ==============================
@@ -120,6 +120,8 @@
     },
     /* The URL of the charm in the store. */
     "url": "cs:precise/apache2-10",
+    /* The number of downloads of this charm */
+    "downloads": 51,
     /* The number of downloads of this charm in the past 30 days */
     "downloads_in_past_30_days": 41,
     /* See Terminology_ */
@@ -251,6 +253,8 @@
     "has_icon": true,
     /* The categories associated with the charm. */
     "categories": [],
+    /* The number of downloads of this charm */
+    "downloads": 51,
     /* The number of downloads of this charm in the past 30 days */
     "downloads_in_past_30_days": 41,
     /* The number of commits to this charm in the past 30 days */

=== added file 'migrations/versions/009_add_downloads_attribute.py'
--- migrations/versions/009_add_downloads_attribute.py	1970-01-01 00:00:00 +0000
+++ migrations/versions/009_add_downloads_attribute.py	2013-07-01 16:01:33 +0000
@@ -0,0 +1,18 @@
+# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+from charmworld.search import reindex
+
+
+def upgrade(db, index_client):
+    """Add the downloads attribute to each charm.
+
+    Default to using their 30day acount and it'll be updated to be more
+    accurate on the next injest run.
+
+    """
+    charms = db.charms.find({})
+    for charm in charms:
+        charm['downloads'] = charm['downloads_in_past_30_days']
+        db.charms.save(charm)
+
+    reindex(index_client, list(db.charms.find({})))