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

Fixes for searches, ajax, waypoints, easyrdf HTTP client errors #1197

Merged
merged 9 commits into from
Sep 9, 2021
Merged

Fixes for searches, ajax, waypoints, easyrdf HTTP client errors #1197

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
dd4c211
fixes fore failing vocabulary/global search; fixes erroring ajax sear…
kouralex Jul 16, 2020
99d8194
catch ValueError instead of Exception, since getVocabulary throws it …
osma Sep 9, 2021
2d5f1ee
fail fast with an error page instead of skipping unknown vocabularies
osma Sep 9, 2021
21ee3c8
Minimal CSS styling for retry button on search result page
osma Sep 9, 2021
22f57ff
Add translations (fi/sv/en) for error message and button shown on sea…
osma Sep 9, 2021
189fe98
Avoid using return value from method that doesn't return anything (bu…
osma Sep 9, 2021
14eac78
use empty() instead of sizeof(xxx) == 0 which is clearer (bug found b…
osma Sep 9, 2021
c777186
Adjust for change to ValueError
osma Sep 9, 2021
ee5e40f
Remove warning message about skipped vocabs (too obtrusive and the us…
osma Sep 9, 2021
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
41 changes: 35 additions & 6 deletions controller/WebController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,23 +329,47 @@ public function invokeGlobalSearch($request)
$vocabObjects = array();
if ($vocids) {
foreach($vocids as $vocid) {
$vocabObjects[] = $this->model->getVocabulary($vocid);
try {
$vocabObjects[] = $this->model->getVocabulary($vocid);
osma marked this conversation as resolved.
Show resolved Hide resolved
} catch (Exception $e) {
osma marked this conversation as resolved.
Show resolved Hide resolved
// skip vocabularies not found in configuration
// please note that this may result in global search
// NB: should not happen in normal UI interaction
}
}
}
$parameters->setVocabularies($vocabObjects);

$nondefaultEndpointVocs = array();
osma marked this conversation as resolved.
Show resolved Hide resolved

if (sizeOf($vocabObjects) != 1) {
// global search, either from all (sizeOf($vocabObjects) == 0) or from selected ones
if (sizeOf($vocabObjects) == 0) {
$vocabObjects = $this->model->getVocabularies();
}
$defaultEndpoint = $this->model->getConfig()->getDefaultEndpoint();
foreach($vocabObjects as $voc) {
if ($voc->getEndpoint() !== $defaultEndpoint) {
$nondefaultEndpointVocs[] = $voc;
}
}
}

$counts = null;
$searchResults = null;
$errored = false;

try {
$countAndResults = $this->model->searchConceptsAndInfo($parameters);
$counts = $countAndResults['count'];
$searchResults = $countAndResults['results'];
} catch (Exception $e) {
header("HTTP/1.0 404 Not Found");
$errored = true;
header("HTTP/1.0 500 Internal Server Error");
if ($this->model->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
$this->invokeGenericErrorPage($request, $e->getMessage());
Copy link
Member Author

Choose a reason for hiding this comment

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

Generic error page is no longer used here, as the search result page can now show a proper warning message.

return;
}
$counts = $countAndResults['count'];
$searchResults = $countAndResults['results'];
$vocabList = $this->model->getVocabularyList();
$sortedVocabs = $this->model->getVocabularyList(false, true);
$langList = $this->model->getLanguages($lang);
Expand All @@ -357,6 +381,8 @@ public function invokeGlobalSearch($request)
'search_results' => $searchResults,
'rest' => $parameters->getOffset()>0,
'global_search' => true,
'search_failed' => $errored,
'skipped_vocabs' => $nondefaultEndpointVocs,
'term' => $request->getQueryParamRaw('q'),
'lang_list' => $langList,
'vocabs' => str_replace(' ', '+', $vocabs),
Expand All @@ -375,6 +401,7 @@ public function invokeVocabularySearch($request)
$template = $this->twig->loadTemplate('vocab-search-listing.twig');
$this->setLanguageProperties($request->getLang());
$vocab = $request->getVocab();
$searchResults = null;
try {
$vocabTypes = $this->model->getTypes($request->getVocabid(), $request->getLang());
} catch (Exception $e) {
Expand All @@ -388,6 +415,7 @@ public function invokeVocabularySearch($request)
'languages' => $this->languages,
'vocab' => $vocab,
'request' => $request,
'search_results' => $searchResults
));

return;
Expand All @@ -411,6 +439,7 @@ public function invokeVocabularySearch($request)
'languages' => $this->languages,
'vocab' => $vocab,
'term' => $request->getQueryParam('q'),
'search_results' => $searchResults
));
return;
}
Expand Down
25 changes: 18 additions & 7 deletions model/DataObject.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,15 @@ public function __construct($model, $resource)
protected function getExternalLabel($exvoc, $exuri, $lang)
{
if ($exvoc) {
$exsparql = $exvoc->getSparql();
$results = $exsparql->queryLabel($exuri, $lang);

return isset($results[$lang]) ? $results[$lang] : null;
try {
Copy link
Member Author

Choose a reason for hiding this comment

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

Here the idea is to catch errors accessing labels from another vocabulary.

$exsparql = $exvoc->getSparql();
$results = $exsparql->queryLabel($exuri, $lang);
return isset($results[$lang]) ? $results[$lang] : null;
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->model->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
}
}
return null;
}
Expand All @@ -63,9 +68,15 @@ protected function getExternalLabel($exvoc, $exuri, $lang)
protected function getExternalNotation($exvoc, $exuri)
{
if ($exvoc) {
$exsparql = $exvoc->getSparql();
$results = $exsparql->queryNotation($exuri);
return isset($results) ? $results : null;
try {
Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above, but accessing notations from external vocabularies.

$exsparql = $exvoc->getSparql();
$results = $exsparql->queryNotation($exuri);
return isset($results) ? $results : null;
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->model->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
}
}
return null;
}
Expand Down
29 changes: 22 additions & 7 deletions model/Model.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,18 @@ private function disambiguateVocabulary($vocabs, $uri, $preferredVocabId = null)
if($preferredVocabId != null) {
foreach ($vocabs as $vocab) {
if($vocab->getId() == $preferredVocabId) {
// double check that a label exists in the preferred vocabulary
if ($vocab->getConceptLabel($uri, null) !== null) {
return $vocab;
} else {
// not found in preferred vocabulary, fall back to next method
try {
Copy link
Member Author

Choose a reason for hiding this comment

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

Idea here is to handle errors that happen when disambiguating between URI namespaces (e.g. is this URI from YSO or YSO Places?)

// double check that a label exists in the preferred vocabulary
if ($vocab->getConceptLabel($uri, null) !== null) {
return $vocab;
} else {
// not found in preferred vocabulary, fall back to next method
break;
}
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
break;
}
}
Expand All @@ -483,8 +490,16 @@ private function disambiguateVocabulary($vocabs, $uri, $preferredVocabId = null)

// no preferred vocabulary, or it was not found, search in which vocabulary the concept has a label
foreach ($vocabs as $vocab) {
if ($vocab->getConceptLabel($uri, null) !== null)
return $vocab;
try {
Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above, catch errors that happen when disambiguating URIs

if ($vocab->getConceptLabel($uri, null) !== null) {
return $vocab;
}
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
break;
}
}

// if the URI couldn't be found, fall back to the first vocabulary
Expand Down
15 changes: 15 additions & 0 deletions resource/css/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,21 @@ div#sidebar-grey > div > div > form.search-options {
top: 10px;
}

.search-result-listing .alert .btn-default {
position: unset;
vertical-align: unset;
margin-left: 15px;
font-size: 18px;
}

.alert h4, .alert h3 {
display: inline;
}

.search-result-listing .alert-warning h4 {
display: block;
}

.search-options .mCSB_container > li > a > .radio {
margin: 0;
padding-left: 25px;
Expand Down
25 changes: 21 additions & 4 deletions resource/js/docready.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ $(function() { // DOCUMENT READY
$('.search-result-listing').append($ready);
}
else {
$trigger.waypoint(function() { waypointCallback(); }, options);
$trigger.waypoint(function() { waypointCallback(this); }, options);
osma marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -894,9 +894,13 @@ $(function() { // DOCUMENT READY
changeOffset += 200;
}

function waypointCallback() {
function waypointCallback(waypoint) {
if ($('.search-result-listing > p .spinner,.search-result-listing .alert-danger').length > 0) {
return false;
}
var number_of_hits = $(".search-result").length;
if (number_of_hits < parseInt($('.search-count p').text().substr(0, $('.search-count p').text().indexOf(' ')), 10)) { $('.search-result-listing').append($loading);
if (number_of_hits < parseInt($('.search-count p').text().substr(0, $('.search-count p').text().indexOf(' ')), 10)) {
$('.search-result-listing').append($loading);
var typeLimit = $('#type-limit').val();
var schemeLimit = $('#scheme-limit').val();
var groupLimit = $('#group-limit').val();
Expand All @@ -917,7 +921,20 @@ $(function() { // DOCUMENT READY
$('.search-result-listing').append($ready);
return false;
}
$('.search-result:nth-last-of-type(4)').waypoint(function() { waypointCallback(); }, options );
waypoint.destroy();
$('.search-result:nth-last-of-type(4)').waypoint(function() { waypointCallback(this); }, options );
},
error: function(jqXHR, textStatus, errorThrown) {
$loading.detach();
var $failedSearch = $('<div class="alert alert-danger"><h4>Error: Loading for more items failed!</h4></div>');
osma marked this conversation as resolved.
Show resolved Hide resolved
var $retryButton = $('<button class="btn btn-default" type="button">Retry</button>');
osma marked this conversation as resolved.
Show resolved Hide resolved
$retryButton.on('click', function () {
$failedSearch.remove();
waypointCallback(waypoint);
return false;
});
$failedSearch.append($retryButton)
$('.search-result-listing').append($failedSearch);
}
});
}
Expand Down
3 changes: 2 additions & 1 deletion resource/js/scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ function loadLimitedResults(parameters) {
clearResultsAndAddSpinner();
$.ajax({
data: parameters,
success : function(data) {
complete : function(jqXHR, textStatus) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This callback will be executed in any case, both for successful and unsuccesful requests.

var data = jqXHR.responseText;
var response = $('.search-result-listing', data).html();
if (window.history.pushState) { window.history.pushState({url: this.url}, '', this.url); }
$('.search-result-listing').append(response);
Expand Down
10 changes: 9 additions & 1 deletion view/search-result.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,15 @@
<div class="search-count{% if limit_type or limit_parent or limit_group or limit_scheme %} search-limited{% endif %}"><p>{% trans %}{{ search_count }} results for '{{ term }}'{% endtrans %}{% if limit_type %}, {% trans "limited to type" %} '{% for type in limit_type %}{{ type }}{% if loop.last == false %}, {% endif %}{% endfor %}'{% endif %}{% if limit_parent %}, {% trans "limited to parent" %} '{{ limit_parent }}'{% endif %}{% if limit_group %}, {% trans "limited to group" %} '{{ limit_group }}'{% endif %}{% if limit_scheme %}, {% trans "limited to scheme" %} '{% for scheme in limit_scheme %}{{ scheme }}{% if loop.last == false %}, {% endif %}{% endfor %}'{% endif %}</p>
</div>{% if limit_type or limit_parent or limit_group or limit_scheme %}<button type="button" class="btn btn-default" id="remove-limits">{% trans "Clear limitations" %}</button>{% endif %}
{% endif %}
{% if search_results is defined and search_results|length == 0 %}<p class="no-results">{% trans 'The search provided no results.' %}</p>{% endif %}
{% if global_search and not search_failed and skipped_vocabs|length > 0 %}
<div class="alert alert-warning">
{% for voc in skipped_vocabs %}
{% set vocTitle = voc.title %}
<h4>{% trans %}Warning: Skipped searching from '{{vocTitle}}' vocabulary!{% endtrans %}</h4>
osma marked this conversation as resolved.
Show resolved Hide resolved
{% endfor %}
</div>
{% endif %}
{% if search_results is not null and search_results|length == 0 %}<p class="no-results">{% trans 'The search provided no results.' %}</p>{% endif %}
{% for concept in search_results %} {# loop through the hits #}
<div class="search-result">
{% spaceless %}
Expand Down
4 changes: 2 additions & 2 deletions view/vocab-search-listing.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
<div class="search-result-listing">
<h2 class="sr-only">{% trans "Search results" %}</h2>
{% include 'search-result.twig' %}
{% if search_results is not defined and not global_search %}
{% if search_results is null %}
<div class="alert alert-danger">
{% if request.vocabid == 'null' %}
{% if request.vocabid == 'null' and not global_search %}
<h4>{% trans %}Error: Requested vocabulary not found!{% endtrans %}</h4>
{% else %}
<h4>{% trans %}Error: Search failed!{% endtrans %}</h4>
Expand Down