Skip to content

Commit

Permalink
Minor bugfix - track filter 'simple-joins' to prevent ambiguous repet…
Browse files Browse the repository at this point in the history
…ition.

This was causing bug where specify min AND max declination would result in
an unintended exception.
  • Loading branch information
timstaley committed Jan 25, 2016
1 parent 5e2a9af commit 79c9041
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 6 deletions.
9 changes: 6 additions & 3 deletions voeventdb/server/restapi/v1/filter_base.py
Expand Up @@ -63,10 +63,12 @@ def generate_filter_set(self, args):
for filter_value in args.getlist(self.querystring_key)
)

def apply_filter(self, query, args):
def apply_filter(self, query, args, pre_joins):
if self.simplejoin_tables:
for tbl in self.simplejoin_tables:
query = query.join(tbl)
if tbl not in pre_joins:
query = query.join(tbl)
pre_joins.append(tbl)
query = query.filter(self.generate_filter_set(args))
return query

Expand All @@ -75,10 +77,11 @@ def apply_filters(query, args):
"""
Apply all QueryFilters, validating the querystring in the process.
"""
pre_joins = []
for querystring_key, filter_value in args.iteritems(multi=True):
if querystring_key in filter_registry:
cls_inst = filter_registry[querystring_key]
query = cls_inst.apply_filter(query, args)
query = cls_inst.apply_filter(query, args, pre_joins)
elif querystring_key in PaginationKeys._value_list:
pass
else:
Expand Down
22 changes: 19 additions & 3 deletions voeventdb/server/tests/test_rest/test_restapi.py
Expand Up @@ -302,7 +302,7 @@ def assign_test_client_and_initdb(self, flask_test_client,
self.ivorn_dec_map[pkt.attrib['ivorn']] = posn.dec
fixture_db_session.add(Voevent.from_etree(pkt))

def test_gt(self):
def test_dec_gt(self):
min_dec = -33.2
url = url_for(apiv1.name + '.' + views.ListIvorn.view_name,
**{filters.DecGreaterThan.querystring_key: min_dec}
Expand All @@ -315,7 +315,7 @@ def test_gt(self):
assert len(rd) == len(matching_ivorns)
# print len(rd)

def test_lt(self):
def test_dec_lt(self):
max_dec = -33.2
url = url_for(apiv1.name + '.' + views.ListIvorn.view_name,
**{filters.DecLessThan.querystring_key: max_dec}
Expand All @@ -326,4 +326,20 @@ def test_lt(self):
matching_ivorns = [ivorn for ivorn in self.ivorn_dec_map
if self.ivorn_dec_map[ivorn] < max_dec]
assert len(rd) == len(matching_ivorns)
# print len(rd)

def test_dec_gt_and_lt(self):
max_dec = 33.2
min_dec = 10.2
url = url_for(apiv1.name + '.' + views.ListIvorn.view_name,
**{filters.DecLessThan.querystring_key: max_dec,
filters.DecGreaterThan.querystring_key: min_dec,
}
)
with self.c as c:
rv = self.c.get(url)
rd = json.loads(rv.data)[ResultKeys.result]
matching_ivorns = [ivorn for ivorn in self.ivorn_dec_map
if self.ivorn_dec_map[ivorn] < max_dec
and self.ivorn_dec_map[ivorn] > min_dec
]
assert len(rd) == len(matching_ivorns)

0 comments on commit 79c9041

Please sign in to comment.