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.

Sign up for GitHub

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

WP-379: Error handling for site search results #925

Merged
merged 4 commits into from
Dec 21, 2023
Merged

WP-379: Error handling for site search results #925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1dc28e9
WP-379: Error handling for site search results
chandra-tacc Dec 19, 2023
2855641
Merge branch 'main' into bug/WP-379
edmondsgarrett Dec 20, 2023
44f86ae
Merge branch 'main' into bug/WP-379
chandra-tacc Dec 21, 2023
f603807
Merge branch 'main' into bug/WP-379
edmondsgarrett Dec 21, 2023
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
284 changes: 189 additions & 95 deletions server/portal/apps/site_search/api/unit_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,156 +1,250 @@
from mock import MagicMock
from tapipy.errors import BaseTapyException
import pytest


@pytest.fixture
def mock_dsl_search(mocker):
yield mocker.patch('portal.apps.site_search.api.views.Search')
yield mocker.patch("portal.apps.site_search.api.views.Search")


@pytest.fixture
def mock_file_search(mocker):
yield mocker.patch('portal.apps.site_search.api.views.search_operation')
yield mocker.patch("portal.apps.site_search.api.views.search_operation")


@pytest.fixture
def mock_cms_search(mocker):
mocked_fn = mocker.patch('portal.apps.site_search.api.views.cms_search')
mocked_fn.return_value = (1, [{'title': 'test res',
'highlight': []}])
mocked_fn = mocker.patch("portal.apps.site_search.api.views.cms_search")
mocked_fn.return_value = (1, [{"title": "test res", "highlight": []}])
yield mocked_fn


@pytest.fixture
def mock_service_account(mocker):
yield mocker.patch('portal.apps.site_search.api.views.service_account', autospec=True)
yield mocker.patch(
"portal.apps.site_search.api.views.service_account", autospec=True
)


@pytest.fixture
def mock_files_search(mocker):
mocked_fn = mocker.patch('portal.apps.site_search.api.views.files_search')
mocked_fn = mocker.patch("portal.apps.site_search.api.views.files_search")

mocked_fn.return_value = (1, [{'name': 'testfile',
'path': '/path/to/testfile'}])
mocked_fn.return_value = (1, [{"name": "testfile", "path": "/path/to/testfile"}])
yield mocked_fn


@pytest.fixture
def configure_public(settings):
settings.PORTAL_DATAFILES_STORAGE_SYSTEMS = [
{
'name': 'Community Data',
'system': 'portal.storage.community',
'scheme': 'community',
'api': 'tapis',
'icon': None,
'siteSearchPriority': 1
"name": "Community Data",
"system": "portal.storage.community",
"scheme": "community",
"api": "tapis",
"icon": None,
"siteSearchPriority": 1,
},
{
'name': 'Public Data',
'system': 'portal.storage.public',
'scheme': 'public',
'api': 'tapis',
'icon': None,
'siteSearchPriority': 0
}
"name": "Public Data",
"system": "portal.storage.public",
"scheme": "public",
"api": "tapis",
"icon": None,
"siteSearchPriority": 0,
},
]


def test_search_with_auth(regular_user, client, mock_cms_search,
mock_files_search):
def test_search_with_auth(regular_user, client, mock_cms_search, mock_files_search):
regular_user.profile.setup_complete = True
regular_user.profile.save()
client.force_login(regular_user)
response = client.get('/api/site-search/?page=0&query_string=test')
response = client.get("/api/site-search/?page=0&query_string=test")

assert response.json() == {
'cms': {'count': 1,
'listing': [{'title': 'test res',
'highlight': []}],
'type': 'cms',
'include': True},
'community': {'count': 1,
'listing': [{'name': 'testfile',
'path': '/path/to/testfile'}],
'type': 'file',
'include': True},
'public': {'count': 1,
'listing': [{'name': 'testfile',
'path': '/path/to/testfile'}],
'type': 'file',
'include': True}}


def test_search_no_auth(client, mock_cms_search, mock_files_search, mock_service_account):
response = client.get('/api/site-search/?page=0&query_string=test')
"cms": {
"count": 1,
"listing": [{"title": "test res", "highlight": []}],
"type": "cms",
"include": True,
},
"community": {
"count": 1,
"listing": [{"name": "testfile", "path": "/path/to/testfile"}],
"type": "file",
"include": True,
},
"public": {
"count": 1,
"listing": [{"name": "testfile", "path": "/path/to/testfile"}],
"type": "file",
"include": True,
},
}


@pytest.mark.parametrize(
"tapis_test_config",
[
{
"public": {
"tapis_error_msg": "SSH_POOL_MISSING_CREDENTIALS",
},
"community": {
"tapis_error_msg": "SSH_POOL_MISSING_CREDENTIALS",
},
"search_result_has_error": False,
},
{
"public": {
"tapis_error_msg": "SSH_FX_PERMISSION_DENIED",
},
"community": {
"tapis_error_msg": "SSH_FX_PERMISSION_DENIED",
},
"search_result_has_error": False,
},
{
"public": {
"tapis_error_msg": "InternalServerError",
},
"community": {
"tapis_error_msg": "InternalServerError",
},
"search_result_has_error": True,
},
{
"public": {
"tapis_error_msg": "InternalServerError",
},
"community": {
"tapis_error_msg": "SSH_FX_PERMISSION_DENIED",
},
"search_result_has_error": True,
},
],
)
def test_search_with_tapis_error(
regular_user, client, mock_cms_search, mocker, tapis_test_config
):
# Test if does not error out when public or community search fails with SSH related errors.
# file search return different error based on the type.
def file_search_side_effect(*args, **kwargs):
system_value = kwargs.get("system")
# Check the specific parameter value and return different results
if system_value == "portal.storage.community":
raise BaseTapyException(tapis_test_config["community"]["tapis_error_msg"])
else:
raise BaseTapyException(tapis_test_config["public"]["tapis_error_msg"])

regular_user.profile.setup_complete = True
regular_user.profile.save()
client.force_login(regular_user)
mocker.patch(
"portal.apps.site_search.api.views.files_search",
side_effect=file_search_side_effect,
)
response = client.get("/api/site-search/?page=0&query_string=test")
# if it is an unhandled error code expect the search to fail.
if tapis_test_config["search_result_has_error"]:
assert response.json() == {"message": "message: InternalServerError"}
else:
assert response.json() == {
"cms": {
"count": 1,
"listing": [{"title": "test res", "highlight": []}],
"type": "cms",
"include": True,
}
}


def test_search_no_auth(
client, mock_cms_search, mock_files_search, mock_service_account
):
response = client.get("/api/site-search/?page=0&query_string=test")

assert response.json() == {
'cms': {'count': 1,
'listing': [{'title': 'test res',
'highlight': []}],
'type': 'cms',
'include': True},
'public': {'count': 1,
'listing': [{'name': 'testfile',
'path': '/path/to/testfile'}],
'type': 'file',
'include': True}}


def test_search_public(client, configure_public, mock_cms_search,
mock_files_search, mock_service_account):
response = client.get('/api/site-search/?page=0&query_string=test')
"cms": {
"count": 1,
"listing": [{"title": "test res", "highlight": []}],
"type": "cms",
"include": True,
},
"public": {
"count": 1,
"listing": [{"name": "testfile", "path": "/path/to/testfile"}],
"type": "file",
"include": True,
},
}


def test_search_public(
client, configure_public, mock_cms_search, mock_files_search, mock_service_account
):
response = client.get("/api/site-search/?page=0&query_string=test")

assert response.json() == {
'cms': {'count': 1,
'listing': [{'title': 'test res',
'highlight': []}],
'type': 'cms',
'include': True},
'public': {'count': 1,
'listing': [{'name': 'testfile',
'path': '/path/to/testfile'}],
'type': 'file',
'include': True}}
"cms": {
"count": 1,
"listing": [{"title": "test res", "highlight": []}],
"type": "cms",
"include": True,
},
"public": {
"count": 1,
"listing": [{"name": "testfile", "path": "/path/to/testfile"}],
"type": "file",
"include": True,
},
}


def test_cms_search_util(mock_dsl_search):
from portal.apps.site_search.api.views import cms_search

dummy_hit = MagicMock()
dummy_hit.to_dict.return_value = {'title': 'test title'}
dummy_hit.meta.highlight.to_dict.return_value = {'body': ['highlight 1']}
dummy_hit.to_dict.return_value = {"title": "test title"}
dummy_hit.meta.highlight.to_dict.return_value = {"body": ["highlight 1"]}

dummy_result = MagicMock()
dummy_result.hits.__iter__.return_value = [dummy_hit]
dummy_result.hits.total.value = 1

mock_dsl_search()\
.query()\
.highlight()\
.highlight()\
.highlight_options()\
.extra()\
.execute.return_value = dummy_result
mock_dsl_search().query().highlight().highlight().highlight_options().extra().execute.return_value = (
dummy_result
)

res = cms_search('test_query', offset=0, limit=10)
assert res == (1, [{'title': 'test title',
'highlight': {'body': ['highlight 1']}}])
res = cms_search("test_query", offset=0, limit=10)
assert res == (1, [{"title": "test title", "highlight": {"body": ["highlight 1"]}}])


def test_file_search_util(mock_file_search, regular_user):
from portal.apps.site_search.api.views import files_search
mock_file_search.return_value = {'count': 1,
'listing':
[{'name': 'testfile',
'path': '/path/to/testfile'}]}
client = regular_user.tapis_oauth.client
res = files_search(client, 'test_query', 'test_system', '/',)

mock_file_search.assert_called_with(client, 'test_system', '/',
query_string='test_query',
filter=None,
offset=0,
limit=10)

assert res == (1, [{'name': 'testfile',
'path': '/path/to/testfile'}])
mock_file_search.return_value = {
"count": 1,
"listing": [{"name": "testfile", "path": "/path/to/testfile"}],
}
client = regular_user.tapis_oauth.client
res = files_search(
client,
"test_query",
"test_system",
"/",
)

mock_file_search.assert_called_with(
client,
"test_system",
"/",
query_string="test_query",
filter=None,
offset=0,
limit=10,
)

assert res == (1, [{"name": "testfile", "path": "/path/to/testfile"}])
16 changes: 15 additions & 1 deletion server/portal/apps/site_search/api/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
from django.conf import settings
from portal.libs.agave.utils import service_account
import logging
from tapipy.errors import BaseTapyException

logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -73,6 +75,8 @@ def get(self, request, *args, **kwargs):
'include': True}
except StopIteration:
pass
except BaseTapyException as e:
self._handle_tapis_ssh_exception(e)

if request.user.is_authenticated and \
request.user.profile.setup_complete:
Expand All @@ -93,5 +97,15 @@ def get(self, request, *args, **kwargs):
'include': True}
except StopIteration:
pass

except BaseTapyException as e:
self._handle_tapis_ssh_exception(e)
return JsonResponse(response)

def _handle_tapis_ssh_exception(self, e):
if 'SSH_POOL_MISSING_CREDENTIALS' in str(e) or 'SSH_FX_PERMISSION_DENIED' in str(e):
# in case of these error types, user is not authenticated
# or does not have access do not fail the entire search
# request, log the issue.
logger.exception("Error retrieving search results due to TAPIS SSH related error: {}".format(str(e)))
else:
raise