-
Notifications
You must be signed in to change notification settings - Fork 0
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-7694] Add DashboardDrawer feature with controller, service, and repository #55
Changes from all commits
43280f4
fe86e5b
f89fa61
82303a1
d5706f9
6ad0322
2463c49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer; | ||
|
||
import java.util.List; | ||
|
||
public record DashboardDrawer( | ||
int datasetId, String studyFullname, String studyAbbreviation, List<String> consentGroups, String studySummary, List<String> studyFocus, | ||
String studyDesign, String sponsor | ||
) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the difference between There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea I was in the same boat at the beginning and was trying to fit it into to the Dashboard thus why it was basically a replication of it until changed the class names. I think I should follow up on this ticket with Cleaning up some of the dashboard method and looking at how this integrates with the dashboard more closely. From more of a backend mindset I think it should relate to a dataset endpoint and not a feature endpoint. |
||
|
||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.stereotype.Controller; | ||
import org.springframework.web.bind.annotation.*; | ||
|
||
@Controller | ||
@RequestMapping("/dashboard-drawer") | ||
public class DashboardDrawerController { | ||
|
||
@Autowired | ||
private DashboardDrawerService dashboardDrawerService; | ||
|
||
@GetMapping | ||
public ResponseEntity<DashboardDrawerList> findAll() { | ||
return dashboardDrawerService.findAll().map(ResponseEntity::ok).orElseGet(() -> ResponseEntity.notFound().build()); | ||
} | ||
|
||
@GetMapping("/{id}") | ||
public ResponseEntity<DashboardDrawer> findByDatasetId(@PathVariable Integer id) { | ||
return dashboardDrawerService.findByDatasetId(id).map(ResponseEntity::ok).orElseGet(() -> ResponseEntity.notFound().build()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer; | ||
|
||
import java.util.List; | ||
|
||
public record DashboardDrawerList(List<DashboardDrawer> dashboardDrawerList) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You made yourself a note to get rid of this record and just return a list. I agree with that note. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made a comment that DashboardDrawerRecord was redundant as it's already a record. DashboardDrawerList should be a list of DashboardDrawer as that is the collection type of this record class. DashboardDrawer is just a record. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; | ||
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; | ||
import org.springframework.stereotype.Repository; | ||
|
||
import java.util.*; | ||
|
||
@Repository | ||
public class DashboardDrawerRepository { | ||
|
||
private final NamedParameterJdbcTemplate template; | ||
|
||
private static final Logger log = LoggerFactory.getLogger(DashboardDrawerRepository.class); | ||
|
||
@Autowired | ||
public DashboardDrawerRepository(NamedParameterJdbcTemplate template) { | ||
this.template = template; | ||
} | ||
|
||
public List<DashboardDrawer> getDashboardDrawerRows() { | ||
String materializedViewSql = """ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Materialized views are pretty powerful tools. I am not in love with how Postgres handles them as you have to build logic to use them unlike Oracles Query Rewrite abilities. https://docs.oracle.com/cd/B10501_01/server.920/a96520/qr.htm This query is not expensive so I am not worried about needing this unless we scale number of studies and retrieved metadata. Can remove materialized view logic if needed. Would make the query much more dynamic / possibly stored procedure. Aggregation gets expensive. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's talk about this in OH. I clearly don't use views often enough and I want to learn more about them from you. |
||
select * from dictionary_db.dict.dataset_meta_materialized_view dmmv; | ||
"""; | ||
|
||
String fallbackSql = """ | ||
SELECT d.dataset_id, | ||
MAX(d.full_name) study_fullname, | ||
MAX(d.abbreviation) study_abbreviation, | ||
ARRAY_AGG(DISTINCT c.description) consent_groups, | ||
MAX(d.description) study_summary, | ||
ARRAY_AGG(DISTINCT dm.value) FILTER (where dm.key IN ('study_focus')) study_focus, | ||
MAX(DISTINCT dm.value) FILTER (where dm.key IN ('study_design')) study_design, | ||
MAX(DISTINCT dm.value) FILTER (where dm.key IN ('sponsor')) sponsor | ||
FROM dataset d | ||
JOIN dataset_meta dm ON d.dataset_id = dm.dataset_id | ||
JOIN consent c ON d.dataset_id = c.dataset_id | ||
GROUP BY d.dataset_id | ||
"""; | ||
|
||
try { | ||
return template.query(materializedViewSql, new DashboardDrawerRowMapper()); | ||
} catch (Exception e) { | ||
log.debug("Materialized view not available, using fallback query. Error: {}", e.getMessage()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why can't we make it so this view always exists? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://www.postgresql.org/docs/current/rules-materializedviews.html Materialized views are a bit different then views. As in they are stored on disk ( in memory on some DBMS ). They are basically a point in time view of a table. But they need to be managed and refreshed. Have not use Postgres in my checkered past but they were a Saint when it came to Oracle. Oracle has query rewrite which makes it much easier to manage. From what I see so far in Postgres we will need to write logic blocks like this or queries will fail if Materialized view is being rewritten. Query rewrite would fall back on the select statement used to create the materialized view if the view doesn't exist or is being refreshed. |
||
return template.query(fallbackSql, new DashboardDrawerRowMapper()); | ||
} | ||
} | ||
|
||
public Optional<DashboardDrawer> getDashboardDrawerRows(Integer datasetId) { | ||
String materializedViewSql = """ | ||
select * from dictionary_db.dict.dataset_meta_materialized_view dmmv where dmmv.dataset_id = :datasetId; | ||
"""; | ||
|
||
String fallbackSql = """ | ||
SELECT d.dataset_id dataset_id, | ||
MAX(d.full_name) study_fullname, | ||
MAX(d.abbreviation) study_abbreviation, | ||
ARRAY_AGG(DISTINCT c.description) consent_groups, | ||
MAX(d.description) study_summary, | ||
ARRAY_AGG(DISTINCT dm.value) FILTER (where dm.key IN ('study_focus')) study_focus, | ||
MAX(DISTINCT dm.value) FILTER (where dm.key IN ('study_design')) study_design, | ||
MAX(DISTINCT dm.value) FILTER (where dm.key IN ('sponsor')) sponsor | ||
FROM dataset d | ||
JOIN dataset_meta dm ON d.dataset_id = dm.dataset_id | ||
JOIN consent c ON d.dataset_id = c.dataset_id | ||
where d.dataset_id = :datasetId | ||
GROUP BY d.dataset_id | ||
"""; | ||
MapSqlParameterSource params = new MapSqlParameterSource(); | ||
params.addValue("datasetId", datasetId); | ||
|
||
try { | ||
return template.query(materializedViewSql, params, new DashboardDrawerRowMapper()).stream().findFirst(); | ||
} catch (Exception e) { | ||
log.debug("Materialized view not available, using fallback query. Error: {}", e.getMessage()); | ||
return template.query(fallbackSql, params, new DashboardDrawerRowMapper()).stream().findFirst(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer; | ||
|
||
import org.springframework.jdbc.core.RowMapper; | ||
|
||
import java.sql.ResultSet; | ||
import java.sql.SQLException; | ||
import java.sql.Array; // For handling SQL Array | ||
import java.util.Arrays; | ||
import java.util.List; | ||
|
||
public class DashboardDrawerRowMapper implements RowMapper<DashboardDrawer> { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would at least make an abstract class for this that I could extend to fulfill multiple Dashboard Layouts. Should probably do this for other Classes such as Dashboard. |
||
@Override | ||
public DashboardDrawer mapRow(ResultSet rs, int rowNum) throws SQLException { | ||
return new DashboardDrawer( | ||
rs.getInt("dataset_id"), rs.getString("study_fullname"), rs.getString("study_abbreviation"), | ||
convertSqlArrayToList(rs.getArray("consent_groups")), rs.getString("study_summary"), | ||
convertSqlArrayToList(rs.getArray("study_focus")), rs.getString("study_design"), rs.getString("sponsor") | ||
); | ||
} | ||
|
||
private List<String> convertSqlArrayToList(Array sqlArray) throws SQLException { | ||
if (sqlArray == null) { | ||
return List.of(); | ||
} else { | ||
Object[] arrayContents = (Object[]) sqlArray.getArray(); | ||
// Check if the array contains a single empty value | ||
if (arrayContents.length == 1 && "".equals(arrayContents[0])) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit hacky but sqlArray returns an single empty value for empty values in the db. Which makes sense as empty values can have meaning. For this component they do not. |
||
return List.of(); | ||
} else { | ||
return Arrays.asList((String[]) sqlArray.getArray()); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer; | ||
|
||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.beans.factory.annotation.Value; | ||
import org.springframework.stereotype.Service; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.Optional; | ||
|
||
@Service | ||
public class DashboardDrawerService { | ||
|
||
private final DashboardDrawerRepository repository; | ||
private final String dashboardLayout; | ||
|
||
@Autowired | ||
public DashboardDrawerService(DashboardDrawerRepository repository, @Value("${dashboard.layout.type}") String dashboardLayout) { | ||
this.repository = repository; | ||
this.dashboardLayout = dashboardLayout; | ||
} | ||
|
||
/** | ||
* Retrieves the Dashboard Drawer for all datasets. | ||
* | ||
* @return All Dashboard Instances and their metadata. | ||
*/ | ||
public Optional<DashboardDrawerList> findAll() { | ||
if (dashboardLayout.equalsIgnoreCase("bdc")) { | ||
List<DashboardDrawer> records = repository.getDashboardDrawerRows(); | ||
return Optional.of(new DashboardDrawerList(records)); | ||
} | ||
|
||
return Optional.of(new DashboardDrawerList(new ArrayList<>())); | ||
} | ||
|
||
/** | ||
* Retrieves the Dashboard Drawer for a specific dataset. | ||
* | ||
* | ||
* @param datasetId the ID of the dataset to fetch. | ||
* @return a single Dashboard instance with drawer-specific metadata. | ||
*/ | ||
public Optional<DashboardDrawer> findByDatasetId(Integer datasetId) { | ||
if ("bdc".equalsIgnoreCase(dashboardLayout)) { | ||
return repository.getDashboardDrawerRows(datasetId); | ||
} | ||
return Optional.empty(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use the dataset table's primary key the return object to make downstream processes more efficient