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.

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

[ALS-5056] Get Query Site of Origin #148

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package edu.harvard.dbmi.avillach.data.entity;

public enum DataSharingStatus {
Unknown, Pending, Error, Complete, NotShared
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package edu.harvard.dbmi.avillach.data.entity;

import io.swagger.v3.oas.annotations.media.Schema;

import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Table;
import javax.persistence.UniqueConstraint;

@Schema(description = "A site that contains a PIC-SURE installation that we can send data to")
@Table(uniqueConstraints = {
@UniqueConstraint(name = "unique_code", columnNames = { "code" }),
@UniqueConstraint(name = "unique_email", columnNames = { "domain" })
})
@Entity(name = "site")
public class Site extends BaseEntity {

@Schema(description = "The site code. Ex: BCH")
@Column(length = 15)
private String code;

@Schema(description = "The site name. Ex: Boston Children's")
@Column(length = 255)
private String name;

@Schema(description = "The email domain of users for this site. Ex: childrens.harvard.edu")
@Column(length = 255)
private String domain;

public String getCode() {
return code;
}

public void setCode(String code) {
this.code = code;
}

public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}

public String getDomain() {
return domain;
}

public void setDomain(String domain) {
this.domain = domain;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package edu.harvard.dbmi.avillach.data.repository;

import edu.harvard.dbmi.avillach.data.entity.NamedDataset;
import edu.harvard.dbmi.avillach.data.entity.Site;

import javax.enterprise.context.ApplicationScoped;
import javax.transaction.Transactional;
import java.util.UUID;

@Transactional
@ApplicationScoped
public class SiteRepository extends BaseRepository<Site, UUID>{
protected SiteRepository() {super(Site.class);}
}
11 changes: 11 additions & 0 deletions pic-sure-api-data/src/main/resources/db/sql/V6__ADD_SITE_TABLE.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
USE `picsure`;

CREATE TABLE `site` (
`uuid` binary(16) NOT NULL,
`code` varchar(15) COLLATE utf8_bin DEFAULT NULL,
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 should not be unique, same code can map to multiple domains

`name` varchar(255) COLLATE utf8_bin DEFAULT NULL,
`domain` varchar(255) COLLATE utf8_bin DEFAULT NULL,
PRIMARY KEY (`uuid`),
CONSTRAINT `unique_code` UNIQUE (`code`),
CONSTRAINT `unique_domain` UNIQUE (`domain`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@
import javax.ws.rs.core.Context;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.SecurityContext;

import edu.harvard.dbmi.avillach.domain.*;
import edu.harvard.dbmi.avillach.service.FormatService;
import edu.harvard.dbmi.avillach.service.PicsureInfoService;
import edu.harvard.dbmi.avillach.service.PicsureQueryService;
import edu.harvard.dbmi.avillach.service.PicsureSearchService;
import edu.harvard.dbmi.avillach.service.*;
import io.swagger.v3.oas.annotations.OpenAPIDefinition;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.info.Info;
Expand Down Expand Up @@ -153,11 +151,15 @@ public QueryStatus query(

@Parameter
@QueryParam("isInstitute")
Boolean isInstitutionQuery
Boolean isInstitutionQuery,

@Context SecurityContext context
) {
return isInstitutionQuery == null || !isInstitutionQuery ?
queryService.query(dataQueryRequest, headers) :
queryService.institutionalQuery(dataQueryRequest, headers);
if (isInstitutionQuery == null || !isInstitutionQuery) {
return queryService.query(dataQueryRequest, headers);
} else {
return queryService.institutionalQuery(dataQueryRequest, headers, context.getUserPrincipal().getName());
}
}

@POST
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import edu.harvard.dbmi.avillach.data.entity.DataSharingStatus;
import edu.harvard.dbmi.avillach.data.entity.Query;
import edu.harvard.dbmi.avillach.data.entity.Resource;
import edu.harvard.dbmi.avillach.data.repository.QueryRepository;
Expand All @@ -12,6 +13,7 @@
import edu.harvard.dbmi.avillach.util.Utilities;
import edu.harvard.dbmi.avillach.util.exception.ApplicationException;
import edu.harvard.dbmi.avillach.util.exception.ProtocolException;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -47,6 +49,9 @@ public class PicsureQueryService {
@Inject
ResourceWebClient resourceWebClient;

@Inject
SiteParsingService siteParsingService;

/**
* Executes a query on a PIC-SURE resource and creates a Query entity in the
* database for the query.
Expand Down Expand Up @@ -277,7 +282,9 @@ public QueryStatus queryMetadata(UUID queryId, HttpHeaders headers){
* and resource specific query (could be a string or a json object)
* @return {@link QueryStatus}
*/
public QueryStatus institutionalQuery(QueryRequest dataQueryRequest, HttpHeaders headers) {
public QueryStatus institutionalQuery(QueryRequest dataQueryRequest, HttpHeaders headers, String email) {
String siteCode = siteParsingService.parseSiteOfOrigin(email).orElseThrow(() -> new RuntimeException("Bad email"));
Copy link
Member Author

Choose a reason for hiding this comment

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

Do I want to throw or just null the field

Copy link
Member Author

Choose a reason for hiding this comment

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

But document the error better in the logs

dataQueryRequest.setInstitutionOfOrigin(siteCode);
Resource resource = verifyQueryRequest(dataQueryRequest, headers);
dataQueryRequest.getResourceCredentials().put(ResourceWebClient.BEARER_TOKEN_KEY, resource.getToken());

Expand Down Expand Up @@ -314,6 +321,12 @@ private Query copyQuery(QueryRequest dataQueryRequest, Resource resource, QueryS
metaData.put("commonAreaUUID", dataQueryRequest.getCommonAreaUUID());
}

if (!StringUtils.isEmpty(dataQueryRequest.getInstitutionOfOrigin())) {
metaData.put("site", dataQueryRequest.getInstitutionOfOrigin());
}

metaData.put("sharingStatus", DataSharingStatus.Unknown);

queryEntity.setQuery(queryJson);

if (!metaData.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package edu.harvard.dbmi.avillach.service;

import edu.harvard.dbmi.avillach.data.entity.Site;
import edu.harvard.dbmi.avillach.data.repository.SiteRepository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.inject.Inject;
import java.util.List;
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class SiteParsingService {
Copy link
Member Author

Choose a reason for hiding this comment

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

Unit test


private static final Logger LOG = LoggerFactory.getLogger(SiteParsingService.class);

private static final Pattern emailRegex = Pattern.compile("^([^@]+)(@)(.*)$");

@Inject
SiteRepository repository;

public Optional<String> parseSiteOfOrigin(String email) {
Matcher matcher = emailRegex.matcher(email);
if (!matcher.find()) {
LOG.warn("Unable to parse domain for email: {}", email);
return Optional.empty();
}

List<Site> matchingDomains = repository.getByColumn("domain", matcher.group(3));
if (matchingDomains.isEmpty()) {
LOG.warn("Unable to match domain for email: {}, looked for domain: {}", email, matcher.group(3));
return Optional.empty();
}
if (matchingDomains.size() > 1) {
LOG.warn("Multiple domains match email. This should never happen! Email: {}", email);
return Optional.empty();
}
return Optional.of(matchingDomains.get(0).getCode());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<class>edu.harvard.dbmi.avillach.data.entity.Query</class>
<class>edu.harvard.dbmi.avillach.data.entity.Resource</class>
<class>edu.harvard.dbmi.avillach.data.entity.NamedDataset</class>
<class>edu.harvard.dbmi.avillach.data.entity.Site</class>

<properties>
<property name="hibernate.archive.autodetection" value="class" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package edu.harvard.dbmi.avillach.service;

import edu.harvard.dbmi.avillach.data.entity.Site;
import edu.harvard.dbmi.avillach.data.repository.SiteRepository;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;

import java.util.List;
import java.util.Optional;

import static org.junit.Assert.assertEquals;

@RunWith(MockitoJUnitRunner.class)
public class SiteParsingServiceTest {

@InjectMocks
private SiteParsingService subject;

@Mock
private SiteRepository repository;

@Test
public void shouldParse() {
Site site = new Site();
site.setCode("BCH");
site.setName("Bowston Children's Hospital");
site.setDomain("childrens.harvard.edu");
Mockito
.when(repository.getByColumn("domain", "childrens.harvard.edu"))
.thenReturn(List.of(site));

Optional<String> actual = subject.parseSiteOfOrigin("[email protected]");
Optional<String> expected = Optional.of("BCH");

Assert.assertEquals(expected, actual);
}

@Test
public void shouldFailWhenNoSite() {
Mockito
.when(repository.getByColumn("domain", "childrens.harvard.edu"))
.thenReturn(List.of());

Optional<String> actual = subject.parseSiteOfOrigin("[email protected]");
Optional<String> expected = Optional.empty();

Assert.assertEquals(expected, actual);
}

@Test
public void shouldFailWhenManySites() {
Site siteA = new Site();
siteA.setCode("BCH");
siteA.setName("Bowston Children's Hospital");
siteA.setDomain("edu");
Site siteB = new Site();
siteB.setCode("CHOP");
siteB.setName("Children's Hospital of Philly");
siteB.setDomain("edu");
Mockito
.when(repository.getByColumn("domain", "edu"))
.thenReturn(List.of(siteA, siteB));

Optional<String> actual = subject.parseSiteOfOrigin("aaaaaaah@edu");
Optional<String> expected = Optional.empty();

Assert.assertEquals(expected, actual);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@

import javax.inject.Inject;
import javax.ws.rs.*;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.SecurityContext;

import org.apache.http.Header;
import org.apache.http.HttpResponse;
Expand Down Expand Up @@ -102,6 +104,7 @@ public QueryStatus query(QueryRequest queryRequest) {
chainRequest.setResourceCredentials(queryRequest.getResourceCredentials());
chainRequest.setResourceUUID(UUID.fromString(properties.getTargetResourceId()));
chainRequest.setCommonAreaUUID(queryRequest.getCommonAreaUUID());
chainRequest.setInstitutionOfOrigin(queryRequest.getInstitutionOfOrigin());

String payload = objectMapper.writeValueAsString(chainRequest);
HttpResponse response = httpClient.retrievePostResponse(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ public class QueryRequest {
@Schema(hidden = true)
private UUID commonAreaUUID;

@Schema(hidden = true)
Copy link
Member Author

Choose a reason for hiding this comment

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

Discuss: we (I) are bloating the queryrequest object with GIC specific fields. Maybe a sumtype would be appropriate here? Make QueryRequestTemplate, have QueryRequest extend it, then have a GICQueryRequest with extra fields. Jackson can handle this.

private String institutionOfOrigin;

public Map<String, String> getResourceCredentials() {
return resourceCredentials;
}
Expand Down Expand Up @@ -87,4 +90,12 @@ public UUID getCommonAreaUUID() {
public void setCommonAreaUUID(UUID commonAreaUUID) {
this.commonAreaUUID = commonAreaUUID;
}

public String getInstitutionOfOrigin() {
return institutionOfOrigin;
}

public void setInstitutionOfOrigin(String institutionOfOrigin) {
this.institutionOfOrigin = institutionOfOrigin;
}
}