From f518f30506f75655c80286c795ddb9644b51cbe3 Mon Sep 17 00:00:00 2001 From: jmarchionatto <60409882+jmarchionatto@users.noreply.github.com> Date: Fri, 18 Mar 2022 15:09:25 -0400 Subject: [PATCH] Issue 3450 there is no way to recreate freetext indexes for terminology entities (#3481) * Add reindex-terminology batch command * Handle number of thread throttling and concurrency with other terminology batch operations * Add required dbcp2 dependency * Fix test * Improve ConnectionPoolInfoProvider setup. Handle maximum connections. * Remove java higher version construction * Remove unused config * Add reindex terminology integration test. Reset termConcept counters before pre-expanding, which otherwise accumulate if it was pre-expanded before. * Address MR comments * Adjust test to tested class change * Adjust test to tested class change Co-authored-by: juan.marchionatto --- .../ca/uhn/fhir/cli/ReindexTerminologyCommand.java | 2 +- .../fhir/cli/ReindexTerminologyCommandTest.java | 1 - ...-recreate-freetext-indexes-for-terminology.yaml | 6 +++--- .../config/util/ConnectionPoolInfoProvider.java | 14 +++++++------- .../fhir/jpa/provider/BaseJpaSystemProvider.java | 6 ++++-- .../util/ConnectionPoolInfoProviderTest.java | 8 ++++---- .../TerminologyFreetextIndexingProviderTest.java | 6 +++--- .../freetext/ReindexTerminologyFreetextR4Test.java | 4 ++-- 8 files changed, 24 insertions(+), 23 deletions(-) diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/ReindexTerminologyCommand.java b/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/ReindexTerminologyCommand.java index 2e2cf7caf61..5ef756db3a2 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/ReindexTerminologyCommand.java +++ b/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/ReindexTerminologyCommand.java @@ -91,7 +91,7 @@ public class ReindexTerminologyCommand extends BaseRequestGeneratingCommand { ParametersUtil.addParameterToParametersBoolean(myFhirCtx, response, RESP_PARAM_SUCCESS, false); ParametersUtil.addParameterToParametersString(myFhirCtx, response, "message", "Internal error. Command result unknown. Check system logs for details"); - ourLog.info("Response:{}{}", NL, myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(response)); + ourLog.error("Response:{}{}", NL, myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(response)); return; } diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/ReindexTerminologyCommandTest.java b/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/ReindexTerminologyCommandTest.java index dd67ba95155..75be4e2f79c 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/ReindexTerminologyCommandTest.java +++ b/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/ReindexTerminologyCommandTest.java @@ -35,7 +35,6 @@ class ReindexTerminologyCommandTest { new RestfulServerExtension(myContext, myProvider); - private final PrintStream standardOut = System.out; private final ByteArrayOutputStream outputStreamCaptor = new ByteArrayOutputStream(); static { diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3450-operation-recreate-freetext-indexes-for-terminology.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3450-operation-recreate-freetext-indexes-for-terminology.yaml index 545e905cbd1..9d723e5cee8 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3450-operation-recreate-freetext-indexes-for-terminology.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3450-operation-recreate-freetext-indexes-for-terminology.yaml @@ -1,5 +1,5 @@ --- -type: fix +type: add issue: 3450 -title: "There was no way to recreate freetext indexes for terminology entities. Command line operation - reindex-terminology was added for this purpose." +title: "Previously there was no way to recreate freetext indexes for terminology entities. A new CLI operation, + reindex-terminology now exists for this purpose." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/util/ConnectionPoolInfoProvider.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/util/ConnectionPoolInfoProvider.java index 61aaad860c4..74aee0d80e0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/util/ConnectionPoolInfoProvider.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/util/ConnectionPoolInfoProvider.java @@ -15,22 +15,22 @@ import java.util.Optional; public class ConnectionPoolInfoProvider implements IConnectionPoolInfoProvider { private static final Logger ourLog = LoggerFactory.getLogger(ConnectionPoolInfoProvider.class); - private IConnectionPoolInfoProvider provider; + private IConnectionPoolInfoProvider myProvider; public ConnectionPoolInfoProvider(DataSource theDataSource) { if (theDataSource.getClass().isAssignableFrom(BasicDataSource.class)) { - provider = new BasicDataSourceConnectionPoolInfoProvider((BasicDataSource) theDataSource); + myProvider = new BasicDataSourceConnectionPoolInfoProvider((BasicDataSource) theDataSource); return; } if ( theDataSource.getClass().isAssignableFrom(ProxyDataSource.class)) { - boolean basiDataSourceWrapped = false; + boolean basiDataSourceWrapped; try { basiDataSourceWrapped = theDataSource.isWrapperFor(BasicDataSource.class); if (basiDataSourceWrapped) { BasicDataSource basicDataSource = theDataSource.unwrap(BasicDataSource.class); - provider = new BasicDataSourceConnectionPoolInfoProvider(basicDataSource); + myProvider = new BasicDataSourceConnectionPoolInfoProvider(basicDataSource); } } catch (SQLException ignored) { } } @@ -39,17 +39,17 @@ public class ConnectionPoolInfoProvider implements IConnectionPoolInfoProvider { @Override public Optional getTotalConnectionSize() { - return provider == null ? Optional.empty() : provider.getTotalConnectionSize(); + return myProvider == null ? Optional.empty() : myProvider.getTotalConnectionSize(); } @Override public Optional getActiveConnections() { - return provider == null ? Optional.empty() : provider.getActiveConnections(); + return myProvider == null ? Optional.empty() : myProvider.getActiveConnections(); } @Override public Optional getMaxWaitMillis() { - return provider == null ? Optional.empty() : provider.getMaxWaitMillis(); + return myProvider == null ? Optional.empty() : myProvider.getMaxWaitMillis(); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/BaseJpaSystemProvider.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/BaseJpaSystemProvider.java index 558ff515c0d..ff2ce315b85 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/BaseJpaSystemProvider.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/BaseJpaSystemProvider.java @@ -41,6 +41,7 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.provider.ProviderConstants; import ca.uhn.fhir.util.ParametersUtil; import ca.uhn.fhir.util.StopWatch; +import org.apache.commons.lang3.exception.ExceptionUtils; import org.hl7.fhir.instance.model.api.IBaseParameters; import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.slf4j.Logger; @@ -142,7 +143,8 @@ public class BaseJpaSystemProvider extends BaseJpaProvider implements IJp } catch (Exception theE) { throw new InternalErrorException(Msg.code(2072) + - "Re-creating terminology freetext indexes failed with exception: " + theE.getMessage()); + "Re-creating terminology freetext indexes failed with exception: " + theE.getMessage() + + NL + "With trace:" + NL + ExceptionUtils.getStackTrace(theE)); } IBaseParameters retVal = ParametersUtil.newInstance(getContext()); @@ -161,6 +163,6 @@ public class BaseJpaSystemProvider extends BaseJpaProvider implements IJp } - + public static final String NL = System.getProperty("line.separator"); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/util/ConnectionPoolInfoProviderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/util/ConnectionPoolInfoProviderTest.java index 93c9a7a3f00..10b3f6dd1a8 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/util/ConnectionPoolInfoProviderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/util/ConnectionPoolInfoProviderTest.java @@ -101,7 +101,7 @@ class ConnectionPoolInfoProviderTest { IConnectionPoolInfoProvider provider = new ConnectionPoolInfoProvider(ds); IConnectionPoolInfoProvider instantiatedProvider = - (IConnectionPoolInfoProvider) ReflectionTestUtils.getField(provider, "provider"); + (IConnectionPoolInfoProvider) ReflectionTestUtils.getField(provider, "myProvider"); assertNotNull(instantiatedProvider); assertTrue(instantiatedProvider.getClass().isAssignableFrom(BasicDataSourceConnectionPoolInfoProvider.class)); @@ -115,7 +115,7 @@ class ConnectionPoolInfoProviderTest { IConnectionPoolInfoProvider provider = new ConnectionPoolInfoProvider(proxyDs); IConnectionPoolInfoProvider instantiatedProvider = - (IConnectionPoolInfoProvider) ReflectionTestUtils.getField(provider, "provider"); + (IConnectionPoolInfoProvider) ReflectionTestUtils.getField(provider, "myProvider"); assertNotNull(instantiatedProvider); assertTrue(instantiatedProvider.getClass().isAssignableFrom(BasicDataSourceConnectionPoolInfoProvider.class)); } @@ -126,7 +126,7 @@ class ConnectionPoolInfoProviderTest { IConnectionPoolInfoProvider provider = new ConnectionPoolInfoProvider(proxyDs); IConnectionPoolInfoProvider instantiatedProvider = - (IConnectionPoolInfoProvider) ReflectionTestUtils.getField(provider, "provider"); + (IConnectionPoolInfoProvider) ReflectionTestUtils.getField(provider, "myProvider"); assertNull(instantiatedProvider); } @@ -135,7 +135,7 @@ class ConnectionPoolInfoProviderTest { IConnectionPoolInfoProvider provider = new ConnectionPoolInfoProvider(unknownDataSource); IConnectionPoolInfoProvider instantiatedProvider = - (IConnectionPoolInfoProvider) ReflectionTestUtils.getField(provider, "provider"); + (IConnectionPoolInfoProvider) ReflectionTestUtils.getField(provider, "myProvider"); assertNull(instantiatedProvider); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/TerminologyFreetextIndexingProviderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/TerminologyFreetextIndexingProviderTest.java index 0505aecf26b..7dcd8622bdc 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/TerminologyFreetextIndexingProviderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/TerminologyFreetextIndexingProviderTest.java @@ -88,15 +88,15 @@ public class TerminologyFreetextIndexingProviderTest { @Test - void testServiceThroes() throws InterruptedException { + void testServiceThrows() throws InterruptedException { String exceptionMsg = "some msg"; when(myTermReadSvc.reindexTerminology()).thenThrow(new InterruptedException(exceptionMsg)); InternalErrorException thrown = assertThrows(InternalErrorException.class, () -> testedProvider.reindexTerminology(myRequestDetails)); - assertEquals(Msg.code(2072) + "Re-creating terminology freetext indexes " + - "failed with exception: " + exceptionMsg, thrown.getMessage()); + assertTrue(thrown.getMessage().startsWith(Msg.code(2072) + "Re-creating terminology freetext indexes " + + "failed with exception: " + exceptionMsg)); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/freetext/ReindexTerminologyFreetextR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/freetext/ReindexTerminologyFreetextR4Test.java index 874c3efdea0..d8128fbb176 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/freetext/ReindexTerminologyFreetextR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/freetext/ReindexTerminologyFreetextR4Test.java @@ -55,7 +55,7 @@ import static org.junit.jupiter.api.Assertions.fail; ,ReindexTerminologyFreetextR4Test.NoopMandatoryTransactionListener.class }) public class ReindexTerminologyFreetextR4Test extends BaseJpaR4Test { - private static final Logger ourLog = LoggerFactory.getLogger(ReindexTerminologyFreetextR4Test.class); + private static final Logger ourLog = LoggerFactory.getLogger(ReindexTerminologyFreetextR4Test.class); public static final String LOINC_URL = "http://loinc.org"; public static final String TEST_FILES_CLASSPATH = "loinc-reindex/"; @@ -180,7 +180,7 @@ public class ReindexTerminologyFreetextR4Test extends BaseJpaR4Test { myTermConceptDao.countByCodeSystemVersion(termCodeSystemVersionWithNoVersionId) ); assertEquals(CS_CONCEPTS_NUMBER, dbTermConceptCountForNullVersion); - long termConceptCountNullVersion = searchAllIndexedTermConceptCount(termCodeSystemVersionWithVersionId); + long termConceptCountNullVersion = searchAllIndexedTermConceptCount(termCodeSystemVersionWithNoVersionId); ourLog.info("=================> Number of freetext found concepts after re-indexing for version {}: {}", NULL, termConceptCountNullVersion); assertEquals(CS_CONCEPTS_NUMBER, termConceptCountNullVersion);