Skip to content
Browse files

Merge branch 'master' of github.com:ox-it/moxie

* 'master' of github.com:ox-it/moxie:
  Fix recursion error
  Placed get_places_by_identifiers into own test
  Old API as well as new one, added test to cover both cases.
  Attempt to make code a bit easier to read...
  Places / refactoring service to allow the search of more than one ID.
  • Loading branch information...
2 parents 3455120 + e0a3f1c commit 46bfad09d7761f5d843541927c39f01312a26229 @martinfilliau martinfilliau committed Mar 27, 2013
Showing with 70 additions and 37 deletions.
  1. +29 −23 moxie/places/representations.py
  2. +31 −10 moxie/places/services.py
  3. +1 −2 moxie/places/views.py
  4. +9 −2 moxie/tests/test_places_services.py
View
52 moxie/places/representations.py
@@ -66,29 +66,35 @@ def as_dict(self):
poi_service = POIService.from_context()
except NoConfiguredService:
poi_service = None
- if self.poi.parent:
- if poi_service and self.add_parent_children_links:
- parent = poi_service.get_place_by_identifier(self.poi.parent)
- else:
- parent = None
- if parent and parent.name:
- representation.add_link('parent', url_for(self.endpoint, ident=self.poi.parent),
- title=parent.name, type=parent.type, type_name=parent.type_name)
- else:
- representation.add_link('parent', url_for(self.endpoint, ident=self.poi.parent))
-
- if len(self.poi.children) > 0:
- # TODO GET with multiple documents, to do at service level
- for child in self.poi.children:
- if poi_service and self.add_parent_children_links:
- p = poi_service.get_place_by_identifier(child)
- else:
- p = None
- if p and p.name:
- representation.update_link('child', url_for(self.endpoint, ident=child),
- title=p.name, type=p.type, type_name=p.type_name)
- else:
- representation.update_link('child', url_for(self.endpoint, ident=child))
+ if poi_service and self.add_parent_children_links:
+ # Merging all IDs (parent and children) into one set to
+ # do only one query to the service
+ pois_ids = set(self.poi.children)
+ if self.poi.parent:
+ pois_ids.add(self.poi.parent)
+ if pois_ids:
+ pois_objects = poi_service.get_places_by_identifiers(pois_ids)
+ # ease lookup by having a dict with ID as key
+ pois = dict((poi.id, poi) for poi in pois_objects)
+
+ def add_link(relation, identifier):
+ """Add a link w/ or w/o title depending if we found a POI
+ :param relation: link rel (parent or child)
+ :param identifier: ID of the POI for lookup
+ """
+ poi = pois[identifier]
+ if poi and poi.name:
+ representation.add_link(relation, url_for(self.endpoint, ident=identifier),
+ title=poi.name, type=poi.type, type_name=poi.type_name)
+ else:
+ representation.add_link(relation, url_for(self.endpoint, ident=identifier))
+
+ if self.poi.parent:
+ add_link('parent', self.poi.parent)
+
+ if self.poi.children:
+ for child in self.poi.children:
+ add_link('child', child)
try:
transport_service = TransportService.from_context()
View
41 moxie/places/services.py
@@ -82,28 +82,49 @@ def get_results(self, original_query, location, start, count, type=None,
return [doc_to_poi(r) for r in response.results], response.size, facets
def get_place_by_identifier(self, ident):
- """Get a place by its main identifier
- Search in all identifiers if not found.
+ """Get place by identifier
:param ident: identifier to lookup
- :return POI or None if no result
+ :return one POI or None if no result
+ """
+ results = self.get_places_by_identifiers([ident])
+ if results:
+ return results[0]
+ else:
+ return None
+
+ def get_places_by_identifiers(self, idents):
+ """Get places by identifiers
+ Search in all identifiers if not found.
+ :param idents: identifiers to lookup
+ :return list of POI or None if no result
"""
- response = searcher.get_by_ids([ident])
- # First do a GET request by its ID
+ response = searcher.get_by_ids(idents)
+ # First do a GET request by IDs
if response.results:
- return doc_to_poi(response.results[0])
+ return [doc_to_poi(result) for result in response.results]
else:
# If no result, do a SEARCH request on IDs
- return self.search_place_by_identifier(ident)
-
+ return self.search_places_by_identifiers(idents)
def search_place_by_identifier(self, ident):
+ """Search for a place by an identifier
+ :param ident: identifier to lookup
+ :return a POI or None if no result
+ """
+ results = self.search_places_by_identifiers([ident])
+ if results:
+ return results[0]
+ else:
+ return None
+
+ def search_places_by_identifiers(self, idents):
"""Search for a place by its identifiers
:param ident: identifier to lookup
:return POI or None if no result
"""
- response = searcher.search_for_ids("identifiers", [ident])
+ response = searcher.search_for_ids("identifiers", idents)
if response.results:
- return doc_to_poi(response.results[0])
+ return [doc_to_poi(result) for result in response.results]
else:
return None
View
3 moxie/places/views.py
@@ -38,8 +38,7 @@ def handle_request(self):
# useful when querying for bus stop naptan number
# TODO pass the location to have the distance from the point
if ' ' not in self.query:
- unique_doc = poi_service.search_place_by_identifier(
- '*:{id}'.format(id=self.query))
+ unique_doc = poi_service.search_places_by_identifiers(['*:{id}'.format(id=self.query)])
if unique_doc:
self.size = 1
self.facets = None
View
11 moxie/tests/test_places_services.py
@@ -10,6 +10,13 @@ def test_get_results_by_id(self):
with mock.patch('moxie.places.services.doc_to_poi') as mock_doc_to_poi:
poi_service = POIService()
mock_doc_to_poi.return_value = None
- results = poi_service.get_place_by_identifier('123')
-
+ poi_service.get_place_by_identifier('123')
mock_searcher.get_by_ids.assert_called_with(['123'])
+
+ def test_get_results_by_multiple_ids(self):
+ with mock.patch('moxie.places.services.searcher') as mock_searcher:
+ with mock.patch('moxie.places.services.doc_to_poi') as mock_doc_to_poi:
+ poi_service = POIService()
+ mock_doc_to_poi.return_value = None
+ poi_service.get_places_by_identifiers(['123', '456'])
+ mock_searcher.get_by_ids.assert_called_with(['123', '456'])

0 comments on commit 46bfad0

Please sign in to comment.
Something went wrong with that request. Please try again.