Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add dashboard search filter support #112

Merged
merged 9 commits into from
Jun 16, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions search_service/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
from flasgger import Swagger

from search_service.api.dashboard import SearchDashboardAPI
from search_service.api.table import SearchTableAPI, SearchTableFilterAPI
from search_service.api.table import SearchTableAPI
from search_service.api.filter import SearchTableFilterAPI, SearchDashboardFilterAPI
from search_service.api.user import SearchUserAPI
from search_service.api.document import DocumentUserAPI, DocumentTableAPI, DocumentTablesAPI, DocumentUsersAPI
from search_service.api.healthcheck import healthcheck
Expand Down Expand Up @@ -81,17 +82,20 @@ def create_app(*, config_module_class: str) -> Flask:
api_bp.add_url_rule('/healthcheck', 'healthcheck', healthcheck)
api = Api(api_bp)
# Table Search API
# TODO: Rename endpoint to be more generic and accept a resource type so that logic can be re-used

api.add_resource(SearchTableFilterAPI, '/search_table')
# TODO: Rename endpoint to be more generic and accept a resource type so that logic can be re-used
ttannis marked this conversation as resolved.
Show resolved Hide resolved
api.add_resource(SearchTableAPI, '/search')

# User Search API
api.add_resource(SearchUserAPI, '/search_user')

# Dashboard Search API
api.add_resource(SearchDashboardAPI, '/search_dashboard')
api.add_resource(SearchDashboardFilterAPI, '/search_dashboard_filter')

# DocumentAPI
# todo: needs to update to handle dashboard/user or other entities use cases.
ttannis marked this conversation as resolved.
Show resolved Hide resolved
api.add_resource(DocumentTablesAPI, '/document_table')
api.add_resource(DocumentTableAPI, '/document_table/<document_id>')

Expand Down
95 changes: 95 additions & 0 deletions search_service/api/filter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
from http import HTTPStatus
from typing import Any, Dict, Iterable # noqa: F401

from flask_restful import Resource, reqparse
from flasgger import swag_from
from marshmallow_annotations.ext.attrs import AttrsSchema

from search_service.api.table import TABLE_INDEX
from search_service.models.table import SearchTableResultSchema
from search_service.models.dashboard import SearchDashboardResultSchema
from search_service.proxy import get_proxy_client


class BaseFilterAPI(Resource):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: how about separate this class into separate file? It just because this file would grow as we introduce more filterable resource.

"""
Base Filter API for search filtering

This API should be generic enough to support every search filter use case.
"""

def __init__(self, *, schema: AttrsSchema) -> None:
self.proxy = get_proxy_client()
self.schema = schema
self.parser = reqparse.RequestParser(bundle_errors=True)

self.parser.add_argument('index', required=False, default=TABLE_INDEX, type=str)
ttannis marked this conversation as resolved.
Show resolved Hide resolved
self.parser.add_argument('page_index', required=False, default=0, type=int)
self.parser.add_argument('query_term', required=False, type=str)
self.parser.add_argument('search_request', type=dict)

super(BaseFilterAPI, self).__init__()

def post(self) -> Iterable[Any]:
"""
Fetch search results based on the page_index, query_term, and
search_request dictionary posted in the request JSON.
:return: list of table results. List can be empty if query
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about updating to: return json payload of schema? Also, from the structure of this generic class, I don't think we can assume it's a list.

doesn't match any tables
"""
args = self.parser.parse_args(strict=True)
page_index = args.get('page_index') # type: int

search_request = args.get('search_request') # type: Dict
if search_request is None:
msg = 'The search request payload is not available in the request'
return {'message': msg}, HTTPStatus.BAD_REQUEST

query_term = args.get('query_term') # type: str
if ':' in query_term:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't know this. Why we don't allow this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly due to colon is not allowed or needs to escape in doing query string filter.

msg = 'The query term contains an invalid character'
return {'message': msg}, HTTPStatus.BAD_REQUEST

try:
results = self.proxy.fetch_search_results_with_filter(
search_request=search_request,
query_term=query_term,
page_index=page_index,
index=args['index']
)

return self.schema(many=True).dump(results).data, HTTPStatus.OK
except RuntimeError as e:
raise e


class SearchTableFilterAPI(BaseFilterAPI):
"""
Search Filter for table
"""
def __init__(self) -> None:
super().__init__(schema=SearchTableResultSchema)

@swag_from('swagger_doc/table/search_table_filter.yml')
def post(self) -> Iterable[Any]:
try:
return super().post()
except RuntimeError:
err_msg = 'Exception encountered while processing search request'
return {'message': err_msg}, HTTPStatus.INTERNAL_SERVER_ERROR


class SearchDashboardFilterAPI(BaseFilterAPI):
"""
Search Filter for Dashboard
"""
def __init__(self) -> None:
super().__init__(schema=SearchDashboardResultSchema)

@swag_from('swagger_doc/dashboard/search_dashboard_filter.yml')
def post(self) -> Iterable[Any]:
try:
return super().post()
except RuntimeError:
err_msg = 'Exception encountered while processing search request'
return {'message': err_msg}, HTTPStatus.INTERNAL_SERVER_ERROR
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
Dashboard search
This is used by the frontend API to search dashboard information.
---
tags:
- 'search_dashboard_filter'
paths:
/search_dashboard:
post:
summary: This is used by the frontend API to search dashboard information.
requestBody:
description: The json data passed from the frontend API to execute a search.
required: true
content:
application/json:
schema:
type: object
properties:
index:
type: string
page_index:
type: integer
schema:
type: integer
default: 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm... Not sure what schema with default value 0 is for.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy paste error, will clean up.

query_term:
type: string
search_request:
type: object
responses:
200:
description: dashboard result information with query string
content:
application/json:
schema:
$ref: '#/components/schemas/SearchDashboardResults'
500:
description: Exception encountered while searching
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorResponse'
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Table search
This is used by the frontend API to search table information.
---
tags:
- 'search'
- 'search_table'
paths:
/search_table:
post:
Expand All @@ -15,6 +15,8 @@ paths:
schema:
type: object
properties:
index:
type: string
page_index:
type: integer
schema:
Expand Down
54 changes: 0 additions & 54 deletions search_service/api/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,57 +51,3 @@ def get(self) -> Iterable[Any]:

err_msg = 'Exception encountered while processing search request'
return {'message': err_msg}, HTTPStatus.INTERNAL_SERVER_ERROR


class SearchTableFilterAPI(Resource):
"""
Search Table API using search filtering

This API should be generic enough to support every search filter use case.
TODO: Deprecate the SearchTableFieldAPI for this more flexible API
"""

def __init__(self) -> None:
self.proxy = get_proxy_client()

self.parser = reqparse.RequestParser(bundle_errors=True)

self.parser.add_argument('index', required=False, default=TABLE_INDEX, type=str)
self.parser.add_argument('page_index', required=False, default=0, type=int)
self.parser.add_argument('query_term', required=False, type=str)
self.parser.add_argument('search_request', type=dict)

super(SearchTableFilterAPI, self).__init__()

@swag_from('swagger_doc/table/search_table_filter.yml')
def post(self) -> Iterable[Any]:
"""
Fetch search results based on the page_index, query_term, and
search_request dictionary posted in the request JSON.
:return: list of table results. List can be empty if query
doesn't match any tables
"""
args = self.parser.parse_args(strict=True)
page_index = args.get('page_index') # type: int

search_request = args.get('search_request') # type: Dict
if search_request is None:
msg = 'The search request payload is not available in the request'
return {'message': msg}, HTTPStatus.BAD_REQUEST

query_term = args.get('query_term') # type: str
if ':' in query_term:
msg = 'The query term contains an invalid character'
return {'message': msg}, HTTPStatus.BAD_REQUEST

try:
results = self.proxy.fetch_table_search_results_with_filter(
search_request=search_request,
query_term=query_term,
page_index=page_index,
index=args['index']
)
return SearchTableResultSchema().dump(results).data, HTTPStatus.OK
except RuntimeError:
err_msg = 'Exception encountered while processing search request'
return {'message': err_msg}, HTTPStatus.INTERNAL_SERVER_ERROR
10 changes: 5 additions & 5 deletions search_service/proxy/atlas.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,11 @@ def fetch_table_search_results(self, *,

return SearchTableResult(total_results=approx_count, results=tables)

def fetch_table_search_results_with_filter(self, *,
query_term: str,
search_request: dict,
page_index: int = 0,
index: str = '') -> SearchTableResult:
def fetch_search_results_with_filter(self, *,
query_term: str,
search_request: dict,
page_index: int = 0,
index: str = '') -> SearchTableResult:
"""
Conduct an 'Advanced Search' to narrow down search results with a use of filters.

Expand Down
13 changes: 7 additions & 6 deletions search_service/proxy/base.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from abc import ABCMeta, abstractmethod
from typing import Any, Dict, List
from typing import Any, Dict, List, Union

from search_service.models.dashboard import SearchDashboardResult
from search_service.models.table import SearchTableResult
Expand Down Expand Up @@ -45,11 +45,12 @@ def delete_document(self, *,
pass

@abstractmethod
def fetch_table_search_results_with_filter(self, *,
query_term: str,
search_request: dict,
page_index: int = 0,
index: str = '') -> SearchTableResult:
def fetch_search_results_with_filter(self, *,
query_term: str,
search_request: dict,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we validate if the key is filterable key? For example, if client sends some unknown key, how search service would react?

example: { 'bogus': ['mode'] }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

page_index: int = 0,
index: str = '') -> Union[SearchTableResult,
SearchDashboardResult]:
pass

@abstractmethod
Expand Down
Loading