diff --git a/src/main/java/org/broadinstitute/consent/http/ConsentModule.java b/src/main/java/org/broadinstitute/consent/http/ConsentModule.java index 800f2b060..7ef49921c 100644 --- a/src/main/java/org/broadinstitute/consent/http/ConsentModule.java +++ b/src/main/java/org/broadinstitute/consent/http/ConsentModule.java @@ -238,7 +238,7 @@ HealthCheckRegistry providesHealthCheckRegistry() { @Provides UseRestrictionConverter providesUseRestrictionConverter() { - return new UseRestrictionConverter(providesClient(), config.getServicesConfiguration()); + return new UseRestrictionConverter(); } @Provides @@ -435,9 +435,9 @@ VoteService providesVoteService() { providesElectionDAO(), providesEmailService(), providesElasticSearchService(), - providesUseRestrictionConverter(), providesVoteDAO(), - providesVoteServiceDAO()); + providesVoteServiceDAO(), + providesOntologyService()); } @Provides diff --git a/src/main/java/org/broadinstitute/consent/http/service/DataAccessReportsParser.java b/src/main/java/org/broadinstitute/consent/http/service/DataAccessReportsParser.java index 03aff67c4..bd1f0f93c 100644 --- a/src/main/java/org/broadinstitute/consent/http/service/DataAccessReportsParser.java +++ b/src/main/java/org/broadinstitute/consent/http/service/DataAccessReportsParser.java @@ -20,15 +20,19 @@ public class DataAccessReportsParser implements ConsentLogger { private final DatasetDAO datasetDAO; private final UseRestrictionConverter useRestrictionConverter; + private final OntologyService ontologyService; private static final String DEFAULT_SEPARATOR = "\t"; private static final String END_OF_LINE = System.lineSeparator(); public DataAccessReportsParser( - DatasetDAO datasetDAO, UseRestrictionConverter useRestrictionConverter) { + DatasetDAO datasetDAO, + UseRestrictionConverter useRestrictionConverter, + OntologyService ontologyService) { this.datasetDAO = datasetDAO; this.useRestrictionConverter = useRestrictionConverter; + this.ontologyService = ontologyService; } public void setApprovedDARHeader(FileWriter darWriter) throws IOException { @@ -163,8 +167,7 @@ private void addDARLine( ? translatedUseRestriction.replace("\n", " ") : ""; DataUse dataUse = useRestrictionConverter.parseDataUsePurpose(dar); - String sDAR = - useRestrictionConverter.translateDataUse(dataUse, DataUseTranslationType.PURPOSE); + String sDAR = ontologyService.translateDataUse(dataUse, DataUseTranslationType.PURPOSE); String formattedSDAR = sDAR.replace("\n", " "); darWriter.write( darCode diff --git a/src/main/java/org/broadinstitute/consent/http/service/UseRestrictionConverter.java b/src/main/java/org/broadinstitute/consent/http/service/UseRestrictionConverter.java index 612d23250..bc68e963e 100644 --- a/src/main/java/org/broadinstitute/consent/http/service/UseRestrictionConverter.java +++ b/src/main/java/org/broadinstitute/consent/http/service/UseRestrictionConverter.java @@ -1,16 +1,8 @@ package org.broadinstitute.consent.http.service; -import jakarta.ws.rs.InternalServerErrorException; -import jakarta.ws.rs.client.Client; -import jakarta.ws.rs.client.Entity; -import jakarta.ws.rs.client.WebTarget; -import jakarta.ws.rs.core.MediaType; -import jakarta.ws.rs.core.Response; import java.util.List; import java.util.Objects; import org.apache.commons.collections4.CollectionUtils; -import org.broadinstitute.consent.http.configurations.ServicesConfiguration; -import org.broadinstitute.consent.http.enumeration.DataUseTranslationType; import org.broadinstitute.consent.http.models.DataAccessRequest; import org.broadinstitute.consent.http.models.DataUse; import org.broadinstitute.consent.http.models.OntologyEntry; @@ -18,13 +10,7 @@ public class UseRestrictionConverter implements ConsentLogger { - private final ServicesConfiguration servicesConfiguration; - private final Client client; - - public UseRestrictionConverter(Client client, ServicesConfiguration config) { - this.client = client; - this.servicesConfiguration = config; - } + public UseRestrictionConverter() {} /** * This method, and its counterpart that processes a map, translates DAR questions to a DataUse @@ -130,22 +116,4 @@ public DataUse parseDataUsePurpose(DataAccessRequest dar) { } return dataUse; } - - public String translateDataUse(DataUse dataUse, DataUseTranslationType type) { - WebTarget target = - client.target(servicesConfiguration.getOntologyURL() + "translate?for=" + type.getValue()); - Response response = - target.request(MediaType.APPLICATION_JSON).post(Entity.json(dataUse.toString())); - if (response.getStatus() == 200) { - try { - return response.readEntity(String.class); - } catch (Exception e) { - logException("Error parsing response from Ontology service", e); - } - } - logException( - "Error response from Ontology service: " + response.readEntity(String.class), - new InternalServerErrorException()); - return null; - } } diff --git a/src/main/java/org/broadinstitute/consent/http/service/VoteService.java b/src/main/java/org/broadinstitute/consent/http/service/VoteService.java index 9020c6357..607347c48 100644 --- a/src/main/java/org/broadinstitute/consent/http/service/VoteService.java +++ b/src/main/java/org/broadinstitute/consent/http/service/VoteService.java @@ -57,9 +57,9 @@ public class VoteService implements ConsentLogger { private final ElectionDAO electionDAO; private final EmailService emailService; private final ElasticSearchService elasticSearchService; - private final UseRestrictionConverter useRestrictionConverter; private final VoteDAO voteDAO; private final VoteServiceDAO voteServiceDAO; + private final OntologyService ontologyService; @Inject public VoteService( @@ -70,9 +70,9 @@ public VoteService( ElectionDAO electionDAO, EmailService emailService, ElasticSearchService elasticSearchService, - UseRestrictionConverter useRestrictionConverter, VoteDAO voteDAO, - VoteServiceDAO voteServiceDAO) { + VoteServiceDAO voteServiceDAO, + OntologyService ontologyService) { this.userDAO = userDAO; this.dacDAO = dacDAO; this.dataAccessRequestDAO = dataAccessRequestDAO; @@ -80,9 +80,9 @@ public VoteService( this.electionDAO = electionDAO; this.emailService = emailService; this.elasticSearchService = elasticSearchService; - this.useRestrictionConverter = useRestrictionConverter; this.voteDAO = voteDAO; this.voteServiceDAO = voteServiceDAO; + this.ontologyService = ontologyService; } /** @@ -289,7 +289,7 @@ public void sendDatasetApprovalNotifications(List votes, User user) { approvedDatasetsInDar.stream() .map( dataset -> - useRestrictionConverter.translateDataUse( + ontologyService.translateDataUse( dataset.getDataUse(), DataUseTranslationType.DATASET)) .distinct() .collect(Collectors.joining(";")); diff --git a/src/test/java/org/broadinstitute/consent/http/service/DataAccessReportsParserTest.java b/src/test/java/org/broadinstitute/consent/http/service/DataAccessReportsParserTest.java index 3fcdb2b8f..965c89495 100644 --- a/src/test/java/org/broadinstitute/consent/http/service/DataAccessReportsParserTest.java +++ b/src/test/java/org/broadinstitute/consent/http/service/DataAccessReportsParserTest.java @@ -29,6 +29,7 @@ class DataAccessReportsParserTest { @Mock private DatasetDAO datasetDAO; @Mock private UseRestrictionConverter useRestrictionConverter; + @Mock private OntologyService ontologyService; private DataAccessReportsParser parser; private final String CONSENT_NAME = "ORSP-1903"; private final String NAME = "Test"; @@ -46,7 +47,7 @@ Future use for methods research (analytic/software/technology development) is no private final String DAR_CODE = "DAR_3"; @BeforeEach - public void setUp() { + void setUp() { Dataset d = new Dataset(); d.setDatasetId(1); // This translates to an identifier of "DUOS-000001" d.setAlias(1); @@ -58,8 +59,8 @@ public void setUp() { Research is limited to samples restricted for use under the following conditions: Data is limited for health/medical/biomedical research. [HMB] """; - when(useRestrictionConverter.translateDataUse(any(), any())).thenReturn(translation); - this.parser = new DataAccessReportsParser(datasetDAO, useRestrictionConverter); + when(ontologyService.translateDataUse(any(), any())).thenReturn(translation); + this.parser = new DataAccessReportsParser(datasetDAO, useRestrictionConverter, ontologyService); } @Test diff --git a/src/test/java/org/broadinstitute/consent/http/service/UseRestrictionConverterTest.java b/src/test/java/org/broadinstitute/consent/http/service/UseRestrictionConverterTest.java index 960141d9a..73cb314a8 100644 --- a/src/test/java/org/broadinstitute/consent/http/service/UseRestrictionConverterTest.java +++ b/src/test/java/org/broadinstitute/consent/http/service/UseRestrictionConverterTest.java @@ -4,107 +4,24 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockserver.model.HttpRequest.request; -import static org.mockserver.model.HttpResponse.response; -import jakarta.ws.rs.client.Client; -import jakarta.ws.rs.client.ClientBuilder; -import jakarta.ws.rs.core.MediaType; import java.util.List; import java.util.UUID; import org.broadinstitute.consent.http.MockServerTestHelper; -import org.broadinstitute.consent.http.configurations.ServicesConfiguration; -import org.broadinstitute.consent.http.enumeration.DataUseTranslationType; import org.broadinstitute.consent.http.models.DataAccessRequest; import org.broadinstitute.consent.http.models.DataAccessRequestData; import org.broadinstitute.consent.http.models.DataUse; -import org.broadinstitute.consent.http.models.DataUseBuilder; import org.broadinstitute.consent.http.models.OntologyEntry; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; -import org.mockserver.model.Header; @ExtendWith(MockitoExtension.class) class UseRestrictionConverterTest extends MockServerTestHelper { - private void mockDataUseTranslateSuccess() { - mockServerClient - .when(request().withMethod("POST").withPath("/translate")) - .respond( - response() - .withStatusCode(200) - .withHeaders(new Header("Content-Type", MediaType.APPLICATION_JSON)) - .withBody( - """ - Samples are restricted for use under the following conditions: - Data is limited for health/medical/biomedical research. [HMB] - Commercial use is not prohibited. - Data use for methods development research irrespective of the specified data use limitations is not prohibited. - Restrictions for use as a control set for diseases other than those defined were not specified.""")); - } - - private void mockDataUseTranslateFailure() { - mockServerClient - .when(request().withMethod("POST").withPath("/translate")) - .respond( - response() - .withStatusCode(500) - .withHeaders(new Header("Content-Type", MediaType.APPLICATION_JSON)) - .withBody("Exception")); - } - - public ServicesConfiguration config() { - ServicesConfiguration config = new ServicesConfiguration(); - config.setLocalURL("http://localhost:8180/"); - config.setOntologyURL(getRootUrl(CONTAINER)); - return config; - } - - /* - * Test that the UseRestrictionConverter makes a call to the ontology service and gets back a valid translation - */ - @Test - void testTranslateDataUsePurpose() { - mockDataUseTranslateSuccess(); - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); - DataUse dataUse = new DataUseBuilder().setHmbResearch(true).build(); - String translation = converter.translateDataUse(dataUse, DataUseTranslationType.PURPOSE); - assertNotNull(translation); - } - - /* - * Test that the UseRestrictionConverter makes a call to the ontology service and gets back a valid translation - */ - @Test - void testTranslateDataUseDataset() { - mockDataUseTranslateSuccess(); - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); - DataUse dataUse = new DataUseBuilder().setHmbResearch(true).build(); - String translation = converter.translateDataUse(dataUse, DataUseTranslationType.DATASET); - assertNotNull(translation); - } - - /* - * Test that when the UseRestrictionConverter makes a failed call to the ontology service, a null is returned. - */ - @Test - void testFailedDataUseTranslateConverterConnection() { - mockDataUseTranslateFailure(); - - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); - DataUse dataUse = new DataUseBuilder().setHmbResearch(true).build(); - String translation = converter.translateDataUse(dataUse, DataUseTranslationType.PURPOSE); - assertNull(translation); - } - @Test void testParseDataUsePurposeEmpty() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); DataUse dataUse = converter.parseDataUsePurpose(dar); assertNull(dataUse.getGeneralUse()); @@ -135,8 +52,7 @@ void testParseDataUsePurposeEmpty() { @Test void testParseDataUsePurposeFalseAsNull() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); DataAccessRequestData data = new DataAccessRequestData(); @@ -184,8 +100,7 @@ void testParseDataUsePurposeFalseAsNull() { @Test void testParseDataUsePurposeMethods() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setMethods(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -194,8 +109,7 @@ void testParseDataUsePurposeMethods() { @Test void testParseDataUseAiLlmUse() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setAiLlmUse(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -204,8 +118,7 @@ void testParseDataUseAiLlmUse() { @Test void testParseDataUsePurposeControls() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setControls(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -214,8 +127,7 @@ void testParseDataUsePurposeControls() { @Test void testParseDataUsePurposeDisease() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); OntologyEntry entry = new OntologyEntry(); entry.setId("id"); @@ -229,8 +141,7 @@ void testParseDataUsePurposeDisease() { @Test void testParseDataUsePurposeNonProfit() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setForProfit(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -239,8 +150,7 @@ void testParseDataUsePurposeNonProfit() { @Test void testParseDataUsePurposeGender() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setOneGender(true); dar.getData().setGender("F"); @@ -250,8 +160,7 @@ void testParseDataUsePurposeGender() { @Test void testParseDataUsePurposePediatric() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setPediatric(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -260,8 +169,7 @@ void testParseDataUsePurposePediatric() { @Test void testParseDataUsePurposeHMB() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setHmb(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -270,8 +178,7 @@ void testParseDataUsePurposeHMB() { @Test void testParseDataUsePurposeOther() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setOther(true); dar.getData().setOtherText("Other Text"); @@ -281,8 +188,7 @@ void testParseDataUsePurposeOther() { @Test void testParseDataUseIllegalBehavior() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setIllegalBehavior(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -291,8 +197,7 @@ void testParseDataUseIllegalBehavior() { @Test void testParseDataUseSexualDiseases() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setSexualDiseases(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -301,8 +206,7 @@ void testParseDataUseSexualDiseases() { @Test void testParseDataUseStigmatizeDiseases() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setStigmatizedDiseases(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -311,8 +215,7 @@ void testParseDataUseStigmatizeDiseases() { @Test void testParseDataUseVulnerablePopulations() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setVulnerablePopulation(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -321,8 +224,7 @@ void testParseDataUseVulnerablePopulations() { @Test void testParseDataUsePsychologicalTraits() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setPsychiatricTraits(true); DataUse dataUse = converter.parseDataUsePurpose(dar); @@ -331,8 +233,7 @@ void testParseDataUsePsychologicalTraits() { @Test void testParseDataUseNotHealth() { - Client client = ClientBuilder.newClient(); - UseRestrictionConverter converter = new UseRestrictionConverter(client, config()); + UseRestrictionConverter converter = new UseRestrictionConverter(); DataAccessRequest dar = createDataAccessRequest(); dar.getData().setNotHealth(true); DataUse dataUse = converter.parseDataUsePurpose(dar); diff --git a/src/test/java/org/broadinstitute/consent/http/service/VoteServiceTest.java b/src/test/java/org/broadinstitute/consent/http/service/VoteServiceTest.java index b14c35e8e..eefabbbaf 100644 --- a/src/test/java/org/broadinstitute/consent/http/service/VoteServiceTest.java +++ b/src/test/java/org/broadinstitute/consent/http/service/VoteServiceTest.java @@ -78,10 +78,10 @@ class VoteServiceTest extends AbstractTestHelper { @Mock private ElectionDAO electionDAO; @Mock private EmailService emailService; @Mock private ElasticSearchService elasticSearchService; - @Mock private UseRestrictionConverter useRestrictionConverter; @Mock private VoteDAO voteDAO; @Mock private VoteServiceDAO voteServiceDAO; @Mock private User user; + @Mock private OntologyService ontologyService; @BeforeEach void initService() { @@ -94,9 +94,9 @@ void initService() { electionDAO, emailService, elasticSearchService, - useRestrictionConverter, voteDAO, - voteServiceDAO); + voteServiceDAO, + ontologyService); } @Test