From e110eed44563ffdf0942bb765d2b0e0a979ab4c4 Mon Sep 17 00:00:00 2001 From: DavidCroftDKFZ <46788708+DavidCroftDKFZ@users.noreply.github.com> Date: Thu, 30 Jan 2025 11:22:18 +0100 Subject: [PATCH] Made all REST calls test with and without country code Depending on whether we are syncing with a national node or with the central Directory, we may or may not need to incoporate the country code into the URL for REST API calls. The changes test both possibilities. --- pom.xml | 2 +- .../directory/DirectoryApi.java | 5 +- .../graphql/DirectoryApiGraphql.java | 4 +- .../directory/rest/DirectoryApiRest.java | 82 +++++++++++++------ .../directory/rest/DirectoryCallsRest.java | 11 --- 5 files changed, 61 insertions(+), 43 deletions(-) diff --git a/pom.xml b/pom.xml index 9453c22..0f063da 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ de.samply directory_sync_service - 1.5.10 + 1.5.11 directory_sync_service Directory sync https://github.com/samply/directory_sync_service diff --git a/src/main/java/de/samply/directory_sync_service/directory/DirectoryApi.java b/src/main/java/de/samply/directory_sync_service/directory/DirectoryApi.java index b66c663..f3620c3 100644 --- a/src/main/java/de/samply/directory_sync_service/directory/DirectoryApi.java +++ b/src/main/java/de/samply/directory_sync_service/directory/DirectoryApi.java @@ -155,7 +155,7 @@ protected boolean deleteStarModel(StarModelData starModelInputData) { // We need to do things this way, because the Directory implements paging // and a single pass may not get all facts. do { - List factIds = getNextPageOfFactIdsForCollection(countryCode, collectionId); + List factIds = getNextPageOfFactIdsForCollection(collectionId); if (factIds == null) { logger.warn("deleteStarModel: Problem getting facts for collection: " + collectionId); @@ -186,11 +186,10 @@ protected boolean deleteStarModel(StarModelData starModelInputData) { /** * Retrieves a list of fact IDs from the Directory associated with a specific collection. * - * @param countryCode The country code, e.g. DE. * @param collectionId The ID of the collection to retrieve fact IDs for. * @return A list of fact IDs for the specified collection, or null if there is an issue retrieving the data. An empty list indicates that there are no more facts left to be retrieved. */ - protected abstract List getNextPageOfFactIdsForCollection(String countryCode, String collectionId); + protected abstract List getNextPageOfFactIdsForCollection(String collectionId); /** * Deletes facts from the Directory service based on a list of fact IDs. diff --git a/src/main/java/de/samply/directory_sync_service/directory/graphql/DirectoryApiGraphql.java b/src/main/java/de/samply/directory_sync_service/directory/graphql/DirectoryApiGraphql.java index d629ec3..aad38cb 100644 --- a/src/main/java/de/samply/directory_sync_service/directory/graphql/DirectoryApiGraphql.java +++ b/src/main/java/de/samply/directory_sync_service/directory/graphql/DirectoryApiGraphql.java @@ -640,12 +640,12 @@ private String wrapValueInHashWithAttribute(String attributeName, String value) /** * Retrieves a list of fact IDs from the Directory associated with a specific collection. * - * @param countryCode The country code, e.g. DE. * @param collectionId The ID of the collection to retrieve fact IDs for. * @return A list of fact IDs for the specified collection, or null if there is an issue retrieving the data. An empty list indicates that there are no more facts left to be retrieved. */ @Override - protected List getNextPageOfFactIdsForCollection(String countryCode, String collectionId) { + protected List getNextPageOfFactIdsForCollection(String collectionId) { + String countryCode = extractCountryCodeFromBbmriEricId(collectionId); List factIds = new ArrayList<>(); // Use getFactPageToggle to ensure that this method gets run only once. diff --git a/src/main/java/de/samply/directory_sync_service/directory/rest/DirectoryApiRest.java b/src/main/java/de/samply/directory_sync_service/directory/rest/DirectoryApiRest.java index 6f5030d..45ec60a 100644 --- a/src/main/java/de/samply/directory_sync_service/directory/rest/DirectoryApiRest.java +++ b/src/main/java/de/samply/directory_sync_service/directory/rest/DirectoryApiRest.java @@ -70,14 +70,20 @@ public Biobank fetchBiobank(BbmriEricId id) { // Return a fake Biobank if we are mocking return new Biobank(); - Biobank biobank = (Biobank) directoryCallsRest.get(directoryEndpointsRest.getBiobankEndpoint(id.getCountryCode()) + "/" + id, Biobank.class); + Biobank biobank = fetchBiobank(id.getCountryCode(), id); if (biobank == null) { - logger.warn("fetchBiobank: No Biobank in Directory with id: " + id); - return null; + logger.info("fetchBiobank: biobank is null, trying URL without country code"); + biobank = fetchBiobank(null, id); + if (biobank == null) + logger.warn("fetchBiobank: No Biobank in Directory with id: " + id); } return biobank; } + private Biobank fetchBiobank(String countryCode, BbmriEricId id) { + return (Biobank) directoryCallsRest.get(directoryEndpointsRest.getBiobankEndpoint(countryCode) + "/" + id, Biobank.class); + } + /** * Make API calls to the Directory to fill a DirectoryCollectionGet object containing attributes * for all of the collections listed in collectionIds. The countryCode is used solely for @@ -97,7 +103,6 @@ public DirectoryCollectionGet fetchCollectionGetOutcomes(String countryCode, Lis return directoryCollectionGet; } - logger.debug("fetchCollectionGetOutcomes: ?????????????????????????????????????????????????????????????????????????????????????????????????????????????"); boolean warnFlag = false; for (String collectionId: collectionIds) { logger.debug("fetchCollectionGetOutcomes: collectionId: " + collectionId); @@ -105,9 +110,8 @@ public DirectoryCollectionGet fetchCollectionGetOutcomes(String countryCode, Lis logger.debug("fetchCollectionGetOutcomes: commandUrl: " + commandUrl); DirectoryCollectionGet singleDirectoryCollectionGet = (DirectoryCollectionGet) directoryCallsRest.get(commandUrl, DirectoryCollectionGet.class); if (singleDirectoryCollectionGet == null) { - logger.warn("fetchCollectionGetOutcomes: singleDirectoryCollectionGet is null, trying URL without country code"); + logger.info("fetchCollectionGetOutcomes: singleDirectoryCollectionGet is null, trying URL without country code"); commandUrl = directoryEndpointsRest.getCollectionEndpoint(null) + "?q=id==%22" + collectionId + "%22"; - logger.debug("fetchCollectionGetOutcomes: new commandUrl: " + commandUrl); singleDirectoryCollectionGet = (DirectoryCollectionGet) directoryCallsRest.get(commandUrl, DirectoryCollectionGet.class); if (singleDirectoryCollectionGet == null) { logger.warn("fetchCollectionGetOutcomes: singleDirectoryCollectionGet is null, does the collection exist in the Directory: " + collectionId); @@ -123,7 +127,6 @@ public DirectoryCollectionGet fetchCollectionGetOutcomes(String countryCode, Lis } directoryCollectionGet.getItems().add(item); } - logger.debug("fetchCollectionGetOutcomes: ?????????????????????????????????????????????????????????????????????????????????????????????????????????????"); if (warnFlag && directoryCollectionGet.isEmpty()) { logger.warn("fetchCollectionGetOutcomes: No entities retrieved from Directory"); @@ -145,15 +148,24 @@ public boolean updateEntities(DirectoryCollectionPut directoryCollectionPut) { return true; } - String response = directoryCallsRest.put(directoryEndpointsRest.getCollectionEndpoint(directoryCollectionPut.getCountryCode()), directoryCollectionPut); + String response = updateEntities(directoryCollectionPut.getCountryCode(), directoryCollectionPut); if (response == null) { - logger.warn("entity update, PUT problem"); - return false; + logger.info("updateEntities: PUT problem, trying URL without country code"); + response = updateEntities(null, directoryCollectionPut); + if (response == null) { + logger.warn("updateEntities: PUT problem even without country code, aborting"); + return false; + } } return true; } + private String updateEntities(String countryCode, DirectoryCollectionPut directoryCollectionPut) { + return directoryCallsRest.put(directoryEndpointsRest.getCollectionEndpoint(countryCode), directoryCollectionPut); + } + + /** * Updates the fact tables block for a specific country with the provided data. * @@ -168,13 +180,22 @@ protected boolean updateFactTablesBlock(String countryCode, List body = new HashMap(); body.put("entities", factTablesBlock); - String response = directoryCallsRest.post(directoryEndpointsRest.getFactEndpoint(countryCode), body); + String response = directoryCallsRest.post(directoryEndpointsRest.getFactEndpoint(countryCode), body); if (response == null) { - logger.warn("updateFactTablesBlock: null response from REST call"); - return false; + logger.info("updateFactTablesBlock: null response from REST call, trying URL without country code"); + response = directoryCallsRest.post(directoryEndpointsRest.getFactEndpoint(null), body); + if (response == null) { + logger.warn("updateFactTablesBlock: null response from REST call even without country code, aborting"); + return false; + } } return true; @@ -183,23 +204,26 @@ protected boolean updateFactTablesBlock(String countryCode, List getNextPageOfFactIdsForCollection(String countryCode, String collectionId) { - String apiUrl = directoryEndpointsRest.getFactEndpoint(countryCode); - + protected List getNextPageOfFactIdsForCollection(String collectionId) { // Get a list of fact IDs for this collection + String apiUrl = directoryEndpointsRest.getFactEndpoint(extractCountryCodeFromBbmriEricId(collectionId)); Map factWrapper = (Map) directoryCallsRest.get(apiUrl + "?q=collection==%22" + collectionId + "%22", Map.class); - if (factWrapper == null) { - logger.warn("deleteStarModel: Problem getting facts for collection, factWrapper == null, collectionId=" + collectionId); - return null; + logger.info("getNextPageOfFactIdsForCollection: Problem getting facts for collection, factWrapper == null, collectionId=" + collectionId + ", trying without country code"); + apiUrl = directoryEndpointsRest.getFactEndpoint(extractCountryCodeFromBbmriEricId(null)); + factWrapper = (Map) directoryCallsRest.get(apiUrl + "?q=collection==%22" + collectionId + "%22", Map.class); + if (factWrapper == null) { + logger.warn("getNextPageOfFactIdsForCollection: Problem getting facts for collection, factWrapper == null, collectionId=" + collectionId + " even without contry code, aborting"); + return null; + } } + if (!factWrapper.containsKey("items")) { - logger.warn("deleteStarModel: Problem getting facts for collection, no item key present: " + collectionId); + logger.warn("getNextPageOfFactIdsForCollection: Problem getting facts for collection, no item key present: " + collectionId); return null; } List> facts = (List>) factWrapper.get("items"); @@ -227,14 +251,20 @@ protected boolean deleteFactsByIds(String countryCode, List factIds) { // Nothing to delete return true; - String apiUrl = directoryEndpointsRest.getFactEndpoint(countryCode); + if (mockDirectory) { + // Dummy return if we're in mock mode + return true; + } // Directory likes to have its delete data wrapped in a map with key "entityIds". - String result = directoryCallsRest.delete(apiUrl, new HashMap<>(Map.of("entityIds", factIds))); - + String result = directoryCallsRest.delete(directoryEndpointsRest.getFactEndpoint(countryCode), new HashMap<>(Map.of("entityIds", factIds))); if (result == null) { - logger.warn("deleteFactsByIds, Problem during delete of factIds"); - return false; + logger.info("deleteFactsByIds, Problem during delete of factIds, trying without country code"); + result = directoryCallsRest.delete(directoryEndpointsRest.getFactEndpoint(null), new HashMap<>(Map.of("entityIds", factIds))); + if (result == null) { + logger.warn("deleteFactsByIds, Problem during delete of factIds even without contry code, aborting"); + return false; + } } return true; diff --git a/src/main/java/de/samply/directory_sync_service/directory/rest/DirectoryCallsRest.java b/src/main/java/de/samply/directory_sync_service/directory/rest/DirectoryCallsRest.java index d8d70dd..67b1eb6 100644 --- a/src/main/java/de/samply/directory_sync_service/directory/rest/DirectoryCallsRest.java +++ b/src/main/java/de/samply/directory_sync_service/directory/rest/DirectoryCallsRest.java @@ -1,24 +1,13 @@ package de.samply.directory_sync_service.directory.rest; -import com.google.gson.Gson; import de.samply.directory_sync_service.Util; import de.samply.directory_sync_service.directory.DirectoryCalls; import de.samply.directory_sync_service.directory.DirectoryCredentials; -import org.apache.http.HttpEntity; -import org.apache.http.HttpResponse; -import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpOptions; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; -import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.entity.StringEntity; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.util.EntityUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.BufferedReader; import java.io.IOException;