Merge lp:~jcsackett/charmworld/keyerror-promulgated into lp:~juju-jitsu/charmworld/trunk

[j.c.sackett]

Removes old search_json view and related items.

charmworld/views/search.py
--------------------------
`search_json` is removed--it is no longer used by any clients, in favor of the
API search endpoint. Use of this method by bots was the cause of the error in
the linked bug.

charmworld/views/helper.py
--------------------------
The `result_sorter` function is deleted; it was only used by search_json.

Misc
----
Tests for deleted code removed.
Route for deleted code removed.

[Curtis Hovey (sinzui)]

If we changed route to
    config.add_route("search-json-obsolete", "/search/json")
then added it to the obsolete view in charmworld/views/api.py
    @view_config(route_name="search-json-obsolete")
    @view_config(route_name="api-obsolete-0")
    @view_config(route_name="api-obsolete-1")
    def obsolete(request):
        ....
We could clearly state the feature is 410 Gone. It wont show up in a list of 404s. Is there any reason that 410 is wrong? Do we want old clients to see 404?

review: Needs Information (code)

[j.c.sackett (jcsackett)]

No, I think that's fine. I've pushed up changes to that effect.

[lp:~jcsackett/charmworld/keyerror-promulgated]

406. By j.c.sackett on 2013-10-02

Obsolete url rather than 404.

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

[Juju Gui Bot (juju-gui-bot)]

review: Approve (continuous-integration)

=== modified file 'charmworld/routes.py'
--- charmworld/routes.py	2013-09-30 15:20:32 +0000
+++ charmworld/routes.py	2013-10-02 15:48:32 +0000
@@ -91,7 +91,7 @@
 
     # Search
     config.add_route("search", "/search")
-    config.add_route("search-json", "/search/json")
+    config.add_route("search-json-obsolete", "/search/json")
 
     # Home page
     config.add_route("home", "/")

=== modified file 'charmworld/views/api.py'
--- charmworld/views/api.py	2013-09-30 14:16:56 +0000
+++ charmworld/views/api.py	2013-10-02 15:48:32 +0000
@@ -73,6 +73,7 @@
                     status_code=status)
 
 
+@view_config(route_name="search-json-obsolete")
 @view_config(route_name="api-obsolete-0")
 @view_config(route_name="api-obsolete-1")
 def obsolete(request):

=== modified file 'charmworld/views/helpers.py'
--- charmworld/views/helpers.py	2013-09-04 14:48:30 +0000
+++ charmworld/views/helpers.py	2013-10-02 15:48:32 +0000
@@ -99,20 +99,6 @@
     return n.split("<", 1)[0].strip()
 
 
-def result_sorter(a, b):
-    review_cmp = cmp(b['promulgated'], a['promulgated'])
-    if review_cmp != 0:
-        return review_cmp
-
-    from charmworld.views import SERIES_SORT
-    series = cmp(SERIES_SORT.index(a['series']),
-                 SERIES_SORT.index(b['series']))
-    if series != 0:
-        return series
-
-    return cmp(b['relevance'], a['relevance'])
-
-
 def sub_filter(d, req):
     if req.GET.get('sub'):
         d['subordinate'] = True

=== modified file 'charmworld/views/search.py'
--- charmworld/views/search.py	2013-08-01 20:07:38 +0000
+++ charmworld/views/search.py	2013-10-02 15:48:32 +0000
@@ -1,21 +1,14 @@
 # Copyright 2012, 2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
-
-import json
 from pyramid.httpexceptions import HTTPFound
 from pyramid.renderers import render_to_response
 from pyramid.view import view_config
-from webob import Response
 
 from charmworld.search import (
-    CHARM,
     IndexNotReady,
     SearchServiceNotAvailable,
 )
 
-from charmworld.views.api import json_response
-from charmworld.views.helpers import result_sorter
-
 
 def do_search(request):
     return request.index_client.search(request.params["search_text"])
@@ -37,46 +30,3 @@
         return response
     return dict((key, value) for key, value in search.items()
                 if key != 'matches')
-
-
-@view_config(
-    route_name="search-json")
-def search_json(request):
-    try:
-        data = do_search(request)
-    except (IndexNotReady, SearchServiceNotAvailable):
-        return json_response(503, {'type': 'search_not_ready'})
-    data['charm_total'] = data.pop('result_total')
-    results = data['results'][CHARM]
-    charms = []
-    for r in results:
-        charm = r['data']
-        if charm.store_url == '':
-            continue
-
-        charms.append({
-            'store_url': charm.store_url,
-            'name': charm.name,
-            'summary': charm.summary,
-            'series': charm.series,
-            'owner': charm.owner,
-            'data_url': '%s/json' % (
-                # request.host_url,
-                charm.short_url),
-            'relevance': r['weight'],
-        })
-
-        if 'subordinate' in data:
-            charms[-1]['subordinate'] = True
-
-    charms.sort(result_sorter)
-
-    data.update({"results": charms,
-                 "results_size": len(charms)})
-
-    return Response(
-        json.dumps(data, indent=2),
-        headerlist=[
-            ("Access-Control-Allow-Origin", "*"),
-            ("Access-Control-Allow-Headers", "X-Requested-With")],
-        content_type=u"application/json")

=== modified file 'charmworld/views/tests/test_helpers.py'
--- charmworld/views/tests/test_helpers.py	2013-07-31 14:45:14 +0000
+++ charmworld/views/tests/test_helpers.py	2013-10-02 15:48:32 +0000
@@ -5,7 +5,6 @@
 from charmworld.views.helpers import (
     format_change,
     interfaces,
-    result_sorter,
 )
 from charmworld.testing import factory
 from charmworld.testing import MongoTestBase
@@ -119,18 +118,6 @@
         ]
         self.assertEqual(expected, result)
 
-    def test_result_sorter_raring(self):
-        charm_a = factory.get_charm_json(series='precise')
-        charm_b = factory.get_charm_json(series='raring')
-        with self.assertNoException():
-            result_sorter(charm_a, charm_b)
-
-    def test_result_sorter_saucy(self):
-        charm_a = factory.get_charm_json(series='saucy')
-        charm_b = factory.get_charm_json(series='raring')
-        with self.assertNoException():
-            result_sorter(charm_a, charm_b)
-
     def test_format_change(self):
         charm = Charm(factory.get_charm_json())
         result = format_change(charm.last_change)

=== modified file 'charmworld/views/tests/test_search.py'
--- charmworld/views/tests/test_search.py	2013-08-01 20:07:38 +0000
+++ charmworld/views/tests/test_search.py	2013-10-02 15:48:32 +0000
@@ -9,8 +9,8 @@
     IndexNotReady,
 )
 from charmworld.testing import (
-    factory,
     ViewTestBase,
+    WebTestBase,
 )
 from charmworld.views import search
 
@@ -65,48 +65,10 @@
                                   request=request)
         self.assertEqual(503, response.status_code)
 
-    def test_search_json(self):
-        # The search json view substitutes the search result object with the
-        # charms and adds some metadata.
-        charm = factory.makeCharm(self.db, name='foo')[1]
-        factory.makeCharm(self.db)
-        self.use_index_client()
-        self.index_client.index_charm(charm)
-        terms = {"search_text": 'foo'}
-        request = self.getRequest(params=terms)
-        keys = sorted([
-            'charm_total',
-            'matches',
-            'matches_human_readable_estimate',
-            'results',
-            'results_size',
-            'search_time'])
-        json = search.search_json(request).json
-        self.assertEqual(keys, sorted(json.keys()))
-        self.assertEqual(1, json['charm_total'])
-        self.assertEqual(1, json['matches'])
-        self.assertEqual('1', json['matches_human_readable_estimate'])
-        self.assertEqual(1, json['results_size'])
-        self.assertEqual(1, len(json['results']))
-        found = json['results'][0]
-        self.assertEqual('foo', found['name'])
-        self.assertEqual(charm['series'], found['series'])
-        self.assertEqual(charm['owner'], found['owner'])
-
-    @patch('charmworld.views.search.do_search',
-           autospec=True, side_effect=IndexNotReady)
-    def test_search_json_503(self, mock):
-        terms = {"search_text": 'foo'}
-        request = self.getRequest(params=terms)
-        response = search.search_json(request)
-        self.assertEqual(503, response.status_code)
-        self.assertEqual({'type': 'search_not_ready'}, response.json)
-
-    def test_search_json_503_on_connection_error(self):
-        terms = {"search_text": 'foo'}
-        request = self.getRequest(params=terms)
-        request.index_client = ElasticSearchClient(
-            ElasticSearch(['http://localhost:70']), 'foo')
-        response = search.search_json(request)
-        self.assertEqual(503, response.status_code)
-        self.assertEqual({'type': 'search_not_ready'}, response.json)
+
+class SearchJSONTests(WebTestBase):
+
+    def test_search_json_obsolete(self):
+        obsolete_message = 'no longer supported'
+        resp = self.app.get('/search/json', status=410)
+        self.assertIn(obsolete_message, resp.body)