-
Notifications
You must be signed in to change notification settings - Fork 169
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
Bug Fix: Return correct count values for RC/DRC and PC/DPC #2405
Changes from all commits
19c5312
a52afe2
4228211
fa6fecc
a765c82
2fbbccc
ef20dc5
65750d4
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 |
---|---|---|
|
@@ -145,9 +145,9 @@ private void cacheRecordsById(Source source, List<Integer> ids) { | |
PreparedStatementRenderer psr = getPartialPreparedStatementRenderer(source, idsSlice); | ||
Set<Integer> cachedIds = loadCache(source, psr, mapper, jdbcTemplate); | ||
// in this batch, need to save any concepts that were not found when loading cache | ||
idsSlice.removeAll(cachedIds); | ||
if (!idsSlice.isEmpty()) { // store zeros in cache | ||
List<CDMCacheEntity> zeroConcepts = idsSlice.stream().map(id -> { | ||
List<Integer> notFoundIds = new ArrayList<Integer>(CollectionUtils.subtract(idsSlice, cachedIds)); | ||
if (!notFoundIds.isEmpty()) { // store zeros in cache | ||
List<CDMCacheEntity> zeroConcepts = notFoundIds.stream().map(id -> { | ||
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 the bug fix (all the other files are for adding the integration test). The 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. Very good explanation. Thank you for researching this and providing a patch. |
||
CDMCacheEntity ce = new CDMCacheEntity(); | ||
ce.setConceptId(id); | ||
ce.setRecordCount(0L); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
IF OBJECT_ID('@results_schema.achilles_result_concept_count', 'U') IS NULL | ||
CREATE TABLE @results_schema.achilles_result_concept_count ( | ||
concept_id int, | ||
record_count bigint, | ||
descendant_record_count bigint, | ||
person_count bigint, | ||
descendant_person_count bigint | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
package org.ohdsi.webapi.test; | ||
|
||
import com.odysseusinc.arachne.commons.types.DBMSType; | ||
|
||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertTrue; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.HashMap; | ||
import java.util.LinkedHashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
|
||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.ohdsi.circe.helper.ResourceHelper; | ||
import org.ohdsi.sql.SqlRender; | ||
import org.ohdsi.sql.SqlTranslate; | ||
import org.ohdsi.webapi.source.SourceRepository; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.beans.factory.annotation.Value; | ||
import org.springframework.http.ResponseEntity; | ||
|
||
public class CDMResultsServiceIT extends WebApiIT { | ||
private static final String CDM_RESULTS_FILE_PATH = "/database/cdm_results.sql"; | ||
|
||
@Value("${cdmResultsService.endpoint.conceptRecordCount}") | ||
private String conceptRecordCountEndpoint; | ||
|
||
@Autowired | ||
private SourceRepository sourceRepository; | ||
|
||
@Before | ||
public void init() throws Exception { | ||
truncateTable(String.format("%s.%s", "public", "source")); | ||
resetSequence(String.format("%s.%s", "public", "source_sequence")); | ||
sourceRepository.saveAndFlush(getCdmSource()); | ||
prepareCdmSchema(); | ||
prepareResultSchema(); | ||
addCDMResults(); | ||
} | ||
|
||
private void addCDMResults() { | ||
String resultSql = SqlRender.renderSql(ResourceHelper.GetResourceAsString( | ||
CDM_RESULTS_FILE_PATH), | ||
new String[] { "results_schema" }, new String[] { RESULT_SCHEMA_NAME }); | ||
String sql = SqlTranslate.translateSql(resultSql, DBMSType.POSTGRESQL.getOhdsiDB()); | ||
jdbcTemplate.execute(sql); | ||
} | ||
|
||
@Test | ||
public void requestConceptRecordCounts_firstTime_returnsResults() { | ||
|
||
// Arrange | ||
List<Integer> conceptIds = Arrays.asList(1); | ||
Map<String, String> queryParameters = new HashMap<String, String>(); | ||
queryParameters.put("sourceName", SOURCE_KEY); | ||
|
||
List<LinkedHashMap<String, List<Integer>>> list = new ArrayList<>(); | ||
@SuppressWarnings("unchecked") | ||
Class<List<LinkedHashMap<String, List<Integer>>>> returnClass = (Class<List<LinkedHashMap<String, List<Integer>>>>)list.getClass(); | ||
|
||
// Act | ||
final ResponseEntity<List<LinkedHashMap<String, List<Integer>>>> entity = getRestTemplate().postForEntity(this.conceptRecordCountEndpoint, conceptIds, | ||
returnClass, queryParameters ); | ||
|
||
// Assertion | ||
assertOK(entity); | ||
List<LinkedHashMap<String, List<Integer>>> results = entity.getBody(); | ||
assertEquals(1, results.size()); | ||
LinkedHashMap<String, List<Integer>> resultHashMap = results.get(0); | ||
assertEquals(1, resultHashMap.size()); | ||
assertTrue(resultHashMap.containsKey("1")); | ||
List<Integer> counts = resultHashMap.get("1"); | ||
assertEquals(100, counts.get(0).intValue()); | ||
assertEquals(101, counts.get(1).intValue()); | ||
assertEquals(102, counts.get(2).intValue()); | ||
assertEquals(103, counts.get(3).intValue()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
insert into @results_schema.achilles_result_concept_count (concept_id, record_count, descendant_record_count, person_count, descendant_person_count) | ||
values (1,100,101,102,103); |
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.
I needed to add this dependency in order to get the project to build and run in my test runner.
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.
should be fine, the scope is test so won't be part of the final bundle.