Skip to content

Commit

Permalink
[ALS-6276] Search improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
Luke Sikina committed Jul 8, 2024
1 parent 5eeab9e commit 87e7f77
Show file tree
Hide file tree
Showing 12 changed files with 136 additions and 75 deletions.
1 change: 1 addition & 0 deletions db/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ CREATE TABLE dict.concept_node (
CONCEPT_TYPE VARCHAR(32) NOT NULL DEFAULT 'Interior',
CONCEPT_PATH VARCHAR(10000) NOT NULL DEFAULT 'INVALID',
PARENT_ID INT,
SEARCHABLE_FIELDS TSVECTOR,
PRIMARY KEY (CONCEPT_NODE_ID),
CONSTRAINT fk_parent FOREIGN KEY (PARENT_ID) REFERENCES dict.CONCEPT_NODE(CONCEPT_NODE_ID),
CONSTRAINT fk_study FOREIGN KEY (DATASET_ID) REFERENCES dict.dataset(DATASET_ID)
Expand Down
5 changes: 5 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@
<artifactId>spring-boot-starter-test</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.json</groupId>
<artifactId>json</artifactId>
<version>20240303</version>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-testcontainers</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,15 @@ public List<Concept> getConcepts(Filter filter, Pageable pageable) {
concept_node.*,
ds.REF as dataset,
continuous_min.VALUE as min, continuous_max.VALUE as max,
categorical_values.VALUE as values
categorical_values.VALUE as values,
meta_description.VALUE AS description
FROM
concept_node
LEFT JOIN dataset AS ds ON concept_node.dataset_id = ds.dataset_id
LEFT JOIN concept_node_meta AS continuous_min ON concept_node.concept_node_id = continuous_min.concept_node_id AND continuous_min.KEY = 'MIN'
LEFT JOIN concept_node_meta AS continuous_max ON concept_node.concept_node_id = continuous_max.concept_node_id AND continuous_max.KEY = 'MAX'
LEFT JOIN concept_node_meta AS categorical_values ON concept_node.concept_node_id = categorical_values.concept_node_id AND categorical_values.KEY = 'VALUES'
LEFT JOIN concept_node_meta AS meta_description ON concept_node.concept_node_id = meta_description.concept_node_id AND meta_description.KEY = 'description'
LEFT JOIN concept_node_meta AS continuous_min ON concept_node.concept_node_id = continuous_min.concept_node_id AND continuous_min.KEY = 'min'
LEFT JOIN concept_node_meta AS continuous_max ON concept_node.concept_node_id = continuous_max.concept_node_id AND continuous_max.KEY = 'max'
LEFT JOIN concept_node_meta AS categorical_values ON concept_node.concept_node_id = categorical_values.concept_node_id AND categorical_values.KEY = 'values'
WHERE concept_node.concept_node_id IN
""";
QueryParamPair filterQ = filterGen.generateFilterQuery(filter, pageable);
Expand All @@ -69,13 +71,15 @@ public Optional<Concept> getConcept(String dataset, String conceptPath) {
concept_node.*,
ds.REF as dataset,
continuous_min.VALUE as min, continuous_max.VALUE as max,
categorical_values.VALUE as values
categorical_values.VALUE as values,
meta_description.VALUE AS description
FROM
concept_node
LEFT JOIN dataset AS ds ON concept_node.dataset_id = ds.dataset_id
LEFT JOIN concept_node_meta AS continuous_min ON concept_node.concept_node_id = continuous_min.concept_node_id AND continuous_min.KEY = 'MIN'
LEFT JOIN concept_node_meta AS continuous_max ON concept_node.concept_node_id = continuous_max.concept_node_id AND continuous_max.KEY = 'MAX'
LEFT JOIN concept_node_meta AS categorical_values ON concept_node.concept_node_id = categorical_values.concept_node_id AND categorical_values.KEY = 'VALUES'
LEFT JOIN concept_node_meta AS meta_description ON concept_node.concept_node_id = meta_description.concept_node_id AND meta_description.KEY = 'description'
LEFT JOIN concept_node_meta AS continuous_min ON concept_node.concept_node_id = continuous_min.concept_node_id AND continuous_min.KEY = 'min'
LEFT JOIN concept_node_meta AS continuous_max ON concept_node.concept_node_id = continuous_max.concept_node_id AND continuous_max.KEY = 'max'
LEFT JOIN concept_node_meta AS categorical_values ON concept_node.concept_node_id = categorical_values.concept_node_id AND categorical_values.KEY = 'values'
WHERE
concept_node.concept_path = :conceptPath
AND ds.REF = :dataset
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package edu.harvard.dbmi.avillach.dictionary.concept;

import edu.harvard.dbmi.avillach.dictionary.concept.model.*;
import org.json.JSONArray;
import org.json.JSONException;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.stereotype.Component;
import org.springframework.util.StringUtils;
Expand All @@ -24,7 +26,7 @@ public Concept mapRow(ResultSet rs, int rowNum) throws SQLException {
private CategoricalConcept mapCategorical(ResultSet rs) throws SQLException {
return new CategoricalConcept(
rs.getString("concept_path"), rs.getString("name"),
rs.getString("display"), rs.getString("dataset"),
rs.getString("display"), rs.getString("dataset"), rs.getString("description"),
rs.getString("values") == null ? List.of() : List.of(rs.getString("values").split(",")),
null,
null
Expand All @@ -34,9 +36,27 @@ private CategoricalConcept mapCategorical(ResultSet rs) throws SQLException {
private ContinuousConcept mapContinuous(ResultSet rs) throws SQLException {
return new ContinuousConcept(
rs.getString("concept_path"), rs.getString("name"),
rs.getString("display"), rs.getString("dataset"),
rs.getInt("min"), rs.getInt("max"),
rs.getString("display"), rs.getString("dataset"), rs.getString("description"),
parseMin(rs.getString("values")), parseMax(rs.getString("values")),
null
);
}

private Integer parseMin(String valuesArr) {
try {
JSONArray arr = new JSONArray(valuesArr);
return arr.length() == 2 ? arr.getInt(0) : 0;
} catch (JSONException ex) {
return 0;
}
}

private Integer parseMax(String valuesArr) {
try {
JSONArray arr = new JSONArray(valuesArr);
return arr.length() == 2 ? arr.getInt(1) : 0;
} catch (JSONException ex) {
return 0;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package edu.harvard.dbmi.avillach.dictionary.concept.model;

import com.fasterxml.jackson.annotation.JsonProperty;
import jakarta.annotation.Nullable;

import java.util.List;
import java.util.Map;

public record CategoricalConcept(
String conceptPath, String name, String display, String dataset,
String conceptPath, String name, String display, String dataset, String description,

List<String> values,

Expand All @@ -19,10 +20,11 @@ public record CategoricalConcept(
) implements Concept {

public CategoricalConcept(CategoricalConcept core, Map<String, String> meta) {
this(core.conceptPath, core.name, core.display, core.dataset, core.values, core.children, meta);
this(core.conceptPath, core.name, core.display, core.dataset, core.description, core.values, core.children, meta);
}


@JsonProperty("type")
@Override
public ConceptType type() {
return ConceptType.Categorical;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
package edu.harvard.dbmi.avillach.dictionary.concept.model;

import com.fasterxml.jackson.annotation.JsonProperty;
import jakarta.annotation.Nullable;

import java.util.Map;

public record ContinuousConcept(
String conceptPath, String name, String display, String dataset,
String conceptPath, String name, String display, String dataset, String description,

@Nullable Integer min, @Nullable Integer max,
Map<String, String> meta
) implements Concept {

public ContinuousConcept(ContinuousConcept core, Map<String, String> meta) {
this(core.conceptPath, core.name, core.display, core.dataset, core.min, core.max, meta);
this(core.conceptPath, core.name, core.display, core.dataset, core.description, core.min, core.max, meta);
}

@JsonProperty("type")
@Override
public ConceptType type() {
return ConceptType.Continuous;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public List<FacetCategory> getFacets(Filter filter) {
facet
LEFT JOIN facet_category ON facet_category.facet_category_id = facet.facet_category_id
LEFT JOIN facet as parent_facet ON facet.parent_id = parent_facet.facet_id
LEFT JOIN (
INNER JOIN (
SELECT
count(*) as facet_count, inner_facet_q.facet_id AS inner_facet_id
FROM
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,23 @@ public QueryParamPair generateFilterQuery(Filter filter, Pageable pageable) {
if (StringUtils.hasText(filter.search())) {
clauses.add(createSearchFilter(filter.search(), params));
}
if (clauses.isEmpty()) {
clauses = List.of("\tSELECT concept_node.concept_node_id FROM concept_node\n");
}
clauses.add("""
(
SELECT
concept_node.concept_node_id
FROM
concept_node
LEFT JOIN concept_node_meta AS continuous_min ON concept_node.concept_node_id = continuous_min.concept_node_id AND continuous_min.KEY = 'min'
LEFT JOIN concept_node_meta AS continuous_max ON concept_node.concept_node_id = continuous_max.concept_node_id AND continuous_max.KEY = 'max'
LEFT JOIN concept_node_meta AS categorical_values ON concept_node.concept_node_id = categorical_values.concept_node_id AND categorical_values.KEY = 'values'
WHERE
continuous_min.value <> '' OR
continuous_max.value <> '' OR
categorical_values.value <> ''
)
"""
);


String query = "(\n" + String.join("\n\tINTERSECT\n", clauses) + "\n) ORDER BY concept_node_id\n";
if (pageable.isPaged()) {
Expand All @@ -53,15 +67,15 @@ public QueryParamPair generateFilterQuery(Filter filter, Pageable pageable) {
}

private String createSearchFilter(String search, MapSqlParameterSource params) {
params.addValue("search", "%" + search + "%");
params.addValue("search", search);
return """
(
SELECT
concept_node.concept_node_id AS concept_node_id
FROM
concept_node
WHERE
concept_node.concept_path LIKE :search
concept_node.searchable_fields @@ (phraseto_tsquery(:search)::text || ':*')::tsquery
)
""";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ class ConceptControllerTest {
@Test
void shouldListConcepts() {
List<Concept> expected = List.of(
new CategoricalConcept("/foo", "foo", "Foo", "my_dataset", List.of(), null, Map.of()),
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", List.of("a", "b"), List.of(), Map.of()),
new ContinuousConcept("/foo//baz", "baz", "Baz", "my_dataset", 0, 100, Map.of())
new CategoricalConcept("/foo", "foo", "Foo", "my_dataset", "foo!", List.of(), null, Map.of()),
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", "foo!", List.of("a", "b"), List.of(), Map.of()),
new ContinuousConcept("/foo//baz", "baz", "Baz", "my_dataset", "foo!", 0, 100, Map.of())
);
Filter filter = new Filter(
List.of(new Facet("questionare", "Questionare", "?", 1, null, "category", null)),
Expand All @@ -54,7 +54,7 @@ void shouldListConcepts() {
@Test
void shouldGetConceptDetails() {
CategoricalConcept expected =
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", List.of("a", "b"), List.of(), Map.of());
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", "foo!", List.of("a", "b"), List.of(), Map.of());
Mockito.when(conceptService.conceptDetail("my_dataset", "/foo//bar"))
.thenReturn(Optional.of(expected));

Expand All @@ -77,11 +77,11 @@ void shouldNotGetConceptDetails() {
@Test
void shouldGetConceptTree() {
Concept fooBar =
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", List.of("a", "b"), List.of(), Map.of());
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", "foo!", List.of("a", "b"), List.of(), Map.of());
Concept fooBaz =
new ContinuousConcept("/foo//baz", "baz", "Baz", "my_dataset", 0, 100, Map.of());
new ContinuousConcept("/foo//baz", "baz", "Baz", "my_dataset", "foo!", 0, 100, Map.of());
CategoricalConcept foo =
new CategoricalConcept("/foo", "foo", "Foo", "my_dataset", List.of(), List.of(fooBar, fooBaz), Map.of());
new CategoricalConcept("/foo", "foo", "Foo", "my_dataset", "foo!", List.of(), List.of(fooBar, fooBaz), Map.of());

Mockito.when(conceptService.conceptTree("my_dataset", "/foo", 1))
.thenReturn(Optional.of(foo));
Expand All @@ -95,11 +95,11 @@ void shouldGetConceptTree() {
@Test
void shouldGetNotConceptTreeForLargeDepth() {
Concept fooBar =
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", List.of("a", "b"), List.of(), Map.of());
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", "foo!", List.of("a", "b"), List.of(), Map.of());
Concept fooBaz =
new ContinuousConcept("/foo//baz", "baz", "Baz", "my_dataset", 0, 100, Map.of());
new ContinuousConcept("/foo//baz", "baz", "Baz", "my_dataset", "foo!", 0, 100, Map.of());
CategoricalConcept foo =
new CategoricalConcept("/foo", "foo", "Foo", "my_dataset", List.of(), List.of(fooBar, fooBaz), Map.of());
new CategoricalConcept("/foo", "foo", "Foo", "my_dataset", "foo!", List.of(), List.of(fooBar, fooBaz), Map.of());

Mockito.when(conceptService.conceptTree("my_dataset", "/foo", 1))
.thenReturn(Optional.of(foo));
Expand All @@ -113,11 +113,11 @@ void shouldGetNotConceptTreeForLargeDepth() {
@Test
void shouldGetNotConceptTreeForNegativeDepth() {
Concept fooBar =
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", List.of("a", "b"), List.of(), Map.of());
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", "foo!", List.of("a", "b"), List.of(), Map.of());
Concept fooBaz =
new ContinuousConcept("/foo//baz", "baz", "Baz", "my_dataset", 0, 100, Map.of());
new ContinuousConcept("/foo//baz", "baz", "Baz", "my_dataset", "foo!", 0, 100, Map.of());
CategoricalConcept foo =
new CategoricalConcept("/foo", "foo", "Foo", "my_dataset", List.of(), List.of(fooBar, fooBaz), Map.of());
new CategoricalConcept("/foo", "foo", "Foo", "my_dataset", "foo!", List.of(), List.of(fooBar, fooBaz), Map.of());
Mockito.when(conceptService.conceptTree("my_dataset", "/foo", -1))
.thenReturn(Optional.of(foo));

Expand All @@ -130,11 +130,11 @@ void shouldGetNotConceptTreeForNegativeDepth() {
@Test
void shouldNotGetConceptTreeWhenConceptDNE() {
Concept fooBar =
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", List.of("a", "b"), List.of(), Map.of());
new CategoricalConcept("/foo//bar", "bar", "Bar", "my_dataset", "foo!", List.of("a", "b"), List.of(), Map.of());
Concept fooBaz =
new ContinuousConcept("/foo//baz", "baz", "Baz", "my_dataset", 0, 100, Map.of());
new ContinuousConcept("/foo//baz", "baz", "Baz", "my_dataset", "foo!", 0, 100, Map.of());
CategoricalConcept foo =
new CategoricalConcept("/foo", "foo", "Foo", "my_dataset", List.of(), List.of(fooBar, fooBaz), Map.of());
new CategoricalConcept("/foo", "foo", "Foo", "my_dataset", "foo!", List.of(), List.of(fooBar, fooBaz), Map.of());

Mockito.when(conceptService.conceptTree("my_dataset", "/foo", 1))
.thenReturn(Optional.of(foo));
Expand Down
Loading

0 comments on commit 87e7f77

Please sign in to comment.