From 5ce48de94af460f26b1a15eb3b2385df9bb9c58f Mon Sep 17 00:00:00 2001 From: Martin Zimmermann Date: Tue, 25 Mar 2014 18:38:17 +0100 Subject: [PATCH] add POST request to get comment counts for multiple URLs The old way via `GET /count?uri=...` still works, but is now deprecated and might be removed in future releases. The new way is much more efficient especially fore multiple listings. The internal implemention is improvable though. --- isso/compat.py | 4 ++++ isso/db/comments.py | 16 ++++++++++------ isso/js/app/api.js | 6 ++---- isso/js/app/count.js | 28 +++++++++++++++++++++++++--- isso/views/comments.py | 13 ++++++++++++- specs/test_comments.py | 13 +++++++++++++ 6 files changed, 66 insertions(+), 14 deletions(-) diff --git a/isso/compat.py b/isso/compat.py index ac09826..e12f268 100644 --- a/isso/compat.py +++ b/isso/compat.py @@ -8,6 +8,8 @@ if not PY2K: map, zip, filter = map, zip, filter from functools import reduce + iteritems = lambda dikt: iter(dikt.items()) + text_type = str string_types = (str, ) @@ -18,6 +20,8 @@ else: map, zip, filter = imap, izip, ifilter reduce = reduce + iteritems = lambda dikt: dikt.iteritems() + text_type = unicode string_types = (str, unicode) diff --git a/isso/db/comments.py b/isso/db/comments.py index 6897c97..7d3aa01 100644 --- a/isso/db/comments.py +++ b/isso/db/comments.py @@ -175,14 +175,18 @@ class Comments: return {'likes': likes + 1, 'dislikes': dislikes} return {'likes': likes, 'dislikes': dislikes + 1} - def count(self, uri): + def count(self, *urls): """ - Return comment count for :param:`uri`. + Return comment count for one ore more urls.. """ - return self.db.execute([ - 'SELECT COUNT(comments.id) FROM comments INNER JOIN threads ON', - ' threads.uri=? AND comments.tid=threads.id AND comments.mode=1;'], - (uri, )).fetchone() + + threads = dict(self.db.execute([ + 'SELECT threads.uri, COUNT(comments.id) FROM comments', + 'LEFT OUTER JOIN threads ON threads.id = tid', + 'GROUP BY threads.uri' + ]).fetchall()) + + return [threads.get(url, 0) for url in urls] def purge(self, delta): """ diff --git a/isso/js/app/api.js b/isso/js/app/api.js index f0bc3a8..b2056aa 100644 --- a/isso/js/app/api.js +++ b/isso/js/app/api.js @@ -139,13 +139,11 @@ define(["app/lib/promise", "app/globals"], function(Q, globals) { return deferred.promise; }; - var count = function(tid) { + var count = function(urls) { var deferred = Q.defer(); - curl("GET", endpoint + "/count?" + qs({uri: tid || location}), null, function(rv) { + curl("POST", endpoint + "/count", JSON.stringify(urls), function(rv) { if (rv.status === 200) { deferred.resolve(JSON.parse(rv.body)); - } else if (rv.status === 404) { - deferred.resolve(0); } else { deferred.reject(rv.body); } diff --git a/isso/js/app/count.js b/isso/js/app/count.js index 077b7f5..d1ecd2a 100644 --- a/isso/js/app/count.js +++ b/isso/js/app/count.js @@ -1,5 +1,8 @@ define(["app/api", "app/dom", "app/markup"], function(api, $, Mark) { return function() { + + var objs = {}; + $.each("a", function(el) { if (! el.href.match("#isso-thread$")) { return; @@ -9,9 +12,28 @@ define(["app/api", "app/dom", "app/markup"], function(api, $, Mark) { el.href.match("^(.+)#isso-thread$")[1] .replace(/^.*\/\/[^\/]+/, ''); - api.count(tid).then(function(rv) { - el.textContent = Mark.up("{{ i18n-num-comments | pluralize : `n` }}", {n: rv}); - }); + if (tid in objs) { + objs[tid].push(el); + } else { + objs[tid] = [el]; + } + }); + + var urls = Object.keys(objs); + + api.count(urls).then(function(rv) { + for (var key in objs) { + if (objs.hasOwnProperty(key)) { + + var index = urls.indexOf(key); + + for (var i = 0; i < objs[key].length; i++) { + objs[key][i].textContent = Mark.up( + "{{ i18n-num-comments | pluralize : `n` }}", + {n: rv[index]}); + } + } + } }); }; }); diff --git a/isso/views/comments.py b/isso/views/comments.py index c231ead..d6a2691 100644 --- a/isso/views/comments.py +++ b/isso/views/comments.py @@ -16,7 +16,7 @@ from werkzeug.exceptions import BadRequest, Forbidden, NotFound from isso.compat import text_type as str from isso import utils, local -from isso.utils import http, parse, html, JSONResponse as JSON +from isso.utils import http, parse, JSONResponse as JSON from isso.utils.crypto import pbkdf2 from isso.views import requires @@ -61,6 +61,7 @@ class API(object): ('fetch', ('GET', '/')), ('new', ('POST', '/new')), ('count', ('GET', '/count')), + ('counts', ('POST', '/count')), ('view', ('GET', '/id/')), ('edit', ('PUT', '/id/')), ('delete', ('DELETE', '/id/')), @@ -352,6 +353,7 @@ class API(object): nv = self.comments.vote(False, id, utils.anonymize(str(request.remote_addr))) return JSON(nv, 200) + # TODO: remove someday (replaced by :func:`counts`) @requires(str, 'uri') def count(self, environ, request, uri): @@ -362,5 +364,14 @@ class API(object): return JSON(rv, 200) + def counts(self, environ, request): + + data = request.get_json() + + if not isinstance(data, list) and not all(isinstance(x, str) for x in data): + raise BadRequest("JSON must be a list of URLs") + + return JSON(self.comments.count(*data), 200) + def checkip(self, env, req): return Response(utils.anonymize(str(req.remote_addr)), 200) diff --git a/specs/test_comments.py b/specs/test_comments.py index 9291cf6..f00217c 100644 --- a/specs/test_comments.py +++ b/specs/test_comments.py @@ -22,6 +22,8 @@ from isso import Isso, core from isso.utils import http from isso.views import comments +from isso.compat import iteritems + from fixtures import curl, loads, FakeIP, JSONClient http.curl = curl @@ -274,6 +276,17 @@ class TestComments(unittest.TestCase): rv = self.get('/count?uri=%2Fpath%2F') self.assertEqual(rv.status_code, 404) + def testMultipleCounts(self): + + expected = {'a': 1, 'b': 2, 'c': 0} + + for uri, count in iteritems(expected): + for _ in range(count): + self.post('/new?uri=%s' % uri, data=json.dumps({"text": "..."})) + + rv = self.post('/count', data=json.dumps(list(expected.keys()))) + self.assertEqual(loads(rv.data), list(expected.values())) + def testModify(self): self.post('/new?uri=test', data=json.dumps({"text": "Tpyo"}))