diff --git a/search-service-bing-geocoder/src/main/java/nl/aerius/search/tasks/BingSearchService.java b/search-service-bing-geocoder/src/main/java/nl/aerius/search/tasks/BingSearchService.java index cbfc254..7b312d8 100644 --- a/search-service-bing-geocoder/src/main/java/nl/aerius/search/tasks/BingSearchService.java +++ b/search-service-bing-geocoder/src/main/java/nl/aerius/search/tasks/BingSearchService.java @@ -22,6 +22,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -76,6 +77,7 @@ public class BingSearchService implements SearchTaskService { * https://docs.microsoft.com/en-us/bingmaps/getting-started/bing-maps-api-best-practices#reducing-usage-transactions */ @Value("${nl.aerius.bing.apiKey:#{null}}") private String apiKey; + @Value("${nl.aerius.bing.maxRetries:3}") private int maxRetries; @Override public Single retrieveSearchResults(final String query) { @@ -119,9 +121,26 @@ private void retrieveSuggestions(final String query, final Map json = Unirest.get(url).asJson(); - final JSONObject body = json.getBody().getObject(); - return body.getJSONArray("resourceSets").getJSONObject(0).getJSONArray("resources"); + JSONObject body = null; + int retry = 0; + while (retry++ < maxRetries) { + final HttpResponse json = Unirest.get(url).asJson(); + body = json.getBody().getObject(); + final int statusCode = body.getInt("statusCode"); + if (statusCode == 200) { + return body.getJSONArray("resourceSets").getJSONObject(0).getJSONArray("resources"); + } else if (statusCode == 429) { + LOG.info("Got too many retries status code from Bing, attempt {}.", retry); + try { + TimeUnit.SECONDS.sleep(1); + } catch (final InterruptedException ie) { + Thread.currentThread().interrupt(); + } + } else { + throw new BingServiceException("Unexpected status code: " + statusCode); + } + } + throw new BingServiceException("Retries failed, last returned: " + body); } private SuggestedLocation createSuggestedLocation(final JSONObject jsonObject) { diff --git a/search-service-bing-geocoder/src/main/java/nl/aerius/search/tasks/BingServiceException.java b/search-service-bing-geocoder/src/main/java/nl/aerius/search/tasks/BingServiceException.java new file mode 100644 index 0000000..1edb3fa --- /dev/null +++ b/search-service-bing-geocoder/src/main/java/nl/aerius/search/tasks/BingServiceException.java @@ -0,0 +1,24 @@ +/* + * Copyright the State of the Netherlands + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package nl.aerius.search.tasks; + +public class BingServiceException extends RuntimeException { + + public BingServiceException(final String message) { + super(message); + } +} diff --git a/search-service-bing-geocoder/src/test/java/nl/aerius/search/tasks/BingSearchServiceTest.java b/search-service-bing-geocoder/src/test/java/nl/aerius/search/tasks/BingSearchServiceTest.java index f3af03d..da6f9b1 100644 --- a/search-service-bing-geocoder/src/test/java/nl/aerius/search/tasks/BingSearchServiceTest.java +++ b/search-service-bing-geocoder/src/test/java/nl/aerius/search/tasks/BingSearchServiceTest.java @@ -45,5 +45,11 @@ void testWorksAtAll() { final SearchTaskResult suggestions = result.blockingGet(); assertEquals(5, suggestions.getSuggestions().size(), "Expected number of results for 'edin' (should include 'edinburgh')"); + + final Single resultYork = delegator.retrieveSearchResults("york"); + + final SearchTaskResult suggestionsYork = resultYork.blockingGet(); + + assertEquals(4, suggestionsYork.getSuggestions().size(), "Expected number of results for 'edin' (should include 'edinburgh')"); } } diff --git a/search-service-mocks/src/main/java/nl/aerius/search/tasks/MockRuntimeExceptionTask.java b/search-service-mocks/src/main/java/nl/aerius/search/tasks/MockRuntimeExceptionTask.java new file mode 100644 index 0000000..6137af3 --- /dev/null +++ b/search-service-mocks/src/main/java/nl/aerius/search/tasks/MockRuntimeExceptionTask.java @@ -0,0 +1,38 @@ +/* + * Copyright the State of the Netherlands + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package nl.aerius.search.tasks; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; + +import io.reactivex.rxjava3.core.Single; + +import nl.aerius.search.domain.SearchCapability; +import nl.aerius.search.domain.SearchTaskResult; + +@Component +@ImplementsCapability(SearchCapability.MOCK_RUNTIME_EXCEPTION) +public class MockRuntimeExceptionTask implements SearchTaskService { + private static final Logger LOG = LoggerFactory.getLogger(MockRuntimeExceptionTask.class); + + @Override + public Single retrieveSearchResults(final String query) { + LOG.debug("Mocking runtim exception for query [{}]", query); + throw new NullPointerException("Some mocked nullpointer"); + } +} diff --git a/search-service/src/main/java/nl/aerius/search/tasks/async/AsyncSearchTaskDelegator.java b/search-service/src/main/java/nl/aerius/search/tasks/async/AsyncSearchTaskDelegator.java index 2bafe4d..f64c4da 100644 --- a/search-service/src/main/java/nl/aerius/search/tasks/async/AsyncSearchTaskDelegator.java +++ b/search-service/src/main/java/nl/aerius/search/tasks/async/AsyncSearchTaskDelegator.java @@ -85,12 +85,14 @@ public SearchResult retrieveSearchResultsAsync(final String query, final Set v.retrieveSearchResults(query)) - .doOnError(e -> LOG.error("Error while executing search task:", e)) .flatMap(Single::toFlowable) - .doAfterNext(r -> task.complete(r)) + .doAfterNext(task::complete) .sequential() - .doOnComplete(() -> task.complete()) - .subscribe(); + .doOnComplete(task::fullyComplete) + .subscribe(x -> {}, e -> { + LOG.error("General error while executing search task:", e); + task.failureComplete(); + }); disposables.put(task.getUuid(), disposable); diff --git a/search-service/src/main/java/nl/aerius/search/tasks/async/SearchResult.java b/search-service/src/main/java/nl/aerius/search/tasks/async/SearchResult.java index c65a227..52036f5 100644 --- a/search-service/src/main/java/nl/aerius/search/tasks/async/SearchResult.java +++ b/search-service/src/main/java/nl/aerius/search/tasks/async/SearchResult.java @@ -25,6 +25,7 @@ import org.slf4j.LoggerFactory; import nl.aerius.search.domain.SearchSuggestion; +import nl.aerius.search.domain.SearchSuggestionBuilder; import nl.aerius.search.domain.SearchTaskResult; public class SearchResult { @@ -45,7 +46,16 @@ public boolean isComplete() { return complete; } - public void complete() { + public void failureComplete() { + if (LOG.isTraceEnabled()) { + LOG.error("Search task {} has completed with a failure.", uuid); + } + results.add(SearchSuggestionBuilder.create("Failure during search, please contact the helpdesk")); + + complete = true; + } + + public void fullyComplete() { if (LOG.isTraceEnabled()) { LOG.trace("Search task {} has fully completed.", uuid); } diff --git a/search-service/src/main/java/nl/aerius/search/tasks/sync/BlockingSearchTaskDelegator.java b/search-service/src/main/java/nl/aerius/search/tasks/sync/BlockingSearchTaskDelegator.java index fb79057..fb4b7f4 100644 --- a/search-service/src/main/java/nl/aerius/search/tasks/sync/BlockingSearchTaskDelegator.java +++ b/search-service/src/main/java/nl/aerius/search/tasks/sync/BlockingSearchTaskDelegator.java @@ -30,6 +30,7 @@ import io.reactivex.rxjava3.schedulers.Schedulers; import nl.aerius.search.domain.SearchSuggestion; +import nl.aerius.search.domain.SearchSuggestionBuilder; import nl.aerius.search.tasks.CapabilityKey; import nl.aerius.search.tasks.SearchTaskService; import nl.aerius.search.tasks.TaskFactory; @@ -69,6 +70,10 @@ public List retrieveSearchResults(final String query, final Se .flatMap(v -> Flowable.fromIterable(v.getSuggestions())) .sorted(TaskUtils.getResultComparator()) .toList() + .onErrorReturn(e -> { + LOG.error("General error while executing search task:", e); + return List.of(SearchSuggestionBuilder.create("Failure during search, please contact the helpdesk")); + }) .blockingGet(); } } diff --git a/search-service/src/main/resources/templates/synchronous-form.html b/search-service/src/main/resources/templates/synchronous-form.html index da3f1ab..9abb883 100644 --- a/search-service/src/main/resources/templates/synchronous-form.html +++ b/search-service/src/main/resources/templates/synchronous-form.html @@ -47,6 +47,7 @@
Mocks
+
diff --git a/search-service/src/test/java/nl/aerius/search/tasks/async/AsyncSearchTaskDelegatorTest.java b/search-service/src/test/java/nl/aerius/search/tasks/async/AsyncSearchTaskDelegatorTest.java index 7b371f9..5362c51 100644 --- a/search-service/src/test/java/nl/aerius/search/tasks/async/AsyncSearchTaskDelegatorTest.java +++ b/search-service/src/test/java/nl/aerius/search/tasks/async/AsyncSearchTaskDelegatorTest.java @@ -33,6 +33,7 @@ import nl.aerius.search.tasks.CapabilityKey; import nl.aerius.search.tasks.Mock0SecondTask; import nl.aerius.search.tasks.MockHalfSecondTask; +import nl.aerius.search.tasks.MockRuntimeExceptionTask; import nl.aerius.search.tasks.MockTenthSecondTask; import nl.aerius.search.tasks.SearchTaskService; import nl.aerius.search.tasks.TaskFactory; @@ -69,14 +70,17 @@ void testResponseDelays() throws InterruptedException { TimeUnit.MILLISECONDS.sleep(50); final SearchResult result3 = delegator.retrieveSearchTask(uuid); assertEquals(1, result3.getResults().size(), "Should have 1 results at this point."); + assertFalse(result3.isComplete(), "Result should not be complete at this point."); TimeUnit.MILLISECONDS.sleep(100); final SearchResult result4 = delegator.retrieveSearchTask(uuid); assertEquals(2, result4.getResults().size(), "Should have 2 results at this point."); + assertFalse(result4.isComplete(), "Result should not be complete at this point."); TimeUnit.MILLISECONDS.sleep(400); final SearchResult result5 = delegator.retrieveSearchTask(uuid); assertEquals(3, result5.getResults().size(), "Should have 3 results at this point."); + assertTrue(result5.isComplete(), "Result should now be complete at this point."); } @Test @@ -98,4 +102,35 @@ void testResponseCancellation() throws InterruptedException { assertTrue(result1.getResults().isEmpty(), "Results should still be empty after the task completes (>100ms) - the individual task was not cancelled"); } + + @Test + void testResponseRuntimeException() throws InterruptedException { + final SearchTaskService mock0Task = new Mock0SecondTask(); + final SearchTaskService mock01Task = new MockTenthSecondTask(); + final SearchTaskService mockRuntimeException = new MockRuntimeExceptionTask(); + + final Set tasks = Set.of(mock0Task, mock01Task, mockRuntimeException); + + final TaskFactory factory = new TaskFactory(tasks); + factory.onFactoryConstructed(); + delegator = new AsyncSearchTaskDelegator(factory); + + final Set caps = Set.of(CapabilityKey.of(SearchCapability.MOCK_0, SearchRegion.NL), + CapabilityKey.of(SearchCapability.MOCK_01, SearchRegion.NL), + CapabilityKey.of(SearchCapability.MOCK_RUNTIME_EXCEPTION, SearchRegion.NL)); + final SearchResult result1 = delegator.retrieveSearchResultsAsync("test", caps); + final String uuid = result1.getUuid(); + + final SearchResult result2 = delegator.retrieveSearchTask(uuid); + + assertFalse(result2.isComplete(), "Result should not be complete at this point."); + assertNotNull(result2.getResults(), "Results should not be null at this point."); + + TimeUnit.MILLISECONDS.sleep(50); + final SearchResult result3 = delegator.retrieveSearchTask(uuid); + assertEquals(1, result3.getResults().size(), "Should have 1 results at this point."); + assertTrue(result3.isComplete(), "Result should now be complete at this point, runtime exception should have triggered."); + assertEquals("Failure during search, please contact the helpdesk", result3.getResults().get(0).getDescription(), + "Description in case of failure"); + } } diff --git a/search-shared/src/main/java/nl/aerius/search/domain/SearchCapability.java b/search-shared/src/main/java/nl/aerius/search/domain/SearchCapability.java index 0069702..515450d 100644 --- a/search-shared/src/main/java/nl/aerius/search/domain/SearchCapability.java +++ b/search-shared/src/main/java/nl/aerius/search/domain/SearchCapability.java @@ -36,7 +36,9 @@ public enum SearchCapability { MOCK_5, MOCK_GROUP_0, - MOCK_GROUP_1; + MOCK_GROUP_1, + + MOCK_RUNTIME_EXCEPTION; public static SearchCapability safeValueOf(final String name) { try {