Merge pull request #1942 from jamesagnew/im_20200619_mariadb_expand_valueset_fix

Im 20200619 mariadb expand valueset fix
This commit is contained in:
IanMMarshall 2020-06-25 20:05:24 -04:00 committed by GitHub
commit ceeff0ccf0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 388 additions and 14 deletions

View File

@ -38,7 +38,7 @@ import java.io.Serializable;
* Note about the CONCAT function below- We need a primary key (an @Id) column
* because hibernate won't allow the view the function without it, but
*/
"SELECT CONCAT(vsc.PID, CONCAT(' ', vscd.PID)) AS PID, " +
"SELECT CONCAT_WS(' ', vsc.PID, vscd.PID) AS PID, " +
" vsc.PID AS CONCEPT_PID, " +
" vsc.VALUESET_PID AS CONCEPT_VALUESET_PID, " +
" vsc.VALUESET_ORDER AS CONCEPT_VALUESET_ORDER, " +

View File

@ -75,6 +75,10 @@ public class ElasticsearchHibernatePropertiesBuilder {
theProperties.put("hibernate.search.default." + ElasticsearchEnvironment.INDEX_MANAGEMENT_WAIT_TIMEOUT, Long.toString(myIndexManagementWaitTimeoutMillis));
theProperties.put("hibernate.search.default." + ElasticsearchEnvironment.REQUIRED_INDEX_STATUS, myRequiredIndexStatus.getElasticsearchString());
// Need the mapping to be dynamic because of terminology indexes.
theProperties.put("hibernate.search.default.elasticsearch.dynamic_mapping", "true");
// Only for unit tests
theProperties.put("hibernate.search.default." + ElasticsearchEnvironment.REFRESH_AFTER_WRITE, Boolean.toString(myDebugRefreshAfterWrite));
theProperties.put("hibernate.search." + ElasticsearchEnvironment.LOG_JSON_PRETTY_PRINTING, Boolean.toString(myDebugPrettyPrintJsonLog));

View File

@ -51,7 +51,7 @@ public class ElasticsearchMappingProvider implements ElasticsearchAnalysisDefini
builder.analyzer("standardAnalyzer").withTokenizer("standard").withTokenFilters("lowercase");
builder.analyzer("exactAnalyzer").withTokenizer("standard");
builder.analyzer("exactAnalyzer").withTokenizer("keyword");
builder.analyzer("conceptParentPidsAnalyzer").withTokenizer("whitespace");

View File

@ -0,0 +1,157 @@
package ca.uhn.fhir.jpa.dao.r4;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.jpa.api.config.DaoConfig;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoCodeSystem;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoValueSet;
import ca.uhn.fhir.jpa.api.dao.IFhirSystemDao;
import ca.uhn.fhir.jpa.api.svc.ISearchCoordinatorSvc;
import ca.uhn.fhir.jpa.bulk.IBulkDataExportSvc;
import ca.uhn.fhir.jpa.config.TestR4ConfigWithElasticSearch;
import ca.uhn.fhir.jpa.dao.BaseJpaTest;
import ca.uhn.fhir.jpa.dao.data.IResourceTableDao;
import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion;
import ca.uhn.fhir.jpa.entity.TermConcept;
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc;
import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistry;
import ca.uhn.fhir.jpa.term.api.ITermCodeSystemStorageSvc;
import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId;
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
import ca.uhn.fhir.util.TestUtil;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.CodeSystem;
import org.hl7.fhir.r4.model.CodeSystem.CodeSystemContentMode;
import org.hl7.fhir.r4.model.CodeableConcept;
import org.hl7.fhir.r4.model.Coding;
import org.hl7.fhir.r4.model.ValueSet;
import org.hl7.fhir.r4.model.ValueSet.ConceptReferenceComponent;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Answers;
import org.mockito.Mock;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.transaction.PlatformTransactionManager;
import java.util.Set;
import java.util.stream.Collectors;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.Assert.assertThat;
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(classes = {TestR4ConfigWithElasticSearch.class})
public class FhirResourceDaoR4TerminologyElasticsearchIT extends BaseJpaTest {
public static final String URL_MY_CODE_SYSTEM = "http://example.com/my_code_system";
public static final String URL_MY_VALUE_SET = "http://example.com/my_value_set";
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4TerminologyElasticsearchIT.class);
@Autowired
protected DaoConfig myDaoConfig;
@Autowired
@Qualifier("myCodeSystemDaoR4")
protected IFhirResourceDaoCodeSystem<CodeSystem, Coding, CodeableConcept> myCodeSystemDao;
@Autowired
protected IResourceTableDao myResourceTableDao;
@Autowired
protected ITermCodeSystemStorageSvc myTermCodeSystemStorageSvc;
@Autowired
@Qualifier("myValueSetDaoR4")
protected IFhirResourceDaoValueSet<ValueSet, Coding, CodeableConcept> myValueSetDao;
@Autowired
FhirContext myFhirContext;
@Autowired
PlatformTransactionManager myTxManager;
@Autowired
private IFhirSystemDao mySystemDao;
@Autowired
private IResourceReindexingSvc myResourceReindexingSvc;
@Autowired
private ISearchCoordinatorSvc mySearchCoordinatorSvc;
@Autowired
private ISearchParamRegistry mySearchParamRegistry;
@Autowired
private IBulkDataExportSvc myBulkDataExportSvc;
@Mock(answer = Answers.RETURNS_DEEP_STUBS)
protected ServletRequestDetails mySrd;
@Test
public void testExpandWithIncludeContainingDashesInInclude() {
CodeSystem codeSystem = new CodeSystem();
codeSystem.setUrl(URL_MY_CODE_SYSTEM);
codeSystem.setContent(CodeSystemContentMode.NOTPRESENT);
IIdType id = myCodeSystemDao.create(codeSystem, mySrd).getId().toUnqualified();
ResourceTable table = myResourceTableDao.findById(id.getIdPartAsLong()).orElseThrow(IllegalStateException::new);
TermCodeSystemVersion cs = new TermCodeSystemVersion();
cs.setResource(table);
TermConcept concept;
concept = new TermConcept(cs, "LA1111-2");
cs.getConcepts().add(concept);
concept = new TermConcept(cs, "LA2222-2");
cs.getConcepts().add(concept);
concept = new TermConcept(cs, "LA3333-2");
cs.getConcepts().add(concept);
concept = new TermConcept(cs, "LA1122-2");
cs.getConcepts().add(concept);
concept = new TermConcept(cs, "LA1133-2");
cs.getConcepts().add(concept);
concept = new TermConcept(cs, "LA4444-2");
cs.getConcepts().add(concept);
concept = new TermConcept(cs, "LA9999-7");
cs.getConcepts().add(concept);
myTermCodeSystemStorageSvc.storeNewCodeSystemVersion(new ResourcePersistentId(table.getId()), URL_MY_CODE_SYSTEM, "SYSTEM NAME", "SYSTEM VERSION" , cs, table);
ValueSet valueSet = new ValueSet();
valueSet.setUrl(URL_MY_VALUE_SET);
valueSet.getCompose()
.addInclude()
.setSystem(codeSystem.getUrl())
.addConcept(new ConceptReferenceComponent().setCode("LA2222-2"))
.addConcept(new ConceptReferenceComponent().setCode("LA1122-2"));
IIdType vsId = myValueSetDao.create(valueSet, mySrd).getId().toUnqualifiedVersionless();
ValueSet expansion = myValueSetDao.expand(vsId, null, null);
Set<String> codes = expansion
.getExpansion()
.getContains()
.stream()
.map(ValueSet.ValueSetExpansionContainsComponent::getCode)
.collect(Collectors.toSet());
ourLog.info("Codes: {}", codes);
assertThat(codes, containsInAnyOrder("LA2222-2", "LA1122-2"));
}
@Override
protected FhirContext getContext() {
return myFhirContext;
}
@Override
protected PlatformTransactionManager getTxManager() {
return myTxManager;
}
@After
public void afterPurgeDatabase() {
purgeDatabase(myDaoConfig, mySystemDao, myResourceReindexingSvc, mySearchCoordinatorSvc, mySearchParamRegistry, myBulkDataExportSvc);
}
@AfterClass
public static void afterClassClearContext() {
TestUtil.clearAllStaticFieldsForUnitTest();
}
}

View File

@ -0,0 +1,216 @@
package ca.uhn.fhir.jpa.term;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.jpa.api.config.DaoConfig;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoCodeSystem;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoValueSet;
import ca.uhn.fhir.jpa.api.dao.IFhirSystemDao;
import ca.uhn.fhir.jpa.api.svc.ISearchCoordinatorSvc;
import ca.uhn.fhir.jpa.bulk.IBulkDataExportSvc;
import ca.uhn.fhir.jpa.config.TestR4ConfigWithElasticSearch;
import ca.uhn.fhir.jpa.dao.BaseJpaTest;
import ca.uhn.fhir.jpa.dao.data.IResourceTableDao;
import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion;
import ca.uhn.fhir.jpa.entity.TermConcept;
import ca.uhn.fhir.jpa.entity.TermConceptParentChildLink;
import ca.uhn.fhir.jpa.model.entity.ResourceTable;
import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc;
import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistry;
import ca.uhn.fhir.jpa.term.api.ITermCodeSystemStorageSvc;
import ca.uhn.fhir.jpa.term.api.ITermDeferredStorageSvc;
import ca.uhn.fhir.jpa.term.api.ITermReadSvcR4;
import ca.uhn.fhir.jpa.term.custom.CustomTerminologySet;
import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.CodeSystem;
import org.hl7.fhir.r4.model.CodeableConcept;
import org.hl7.fhir.r4.model.Coding;
import org.hl7.fhir.r4.model.ValueSet;
import org.junit.After;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Answers;
import org.mockito.Mock;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.transaction.PlatformTransactionManager;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.anyCollection;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(classes = {TestR4ConfigWithElasticSearch.class})
public class ValueSetExpansionR4ElasticsearchIT extends BaseJpaTest {
@Autowired
protected DaoConfig myDaoConfig;
@Autowired
@Qualifier("myCodeSystemDaoR4")
protected IFhirResourceDaoCodeSystem<CodeSystem, Coding, CodeableConcept> myCodeSystemDao;
@Autowired
protected IResourceTableDao myResourceTableDao;
@Autowired
protected ITermCodeSystemStorageSvc myTermCodeSystemStorageSvc;
@Autowired
@Qualifier("myValueSetDaoR4")
protected IFhirResourceDaoValueSet<ValueSet, Coding, CodeableConcept> myValueSetDao;
@Autowired
protected ITermReadSvcR4 myTermSvc;
@Autowired
protected ITermDeferredStorageSvc myTerminologyDeferredStorageSvc;
@Autowired
FhirContext myFhirContext;
@Autowired
PlatformTransactionManager myTxManager;
@Autowired
private IFhirSystemDao mySystemDao;
@Autowired
private IResourceReindexingSvc myResourceReindexingSvc;
@Autowired
private ISearchCoordinatorSvc mySearchCoordinatorSvc;
@Autowired
private ISearchParamRegistry mySearchParamRegistry;
@Autowired
private IBulkDataExportSvc myBulkDataExportSvc;
@Mock
private IValueSetConceptAccumulator myValueSetCodeAccumulator;
@Mock(answer = Answers.RETURNS_DEEP_STUBS)
protected ServletRequestDetails mySrd;
protected static final String CS_URL = "http://example.com/my_code_system";
@After
public void after() {
myDaoConfig.setMaximumExpansionSize(DaoConfig.DEFAULT_MAX_EXPANSION_SIZE);
}
@After
public void afterPurgeDatabase() {
purgeDatabase(myDaoConfig, mySystemDao, myResourceReindexingSvc, mySearchCoordinatorSvc, mySearchParamRegistry, myBulkDataExportSvc);
}
void createCodeSystem() {
CodeSystem codeSystem = new CodeSystem();
codeSystem.setUrl(CS_URL);
codeSystem.setContent(CodeSystem.CodeSystemContentMode.NOTPRESENT);
codeSystem.setName("SYSTEM NAME");
codeSystem.setVersion("SYSTEM VERSION");
IIdType id = myCodeSystemDao.create(codeSystem, mySrd).getId().toUnqualified();
ResourceTable table = myResourceTableDao.findById(id.getIdPartAsLong()).orElseThrow(IllegalArgumentException::new);
TermCodeSystemVersion cs = new TermCodeSystemVersion();
cs.setResource(table);
TermConcept parent;
parent = new TermConcept(cs, "ParentWithNoChildrenA");
cs.getConcepts().add(parent);
parent = new TermConcept(cs, "ParentWithNoChildrenB");
cs.getConcepts().add(parent);
parent = new TermConcept(cs, "ParentWithNoChildrenC");
cs.getConcepts().add(parent);
TermConcept parentA = new TermConcept(cs, "ParentA");
cs.getConcepts().add(parentA);
TermConcept childAA = new TermConcept(cs, "childAA");
parentA.addChild(childAA, TermConceptParentChildLink.RelationshipTypeEnum.ISA);
TermConcept childAAA = new TermConcept(cs, "childAAA");
childAAA.addPropertyString("propA", "valueAAA");
childAAA.addPropertyString("propB", "foo");
childAA.addChild(childAAA, TermConceptParentChildLink.RelationshipTypeEnum.ISA);
TermConcept childAAB = new TermConcept(cs, "childAAB");
childAAB.addPropertyString("propA", "valueAAB");
childAAB.addPropertyString("propB", "foo");
childAAB.addDesignation()
.setUseSystem("D1S")
.setUseCode("D1C")
.setUseDisplay("D1D")
.setValue("D1V");
childAA.addChild(childAAB, TermConceptParentChildLink.RelationshipTypeEnum.ISA);
TermConcept childAB = new TermConcept(cs, "childAB");
parentA.addChild(childAB, TermConceptParentChildLink.RelationshipTypeEnum.ISA);
TermConcept parentB = new TermConcept(cs, "ParentB");
cs.getConcepts().add(parentB);
myTermCodeSystemStorageSvc.storeNewCodeSystemVersion(new ResourcePersistentId(table.getId()), CS_URL, "SYSTEM NAME", "SYSTEM VERSION", cs, table);
}
@Test
public void testExpandValueSetInMemoryRespectsMaxSize() {
createCodeSystem();
// Add lots more codes
CustomTerminologySet additions = new CustomTerminologySet();
for (int i = 0; i < 100; i++) {
additions.addRootConcept("CODE" + i, "Display " + i);
}
myTermCodeSystemStorageSvc.applyDeltaCodeSystemsAdd(CS_URL, additions);
// Codes available exceeds the max
myDaoConfig.setMaximumExpansionSize(50);
ValueSet vs = new ValueSet();
ValueSet.ConceptSetComponent include = vs.getCompose().addInclude();
include.setSystem(CS_URL);
try {
myTermSvc.expandValueSet(null, vs);
fail();
} catch (InternalErrorException e) {
assertEquals("Expansion of ValueSet produced too many codes (maximum 50) - Operation aborted!", e.getMessage());
}
// Increase the max so it won't exceed
myDaoConfig.setMaximumExpansionSize(150);
vs = new ValueSet();
include = vs.getCompose().addInclude();
include.setSystem(CS_URL);
ValueSet outcome = myTermSvc.expandValueSet(null, vs);
assertEquals(109, outcome.getExpansion().getContains().size());
}
@Test
public void testExpandValueSetWithValueSetCodeAccumulator() {
createCodeSystem();
when(myValueSetCodeAccumulator.getCapacityRemaining()).thenReturn(100);
ValueSet vs = new ValueSet();
ValueSet.ConceptSetComponent include = vs.getCompose().addInclude();
include.setSystem(CS_URL);
myTermSvc.expandValueSet(null, vs, myValueSetCodeAccumulator);
verify(myValueSetCodeAccumulator, times(9)).includeConceptWithDesignations(anyString(), anyString(), nullable(String.class), anyCollection());
}
@Override
protected FhirContext getContext() {
return myFhirContext;
}
@Override
protected PlatformTransactionManager getTxManager() {
return myTxManager;
}
}

View File

@ -54,11 +54,11 @@ public class AddColumnTask extends BaseTableColumnTypeTask {
String sql;
switch (getDriverType()) {
case MYSQL_5_7:
case MARIADB_10_1:
// Quote the column name as "SYSTEM" is a reserved word in MySQL
sql = "alter table " + getTableName() + " add column `" + getColumnName() + "` " + typeStatement;
break;
case DERBY_EMBEDDED:
case MARIADB_10_1:
case POSTGRES_9_4:
sql = "alter table " + getTableName() + " add column " + getColumnName() + " " + typeStatement;
break;

View File

@ -26,6 +26,7 @@ import org.intellij.lang.annotations.Language;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.sql.Driver;
import java.sql.SQLException;
import java.util.List;
import java.util.Set;
@ -52,8 +53,8 @@ public class DropColumnTask extends BaseTableColumnTask {
return;
}
if(getDriverType().equals(DriverTypeEnum.MYSQL_5_7)) {
// Some DBs such as MYSQL require that foreign keys depending on the column be dropped before the column itself is dropped.
if(getDriverType().equals(DriverTypeEnum.MYSQL_5_7) || getDriverType().equals(DriverTypeEnum.MARIADB_10_1)) {
// Some DBs such as MYSQL and Maria DB require that foreign keys depending on the column be dropped before the column itself is dropped.
logInfo(ourLog, "Dropping any foreign keys on table {} depending on column {}", getTableName(), getColumnName());
Set<String> foreignKeys = JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), getColumnName(), getTableName());
if(foreignKeys != null) {

View File

@ -101,10 +101,10 @@ public class DropForeignKeyTask extends BaseTableTask {
List<String> sqls = new ArrayList<>();
switch (theDriverType) {
case MYSQL_5_7:
case MARIADB_10_1:
// Lousy MYQL....
sqls.add("alter table " + theTableName + " drop foreign key " + theConstraintName);
break;
case MARIADB_10_1:
case POSTGRES_9_4:
case DERBY_EMBEDDED:
case H2_EMBEDDED:

View File

@ -107,12 +107,10 @@ public class DropIndexTask extends BaseTableTask {
// Drop constraint
switch (theDriverType) {
case MYSQL_5_7:
case MARIADB_10_1:
// Need to quote the index name as the word "PRIMARY" is reserved in MySQL
sql.add("alter table " + theTableName + " drop index `" + theIndexName + "`");
break;
case MARIADB_10_1:
sql.add("alter table " + theTableName + " drop index " + theIndexName);
break;
case H2_EMBEDDED:
sql.add("drop index " + theIndexName);
break;

View File

@ -139,10 +139,8 @@ public class RenameColumnTask extends BaseTableTask {
case DERBY_EMBEDDED:
sql = "RENAME COLUMN " + getTableName() + "." + myOldName + " TO " + myNewName;
break;
case MARIADB_10_1:
sql = "ALTER TABLE " + getTableName() + " CHANGE COLUMN " + myOldName + " TO " + myNewName;
break;
case MYSQL_5_7:
case MARIADB_10_1:
// Quote the column names as "SYSTEM" is a reserved word in MySQL
sql = "ALTER TABLE " + getTableName() + " CHANGE COLUMN `" + myOldName + "` `" + myNewName + "` " + theExistingType + " " + theExistingNotNull;
break;

View File

@ -110,10 +110,10 @@ public class RenameIndexTask extends BaseTableTask {
// Drop constraint
switch (theDriverType) {
case MYSQL_5_7:
case MARIADB_10_1:
// Quote the index names as "PRIMARY" is a reserved word in MySQL
sql.add("rename index `" + theOldIndexName + "` to `" + theNewIndexName + "`");
break;
case MARIADB_10_1:
case DERBY_EMBEDDED:
sql.add("rename index " + theOldIndexName + " to " + theNewIndexName);
break;

View File

@ -27,7 +27,7 @@ public class RenameColumnTaskDbSpecificTest {
@Test
public void testBuildSqlStatementForMariaDB() {
assertEquals("ALTER TABLE SOMETABLE CHANGE COLUMN myTextCol TO TEXTCOL", createRenameColumnSql(DriverTypeEnum.MARIADB_10_1));
assertEquals("ALTER TABLE SOMETABLE CHANGE COLUMN `myTextCol` `TEXTCOL` integer null", createRenameColumnSql(DriverTypeEnum.MARIADB_10_1));
}
@Test