From f94fb7fecc1baa701447481d7bc5878ec6749f6c Mon Sep 17 00:00:00 2001 From: Paul Bastide Date: Fri, 5 Nov 2021 16:07:25 -0400 Subject: [PATCH] Avoid creating unnecessary tables for Resource and DomainResource resource types #713 - Checks for DOMAINRESOURCE and RESOURCE tables view - Removes the artifacts - Bonus: added fix for null condition on a code in DefaultMemberMatchStrategy Signed-off-by: Paul Bastide --- .../java/com/ibm/fhir/schema/app/Main.java | 1 + .../MigrateV0021AbstractTypeRemoval.java | 68 +++++++++++++------ ...UnusedTableRemovalNeedsV0021Migration.java | 15 ++-- .../schema/derby/DerbyFhirDatabaseTest.java | 14 ++-- .../schema/derby/DerbySchemaVersionsTest.java | 12 ++-- .../strategy/DefaultMemberMatchStrategy.java | 3 +- 6 files changed, 72 insertions(+), 41 deletions(-) diff --git a/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/app/Main.java b/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/app/Main.java index ce03d931753..132b49f6d3c 100644 --- a/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/app/Main.java +++ b/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/app/Main.java @@ -2561,6 +2561,7 @@ private void applyTableRemovalForV0021() { logger.info("V0021 Migration: Removing Abstract Tables from schema '" + schemaName + "'"); MigrateV0021AbstractTypeRemoval cmd = new MigrateV0021AbstractTypeRemoval(adapter, adminSchemaName, schemaName, forceUnusedTableRemoval); adapter.runStatement(cmd); + logger.info("V0021 Migration: Completed the Removal of the Abstract Tables from schema '" + schemaName + "'"); } } catch (DataAccessException x) { // Something went wrong, so mark the transaction as failed diff --git a/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/control/MigrateV0021AbstractTypeRemoval.java b/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/control/MigrateV0021AbstractTypeRemoval.java index 5000c8deda9..21ca78dae5c 100644 --- a/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/control/MigrateV0021AbstractTypeRemoval.java +++ b/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/control/MigrateV0021AbstractTypeRemoval.java @@ -14,6 +14,7 @@ import java.util.Arrays; import java.util.List; import java.util.logging.Logger; +import java.util.stream.Collectors; import com.ibm.fhir.database.utils.api.DataAccessException; import com.ibm.fhir.database.utils.api.IDatabaseAdapter; @@ -34,6 +35,19 @@ public class MigrateV0021AbstractTypeRemoval implements IDatabaseStatement { private static final Logger LOG = Logger.getLogger(MigrateV0021AbstractTypeRemoval.class.getName()); + private static final List VALUE_TYPES = Arrays.asList( + "RESOURCE_TOKEN_REFS", + "DATE_VALUES", + "LATLNG_VALUES", + "NUMBER_VALUES", + "QUANTITY_VALUES", + "STR_VALUES", + "PROFILES", + "TAGS", + "SECURITY"); + + private static final List VALUE_TYPES_LOWER = VALUE_TYPES.stream().map(s -> s.toLowerCase()).collect(Collectors.toList()); + // The Adapter private final IDatabaseAdapter adapter; @@ -171,30 +185,16 @@ private void checkDataTables(IDatabaseTranslator translator, Connection c) { */ private void removeBaseArtifacts(IDatabaseTranslator translator, Connection c) { List tables = Arrays.asList("DOMAINRESOURCE_", "RESOURCE_"); - List valueTypes = Arrays.asList( - "DATE_VALUES", - "LATLNG_VALUES", - "NUMBER_VALUES", - "QUANTITY_VALUES", - "RESOURCE_TOKEN_REFS", - "STR_VALUES", - "PROFILES", - "TAGS", - "SECURITY"); + // Run across both tables for (String tablePrefix : tables) { - // Drop the Values Tables - for (String valueType : valueTypes) { - adapter.dropTable(schemaName, tablePrefix + valueType); - } - - // Drop the supporting tables - adapter.dropTable(schemaName, tablePrefix + "logical_resources"); - adapter.dropTable(schemaName, tablePrefix + "resources"); - // Drop the View for the Table - adapter.dropView(schemaName, tablePrefix + "_token_values_V"); + if (translator.getType() == DbType.POSTGRESQL) { + runDropTableResourceGroup(translator, c, schemaName.toLowerCase(), tablePrefix.toLowerCase(), VALUE_TYPES_LOWER); + } else { + runDropTableResourceGroup(translator, c, schemaName, tablePrefix, VALUE_TYPES); + } } // Pattern for multitenant is to prefix tables with DRP_ @@ -206,8 +206,34 @@ private void removeBaseArtifacts(IDatabaseTranslator translator, Connection c) { // Drop the tables, when the tables don't exists the errors are swallowed // and logs print warnings saying the tables don't exist. That's OK. for (String deprecatedTable : UnusedTableRemovalNeedsV0021Migration.DEPRECATED_TABLES) { - adapter.dropTable(schemaName, prefix + deprecatedTable); + String table = prefix + deprecatedTable; + if (translator.getType() == DbType.POSTGRESQL) { + adapter.dropTable(schemaName.toLowerCase(), table.toLowerCase()); + } else { + adapter.dropTable(schemaName, table); + } + } + } + + /** + * run drop table resource group + * @param translator + * @param c + * @param schemaName + * @param tablePrefix + * @param valueTypes + */ + public void runDropTableResourceGroup(IDatabaseTranslator translator, Connection c, String schemaName, String tablePrefix, List valueTypes) { + adapter.dropView(schemaName, tablePrefix + "token_values_v"); + + // Drop the Values Tables + for (String valueType : valueTypes) { + adapter.dropTable(schemaName, tablePrefix + valueType); } + + // Drop the supporting tables + adapter.dropTable(schemaName, tablePrefix + "logical_resources"); + adapter.dropTable(schemaName, tablePrefix + "resources"); } /** diff --git a/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/control/UnusedTableRemovalNeedsV0021Migration.java b/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/control/UnusedTableRemovalNeedsV0021Migration.java index 9ee8b9d6fae..6d1fd60faaf 100644 --- a/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/control/UnusedTableRemovalNeedsV0021Migration.java +++ b/fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/control/UnusedTableRemovalNeedsV0021Migration.java @@ -15,6 +15,7 @@ import com.ibm.fhir.database.utils.api.IDatabaseSupplier; import com.ibm.fhir.database.utils.api.IDatabaseTranslator; +import com.ibm.fhir.database.utils.model.DbType; /** * Checks to see if any of the tables exist in the target database. @@ -87,7 +88,7 @@ public Boolean run(IDatabaseTranslator translator, Connection c) { * @return */ public boolean checkDb2(IDatabaseTranslator translator, Connection c) { - final String sql = "SELECT tabname FROM SYSCAT.TABLES TABS" + final String sql = "SELECT 1 FROM SYSCAT.TABLES TABS" + " WHERE TABS.tabschema = ? " + " AND TABS.tabname in ( " + addParameterMarkers(TABLE_COUNT) + " )"; return hasTables(translator, c, sql); @@ -103,7 +104,7 @@ public boolean checkDb2(IDatabaseTranslator translator, Connection c) { public boolean checkDerby(IDatabaseTranslator translator, Connection c) { // Grab the list of tables for the configured schema from the Derby sys catalog final String sql = "" - + "SELECT tables.tablename FROM sys.systables AS tables " + + "SELECT 1 FROM sys.systables AS tables " + " JOIN sys.sysschemas AS schemas " + " ON (tables.schemaid = schemas.schemaid) " + " WHERE schemas.schemaname = ?" @@ -122,8 +123,8 @@ public boolean checkPostgres(IDatabaseTranslator translator, Connection c) { // Grab the list of tables for the configured schema from the PostgreSQL schema // catalog final String sql = "" - + "SELECT table_name FROM information_schema.tables " - + " WHERE table_schema = ?" + + "SELECT 1 FROM information_schema.tables " + + " WHERE table_schema = lower(?)" + " AND table_name in (" + addParameterMarkers(TABLE_COUNT) + ")"; return hasTables(translator, c, sql); } @@ -157,7 +158,11 @@ private boolean hasTables(IDatabaseTranslator translator, Connection c, final St ps.setString(i++, schemaName); for (String deprecatedTable : DEPRECATED_TABLES) { - ps.setString(i++, deprecatedTable); + if (translator.getType() == DbType.POSTGRESQL) { + ps.setString(i++, deprecatedTable.toLowerCase()); + } else { + ps.setString(i++, deprecatedTable); + } } ResultSet rs = ps.executeQuery(); return rs.next(); diff --git a/fhir-persistence-schema/src/test/java/com/ibm/fhir/schema/derby/DerbyFhirDatabaseTest.java b/fhir-persistence-schema/src/test/java/com/ibm/fhir/schema/derby/DerbyFhirDatabaseTest.java index 5d6449c05c0..6032398d173 100644 --- a/fhir-persistence-schema/src/test/java/com/ibm/fhir/schema/derby/DerbyFhirDatabaseTest.java +++ b/fhir-persistence-schema/src/test/java/com/ibm/fhir/schema/derby/DerbyFhirDatabaseTest.java @@ -9,7 +9,6 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; -import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; import java.sql.Connection; @@ -31,7 +30,6 @@ import com.ibm.fhir.database.utils.model.PhysicalDataModel; import com.ibm.fhir.database.utils.pool.PoolConnectionProvider; import com.ibm.fhir.database.utils.transaction.SimpleTransactionProvider; -import com.ibm.fhir.database.utils.version.CreateVersionHistory; import com.ibm.fhir.database.utils.version.CreateWholeSchemaVersion; import com.ibm.fhir.database.utils.version.VersionHistoryService; import com.ibm.fhir.schema.app.Main; @@ -62,7 +60,7 @@ public void testFhirSchema() throws Exception { try (DerbyFhirDatabase db = new DerbyFhirDatabase(DB_NAME)) { System.out.println("FHIR database exists."); checkDatabase(db, db.getSchemaName()); - + // Test the schema drop testDrop(db, db.getSchemaName()); } @@ -87,9 +85,9 @@ protected void testDrop(IConnectionProvider cp, String schemaName) throws SQLExc FhirSchemaGenerator gen = new FhirSchemaGenerator(ADMIN_SCHEMA_NAME, schemaName, false); gen.buildSchema(pdm); pdm.drop(adapter, FhirSchemaGenerator.SCHEMA_GROUP_TAG, FhirSchemaGenerator.FHIRDATA_GROUP); - + CreateWholeSchemaVersion.dropTable(schemaName, adapter); - + // Check that the schema is empty List schemaObjects = adapter.listSchemaObjects(schemaName); boolean schemaIsEmpty = schemaObjects.isEmpty(); @@ -100,7 +98,7 @@ protected void testDrop(IConnectionProvider cp, String schemaName) throws SQLExc } assertTrue(schemaIsEmpty); } - + // Now we're all done we can finally clean up the version-history for // our schema. Make sure we don't get a version for an object we know // we created @@ -137,10 +135,10 @@ protected void checkDatabase(IConnectionProvider cp, String schemaName) throws S JdbcTarget tgt = new JdbcTarget(c); DerbyAdapter adapter = new DerbyAdapter(tgt); checkRefSequence(adapter); - + // Check that we have the correct number of tables. This will need to be updated // whenever tables, views or sequences are added or removed - assertEquals(adapter.listSchemaObjects(schemaName).size(), 1943); + assertEquals(adapter.listSchemaObjects(schemaName).size(), 1917); c.commit(); } catch (Throwable t) { c.rollback(); diff --git a/fhir-persistence-schema/src/test/java/com/ibm/fhir/schema/derby/DerbySchemaVersionsTest.java b/fhir-persistence-schema/src/test/java/com/ibm/fhir/schema/derby/DerbySchemaVersionsTest.java index abd06e68b5f..3a01f97d637 100644 --- a/fhir-persistence-schema/src/test/java/com/ibm/fhir/schema/derby/DerbySchemaVersionsTest.java +++ b/fhir-persistence-schema/src/test/java/com/ibm/fhir/schema/derby/DerbySchemaVersionsTest.java @@ -3,7 +3,7 @@ * * SPDX-License-Identifier: Apache-2.0 */ - + package com.ibm.fhir.schema.derby; import static org.testng.Assert.assertEquals; @@ -27,7 +27,7 @@ public class DerbySchemaVersionsTest { private static final String TARGET_DIR = "target/derby/"; private static final String SCHEMA_NAME = "APP"; - + @Test public void test() throws Exception { String dbPath = TARGET_DIR + "fhirdb"; @@ -41,18 +41,18 @@ public void test() throws Exception { PoolConnectionProvider connectionPool = new PoolConnectionProvider(cp, 10); ITransactionProvider transactionProvider = new SimpleTransactionProvider(connectionPool); SchemaVersionsManager svm = new SchemaVersionsManager(db.getTranslator(), connectionPool, transactionProvider, SCHEMA_NAME); - + svm.updateSchemaVersionId(FhirSchemaVersion.V0001); assertEquals(svm.getVersionForSchema(), FhirSchemaVersion.V0001.vid()); svm.updateSchemaVersionId(FhirSchemaVersion.V0002); assertEquals(svm.getVersionForSchema(), FhirSchemaVersion.V0002.vid()); svm.updateSchemaVersionId(FhirSchemaVersion.V0003); assertEquals(svm.getVersionForSchema(), FhirSchemaVersion.V0003.vid()); - + // Make sure we can correctly determine the latest schema version value svm.updateSchemaVersion(); - assertEquals(svm.getVersionForSchema(), FhirSchemaVersion.V0020.vid()); - + assertEquals(svm.getVersionForSchema(), FhirSchemaVersion.V0021.vid()); + assertTrue(svm.isLatestSchema()); } } diff --git a/operation/fhir-operation-member-match/src/main/java/com/ibm/fhir/operation/davinci/hrex/provider/strategy/DefaultMemberMatchStrategy.java b/operation/fhir-operation-member-match/src/main/java/com/ibm/fhir/operation/davinci/hrex/provider/strategy/DefaultMemberMatchStrategy.java index 44a3fd08002..023409647c7 100644 --- a/operation/fhir-operation-member-match/src/main/java/com/ibm/fhir/operation/davinci/hrex/provider/strategy/DefaultMemberMatchStrategy.java +++ b/operation/fhir-operation-member-match/src/main/java/com/ibm/fhir/operation/davinci/hrex/provider/strategy/DefaultMemberMatchStrategy.java @@ -255,8 +255,9 @@ private void addIdValue(Identifier identifier) { // If there is more than one Coding it has to be MB. for (Coding coding : identifier.getType().getCoding()) { if (coding.getSystem() != null - && "http://terminology.hl7.org/CodeSystem/v2-0203" + && "http://terminology.hl7.org/CodeSystem/v2-0203" .equals(coding.getSystem().getValue()) + && coding.getCode() != null && "MB".equals(coding.getCode().getValue())) { // We only want to extract the code system v2-0203 with MB Uri sys = identifier.getSystem();