Skip to content

Commit

Permalink
[ALS-7760] Replicate Old Search in new Data-Dictionary (#53)
Browse files Browse the repository at this point in the history
**[CHORE] GH Actions Fix**
- Fixed GitHub Actions to resolve workflow issues.

**Testing Enhancements**
- Added `@ActiveProfiles("test")` to testing classes to remove spam logs from `DataSourceVerifier` during unit tests.

**JSON Parsing Refactor**
- Created `JsonBlobParser` for improved JSON parsing.
- Refactored `ConceptResultSetUtil` to use `JsonBlobParser` for clearer separation of concerns.

**Code Cleanup**
- Removed unused imports from `ConceptShell` and `ContinuousConcept` classes to improve code readability.

**Legacy Search Feature**
- Implemented a new legacy search feature including service, controller, and related model classes.
- Updated `ConceptRepository` to support legacy search queries.
- Added test cases to ensure functionality.

**Testing Improvements**
- Added unit tests for `LegacySearchQueryMapper` to validate JSON parsing and string replacement.
- Introduced integration tests for `LegacySearchController` to verify search response correctness using a PostgreSQL container.

**Initial Configurations**
- Added application properties for database configuration and dashboard settings.
- Introduced Docker commands for local development with sample weights configuration.
- Included `weights.csv` and `dictonaryRequest.http` as sample data for testing API requests.

**Search Query Refactor**
- Created `LegacySearchRepository` to handle legacy search functionalities.
- Moved query logic (`ALLOW_FILTERING_Q`) to `QueryUtility`.
- Removed legacy search code from `ConceptRepository` for better separation of concerns.

**Filter Processing Refactor**
- Introduced `FilterProcessor` to centralize filter processing logic.
- Enhanced methods in `MetadataResultSetUtil` (`getDescription`, `getParentName`, `getParentDisplay`) for better validation using `StringUtils`.

**Repository Enhancements**
- Added `LegacySearchRepositoryTest` to verify legacy search functionalities.
- Refactored legacy search logic from `ConceptService` into `LegacySearchRepository`.
- Cleaned up unused imports and methods for better maintainability.

---

Co-authored-by: Luke Sikina <lucas.skina@gmail.com>
  • Loading branch information
Gcolon021 and Luke Sikina authored Nov 18, 2024
1 parent cd06343 commit 89d5728
Show file tree
Hide file tree
Showing 47 changed files with 828 additions and 101 deletions.
11 changes: 11 additions & 0 deletions dictionaryweights/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
## Docker commands for local development
### Docker build
```bash
docker build --no-cache --build-arg SPRING_PROFILE=bdc-dev -t weights:latest .
```

### Docker run
You will need a local weights.csv file.
```bash
docker run --rm -t --name dictionary-weights --network=host -v ./weights.csv:/weights.csv weights:latest
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
spring.application.name=dictionaryweights
spring.main.web-application-type=none

spring.datasource.url=jdbc:postgresql://localhost:5432/dictionary_db?currentSchema=dict
spring.datasource.username=username
spring.datasource.password=password
spring.datasource.driver-class-name=org.postgresql.Driver

weights.filename=/weights.csv
7 changes: 7 additions & 0 deletions dictionaryweights/weights.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
concept_node.DISPLAY,2
concept_node.CONCEPT_PATH,2
dataset.FULL_NAME,1
dataset.DESCRIPTION,1
parent.DISPLAY,1
grandparent.DISPLAY,1
concept_node_meta_str,1
15 changes: 15 additions & 0 deletions dictonaryReqeust.http
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# curl 'https://dev.picsure.biodatacatalyst.nhlbi.nih.gov/picsure/proxy/dictionary-api/concepts?page_number=1&page_size=1'
# -H 'origin: https://dev.picsure.biodatacatalyst.nhlbi.nih.gov'
# -H 'referer: https://dev.picsure.biodatacatalyst.nhlbi.nih.gov/'
# --data-raw '{"facets":[],"search":"","consents":[]}'
POST http://localhost:80/concepts?page_number=0&page_size=100
Content-Type: application/json

{"facets":[],"search":"lipid triglyceride"}

###

POST http://localhost:80/search
Content-Type: application/json

{"@type":"GeneralQueryRequest","resourceCredentials":{},"query":{"searchTerm":"breast","includedTags":[],"excludedTags":[],"returnTags":"true","offset":0,"limit":10000000},"resourceUUID":null}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import edu.harvard.dbmi.avillach.dictionary.concept.model.Concept;
import edu.harvard.dbmi.avillach.dictionary.filter.Filter;
import edu.harvard.dbmi.avillach.dictionary.filter.QueryParamPair;
import edu.harvard.dbmi.avillach.dictionary.legacysearch.SearchResultRowMapper;
import edu.harvard.dbmi.avillach.dictionary.util.MapExtractor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
Expand All @@ -15,32 +16,19 @@
import java.util.Map;
import java.util.Optional;

import static edu.harvard.dbmi.avillach.dictionary.util.QueryUtility.ALLOW_FILTERING_Q;


@Repository
public class ConceptRepository {

private static final String ALLOW_FILTERING_Q = """
WITH allow_filtering AS (
SELECT
concept_node.concept_node_id AS concept_node_id,
(string_agg(concept_node_meta.value, ' ') NOT LIKE '%' || 'true' || '%') AS allowFiltering
FROM
concept_node
JOIN concept_node_meta ON
concept_node.concept_node_id = concept_node_meta.concept_node_id
AND concept_node_meta.KEY IN (:disallowed_meta_keys)
GROUP BY
concept_node.concept_node_id
)
""";

private final NamedParameterJdbcTemplate template;
private final ConceptRowMapper mapper;
private final ConceptFilterQueryGenerator filterGen;
private final ConceptMetaExtractor conceptMetaExtractor;
private final ConceptResultSetExtractor conceptResultSetExtractor;
private final List<String> disallowedMetaFields;


@Autowired
public ConceptRepository(
NamedParameterJdbcTemplate template, ConceptRowMapper mapper, ConceptFilterQueryGenerator filterGen,
Expand Down Expand Up @@ -240,4 +228,6 @@ WITH RECURSIVE nodes AS (
return Optional.ofNullable(template.query(sql, params, conceptResultSetExtractor));

}


}
Original file line number Diff line number Diff line change
Expand Up @@ -2,83 +2,43 @@

import edu.harvard.dbmi.avillach.dictionary.concept.model.CategoricalConcept;
import edu.harvard.dbmi.avillach.dictionary.concept.model.ContinuousConcept;
import org.json.JSONArray;
import org.json.JSONException;
import edu.harvard.dbmi.avillach.dictionary.util.JsonBlobParser;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

import java.math.BigDecimal;
import java.math.BigInteger;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

@Component
public class ConceptResultSetUtil {

private static final Logger log = LoggerFactory.getLogger(ConceptResultSetUtil.class);
private final JsonBlobParser jsonBlobParser;

@Autowired
public ConceptResultSetUtil(JsonBlobParser jsonBlobParser) {
this.jsonBlobParser = jsonBlobParser;
}

public CategoricalConcept mapCategorical(ResultSet rs) throws SQLException {
return new CategoricalConcept(
rs.getString("concept_path"), rs.getString("name"), rs.getString("display"), rs.getString("dataset"),
rs.getString("description"), rs.getString("values") == null ? List.of() : parseValues(rs.getString("values")),
rs.getString("description"), rs.getString("values") == null ? List.of() : jsonBlobParser.parseValues(rs.getString("values")),
rs.getBoolean("allowFiltering"), rs.getString("studyAcronym"), null, null
);
}

public ContinuousConcept mapContinuous(ResultSet rs) throws SQLException {
return new ContinuousConcept(
rs.getString("concept_path"), rs.getString("name"), rs.getString("display"), rs.getString("dataset"),
rs.getString("description"), rs.getBoolean("allowFiltering"), parseMin(rs.getString("values")),
parseMax(rs.getString("values")), rs.getString("studyAcronym"), null
rs.getString("description"), rs.getBoolean("allowFiltering"), jsonBlobParser.parseMin(rs.getString("values")),
jsonBlobParser.parseMax(rs.getString("values")), rs.getString("studyAcronym"), null
);
}

public List<String> parseValues(String valuesArr) {
try {
ArrayList<String> vals = new ArrayList<>();
JSONArray arr = new JSONArray(valuesArr);
for (int i = 0; i < arr.length(); i++) {
vals.add(arr.getString(i));
}
return vals;
} catch (JSONException ex) {
return List.of();
}
}

public Float parseMin(String valuesArr) {
return parseFromIndex(valuesArr, 0);
}

private Float parseFromIndex(String valuesArr, int index) {
try {
JSONArray arr = new JSONArray(valuesArr);
if (arr.length() != 2) {
return 0F;
}
Object raw = arr.get(index);
return switch (raw) {
case Double d -> d.floatValue();
case Integer i -> i.floatValue();
case String s -> Double.valueOf(s).floatValue();
case BigDecimal d -> d.floatValue();
case BigInteger i -> i.floatValue();
default -> 0f;
};
} catch (JSONException ex) {
log.warn("Invalid json array for values: ", ex);
return 0F;
} catch (NumberFormatException ex) {
log.warn("Valid json array but invalid val within: ", ex);
return 0F;
}
}

public Float parseMax(String valuesArr) {
return parseFromIndex(valuesArr, 1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,5 @@ public Optional<Concept> conceptTree(String dataset, String conceptPath, int dep
public Optional<Concept> conceptDetailWithoutAncestors(String dataset, String conceptPath) {
return getConcept(dataset, conceptPath, false);
}

}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package edu.harvard.dbmi.avillach.dictionary.concept.model;

import edu.harvard.dbmi.avillach.dictionary.dataset.Dataset;
import jakarta.annotation.Nullable;

import java.util.List;
import java.util.Map;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import edu.harvard.dbmi.avillach.dictionary.dataset.Dataset;
import jakarta.annotation.Nullable;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Profile;
import org.springframework.context.event.ContextRefreshedEvent;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Service;

import javax.sql.DataSource;
import java.sql.Connection;
import java.sql.SQLException;

@Service
@Profile("!test")
@Configuration
public class DataSourceVerifier {

private static final Logger LOG = LoggerFactory.getLogger(DataSourceVerifier.class);
Expand All @@ -28,11 +30,10 @@ public void verifyDataSourceConnection() {
try (Connection connection = dataSource.getConnection()) {
if (connection != null) {
LOG.info("Datasource connection verified successfully.");
} else {
LOG.info("Failed to obtain a connection from the datasource.");
}
} catch (SQLException e) {
LOG.info("Error verifying datasource connection: {}", e.getMessage());
LOG.info("Failed to obtain a connection from the datasource.");
LOG.debug("Error verifying datasource connection: {}", e.getMessage());
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,28 @@
package edu.harvard.dbmi.avillach.dictionary.facet;

import edu.harvard.dbmi.avillach.dictionary.filter.Filter;
import edu.harvard.dbmi.avillach.dictionary.filter.FilterProcessor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.core.MethodParameter;
import org.springframework.http.HttpInputMessage;
import org.springframework.http.converter.HttpMessageConverter;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.servlet.mvc.method.annotation.RequestBodyAdvice;

import java.io.IOException;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.function.Function;

@ControllerAdvice
public class FilterPreProcessor implements RequestBodyAdvice {

private final FilterProcessor filterProcessor;

@Autowired
public FilterPreProcessor(FilterProcessor filterProcessor) {
this.filterProcessor = filterProcessor;
}


@Override
public boolean supports(MethodParameter methodParameter, Type targetType, Class<? extends HttpMessageConverter<?>> converterType) {
return true;
Expand All @@ -35,26 +41,13 @@ public Object afterBodyRead(
Class<? extends HttpMessageConverter<?>> converterType
) {
if (body instanceof Filter filter) {
List<Facet> newFacets = filter.facets();
List<String> newConsents = filter.consents();
if (filter.facets() != null) {
newFacets = new ArrayList<>(filter.facets());
newFacets.sort(Comparator.comparing(Facet::name));
}
if (filter.consents() != null) {
newConsents = new ArrayList<>(newConsents);
newConsents.sort(Comparator.comparing(Function.identity()));
}
filter = new Filter(newFacets, filter.search(), newConsents);

if (StringUtils.hasLength(filter.search())) {
filter = new Filter(filter.facets(), filter.search().replaceAll("_", "/"), filter.consents());
}
return filter;
return filterProcessor.processsFilter(filter);
}
return body;
}



@Override
public Object handleEmptyBody(
Object body, HttpInputMessage inputMessage, MethodParameter parameter, Type targetType,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package edu.harvard.dbmi.avillach.dictionary.filter;

import edu.harvard.dbmi.avillach.dictionary.facet.Facet;
import org.springframework.stereotype.Component;
import org.springframework.util.StringUtils;

import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.function.Function;

@Component
public class FilterProcessor {

public Filter processsFilter(Filter filter) {
List<Facet> newFacets = filter.facets();
List<String> newConsents = filter.consents();
if (filter.facets() != null) {
newFacets = new ArrayList<>(filter.facets());
newFacets.sort(Comparator.comparing(Facet::name));
}
if (filter.consents() != null) {
newConsents = new ArrayList<>(newConsents);
newConsents.sort(Comparator.comparing(Function.identity()));
}
filter = new Filter(newFacets, filter.search(), newConsents);

if (StringUtils.hasLength(filter.search())) {
filter = new Filter(filter.facets(), filter.search().replaceAll("_", "/"), filter.consents());
}
return filter;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package edu.harvard.dbmi.avillach.dictionary.legacysearch;

import edu.harvard.dbmi.avillach.dictionary.legacysearch.model.LegacyResponse;
import edu.harvard.dbmi.avillach.dictionary.legacysearch.model.LegacySearchQuery;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;

import java.io.IOException;

@Controller
public class LegacySearchController {

private final LegacySearchService legacySearchService;
private final LegacySearchQueryMapper legacySearchQueryMapper;

@Autowired
public LegacySearchController(LegacySearchService legacySearchService, LegacySearchQueryMapper legacySearchQueryMapper) {
this.legacySearchService = legacySearchService;
this.legacySearchQueryMapper = legacySearchQueryMapper;
}

@RequestMapping(path = "/search")
public ResponseEntity<LegacyResponse> legacySearch(@RequestBody String jsonString) throws IOException {
LegacySearchQuery legacySearchQuery = legacySearchQueryMapper.mapFromJson(jsonString);
return ResponseEntity
.ok(new LegacyResponse(legacySearchService.getSearchResults(legacySearchQuery.filter(), legacySearchQuery.pageable())));
}

}
Loading

0 comments on commit 89d5728

Please sign in to comment.