fix: handle correctly missing beacon access token and empty beacon re…

…sponse We will skip beacon query in the following Keycloak 4xx status: - 400 = invalid/missing keycloak access token - 401 = unauthorized access - 403 = Forbidden access Even when access to beacon is valid, the beacon can retrieve empty response. In that situation, we must retrieve directly empty results and skip CKAN query.
GenomicDataInfrastructure · Apr 10, 2024 · 7d41534 · 7d41534
1 parent 7b5a26a
commit 7d41534
Show file tree

Hide file tree

Showing 7 changed files with 134 additions and 11 deletions.
diff --git a/...a/io/github/genomicdatainfrastructure/discovery/services/BeaconDatasetsSearchService.java b/...a/io/github/genomicdatainfrastructure/discovery/services/BeaconDatasetsSearchService.java
@@ -27,14 +27,22 @@
 import io.github.genomicdatainfrastructure.discovery.remote.keycloak.api.KeycloakQueryApi;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
+import jakarta.ws.rs.WebApplicationException;
+import lombok.extern.java.Log;
+
 import java.util.ArrayList;
 import java.util.Objects;
+import java.util.logging.Level;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Set;
 
+@Log
 @ApplicationScoped
 public class BeaconDatasetsSearchService implements DatasetsSearchService {
 
+    private static final Set<Integer> SKIP_BEACON_QUERY_STATUS = Set.of(400, 401, 403);
+    private static final String BEACON_ACCESS_TOKEN_INFO = "Skipping beacon search, user is not authorized or the token is invalid.";
     private static final String BEACON_IDP_ALIAS = "LSAAI";
     private static final String BEARER_PATTERN = "Bearer %s";
     private static final String BEACON_DATASET_TYPE = "dataset";
@@ -62,25 +70,45 @@ public BeaconDatasetsSearchService(
     public DatasetsSearchResponse search(DatasetSearchQuery query, String accessToken) {
         var beaconQuery = BeaconIndividualsRequestMapper.from(query);
 
-        if (accessToken == null || beaconQuery.getQuery().getFilters().isEmpty()) {
+        var beaconAuthorization = retrieveBeaconAuthorization(accessToken);
+
+        if (beaconAuthorization == null || beaconQuery.getQuery().getFilters().isEmpty()) {
             return datasetsSearchService.search(query, accessToken);
         }
 
-        var beaconAuthorization = retrieveBeaconAuthorization(accessToken);
-
         var beaconResponse = queryOnBeacon(beaconAuthorization, beaconQuery);
 
-        var enhancedQuery = enhanceQueryFacets(query, beaconResponse);
-
-        var datasetsReponse = datasetsSearchService.search(enhancedQuery, accessToken);
+        var datasetsReponse = DatasetsSearchResponse.builder()
+                .count(0)
+                .build();
+        if (!beaconResponse.isEmpty()) {
+            var enhancedQuery = enhanceQueryFacets(query, beaconResponse);
+            datasetsReponse = datasetsSearchService.search(enhancedQuery, accessToken);
+        }
 
         return enhanceDatasetsResponse(beaconAuthorization, datasetsReponse, beaconResponse);
     }
 
     private String retrieveBeaconAuthorization(String accessToken) {
+        if (accessToken == null) {
+            return null;
+        }
+
         var keycloakAuthorization = BEARER_PATTERN.formatted(accessToken);
-        var response = keycloakQueryApi.retriveIdpTokens(BEACON_IDP_ALIAS, keycloakAuthorization);
-        return BEARER_PATTERN.formatted(response.getAccessToken());
+        try {
+            var response = keycloakQueryApi.retriveIdpTokens(
+                    BEACON_IDP_ALIAS,
+                    keycloakAuthorization
+            );
+            return BEARER_PATTERN.formatted(response.getAccessToken());
+        } catch (WebApplicationException exception) {
+            if (SKIP_BEACON_QUERY_STATUS.contains(exception.getResponse().getStatus())) {
+                log.log(Level.INFO, BEACON_ACCESS_TOKEN_INFO);
+                log.log(Level.WARNING, exception, exception::getMessage);
+                return null;
+            }
+            throw exception;
+        }
     }
 
     private List<BeaconResultSet> queryOnBeacon(

diff --git a/src/main/openapi/keycloak.yaml b/src/main/openapi/keycloak.yaml
@@ -23,7 +23,7 @@ paths:
           required: true
           schema:
             type: string
-        - name: Autorization
+        - name: Authorization
           in: header
           description: The authorization header
           required: true
@@ -36,8 +36,24 @@ paths:
             application/json:
               schema:
                 $ref: "#/components/schemas/KeycloakTokenResponse"
+        "400":
+          description: "Bad Request"
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/KeyCloakErrorResponse"
         "401":
           description: "Unauthorized"
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/KeyCloakErrorResponse"
+        "403":
+          description: "Forbidden"
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/KeyCloakErrorResponse"
       security:
         - keycloak_auth:
             - read:token
@@ -80,3 +96,8 @@ components:
         accessTokenExpiration:
           type: integer
           title: The expiration time of the access token
+    KeyCloakErrorResponse:
+      type: object
+      properties:
+        errorMessage:
+          type: string
diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties
@@ -30,9 +30,11 @@ quarkus.openapi-generator.codegen.spec.keycloak_yaml.generate-part-filename=fals
 quarkus.rest-client.ckan_yaml.url=http://localhost:4000
 quarkus.rest-client.keycloak_yaml.url=http://localhost:4000
 quarkus.rest-client.beacon_yaml.url=http://localhost:4000
-quarkus.rest-client.read-timeout=1000
 %dev.quarkus.oidc.auth-server-url=https://keycloak-test.healthdata.nl/realms/ckan
 %dev.quarkus.oidc.client-id=ckan
 %dev.quarkus.rest-client.ckan_yaml.url=https://ckan-test.healthdata.nl/
+%dev.quarkus.rest-client.keycloak_yaml.url=https://keycloak-test.healthdata.nl/realms/ckan
+%dev.quarkus.rest-client.beacon_yaml.url=https://beacon-network-backend-demo.ega-archive.org/beacon-network
 %dev.quarkus.rest-client.logging.body-limit=10000
-%dev.quarkus.log.category."org.jboss.resteasy.reactive.client.logging".level=DEBUG
+%dev.quarkus.rest-client.logging.scope=request-response
+%dev.quarkus.log.category."org.jboss.resteasy.reactive.client.logging".level=DEBUG
diff --git a/src/test/java/io/github/genomicdatainfrastructure/discovery/api/DatasetSearchTest.java b/src/test/java/io/github/genomicdatainfrastructure/discovery/api/DatasetSearchTest.java
@@ -73,4 +73,27 @@ void can_search_datasets_with_beacon_filters() {
                 .body("count", equalTo(1))
                 .body("results[0].recordsCount", equalTo(64));
     }
+
+    @Test
+    void skip_search_datasets_when_beacon_returns_empty_resultsets() {
+        var query = DatasetSearchQuery.builder()
+                .facets(List.of(
+                        DatasetSearchQueryFacet.builder()
+                                .facetGroup("beacon")
+                                .facet("dummy")
+                                .value("DUMMY:FILTER")
+                                .build()
+                ))
+                .build();
+        given()
+                .auth()
+                .oauth2(getAccessToken("alice"))
+                .contentType("application/json")
+                .body(query)
+                .when()
+                .post("/api/v1/datasets/search")
+                .then()
+                .statusCode(200)
+                .body("count", equalTo(0));
+    }
 }
diff --git a/src/test/resources/mappings/individuals.json b/src/test/resources/mappings/individuals.json
@@ -1,4 +1,5 @@
 {
+    "priority": 2,
     "request": {
         "method": "POST",
         "url": "/v2.0.0/individuals"

diff --git a/src/test/resources/mappings/individuals_no_results.json b/src/test/resources/mappings/individuals_no_results.json
@@ -0,0 +1,45 @@
+{
+    "priority": 1,
+    "request": {
+        "method": "POST",
+        "url": "/v2.0.0/individuals",
+        "bodyPatterns": [
+            {
+                "matchesJsonPath": "$.query.filters[?(@.id == 'DUMMY:FILTER')]"
+            }
+        ]
+    },
+    "response": {
+        "status": 200,
+        "headers": {
+            "Content-Type": "application/json"
+        },
+        "jsonBody": {
+            "meta": {
+                "apiVersion": "v2.0.0",
+                "beaconId": "es.elixir.bsc.beacon-network",
+                "receivedRequestSummary": {
+                    "apiVersion": "2.0",
+                    "filters": [
+                        "DUMMY:FILTER"
+                    ],
+                    "pagination": {
+                        "limit": 0,
+                        "skip": 0
+                    },
+                    "requestedGranularity": "count",
+                    "requestedSchemas": [],
+                    "testMode": false
+                },
+                "returnedGranularity": "count"
+            },
+            "responseSummary": {
+                "exists": false,
+                "numTotalResults": 0
+            },
+            "response": {
+                "resultSets": []
+            }
+        }
+    }
+}
diff --git a/src/test/resources/mappings/individuals_no_results.json.license b/src/test/resources/mappings/individuals_no_results.json.license
@@ -0,0 +1,3 @@
+SPDX-FileCopyrightText: 2024 PNED G.I.E.
+
+SPDX-License-Identifier: Apache-2.0
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		SPDX-FileCopyrightText: 2024 PNED G.I.E.

		SPDX-License-Identifier: Apache-2.0