diff --git a/api/src/main/java/org/openmrs/module/openconceptlab/CacheService.java b/api/src/main/java/org/openmrs/module/openconceptlab/CacheService.java index 3d93ff4..2eec1a3 100644 --- a/api/src/main/java/org/openmrs/module/openconceptlab/CacheService.java +++ b/api/src/main/java/org/openmrs/module/openconceptlab/CacheService.java @@ -21,9 +21,21 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; +/** + * In-memory cache layer for the OCL import pipeline. A new instance is created per import run + * in Importer.processInput() and is garbage collected when the import completes. + *

+ * Most entity caches (concepts, conceptMaps, etc.) are cleared on each flush/clear cycle via + * {@link #clearCache()}. The item URL caches ({@code itemsByUrl}, {@code checkedItemUrls}) grow + * monotonically for the lifetime of the import since they must persist across batches and phases. + * For typical imports (~15K items), this is a few MB. For very large imports (hundreds of thousands + * of items), memory usage should be monitored. + */ public class CacheService { ConceptService conceptService; @@ -59,6 +71,11 @@ public CacheService(ConceptService conceptService, OclConceptService oclConceptS // Cache for ConceptReferenceTerms keyed by "sourceId:code" Map conceptReferenceTerms = new HashMap<>(); + // Cache for item URL lookups - persists across clearCache() calls since items are used for metadata only + private final Map itemsByUrl = new HashMap<>(); + private final Set checkedItemUrls = new HashSet<>(); + private boolean skipDbItemLookups = false; + /** * Pre-loads all concept sources into the cache to avoid repeated database queries. */ @@ -116,14 +133,51 @@ private void preloadConceptMapTypes() { } /** - * Gets the last successful item for a given URL by looking up in the database. - * This searches across all previous imports to find if this URL was previously imported. + * Gets the last successful item for a given URL. Results are cached to avoid + * repeated database queries for the same URL across batches and import phases. + * The cache persists across clearCache() calls since items are used for metadata only. */ public Item getLastSuccessfulItemByUrl(String url, ImportService importService) { if (url == null) { return null; } - return importService.getLastSuccessfulItemByUrl(url, this); + + if (checkedItemUrls.contains(url)) { + return itemsByUrl.get(url); + } + + if (skipDbItemLookups) { + return null; + } + + Item item = importService.getLastSuccessfulItemByUrl(url, this); + checkedItemUrls.add(url); + if (item != null) { + itemsByUrl.put(url, item); + } + return item; + } + + /** + * Caches an item by its URL for fast lookup during the import. + * Called after successfully saving a concept or mapping to make it + * available for subsequent lookups (e.g., mapping phase looking up concept items) + * without a database query. + */ + public void cacheItem(Item item) { + if (item != null && item.getUrl() != null && item.getState() != ItemState.ERROR) { + itemsByUrl.put(item.getUrl(), item); + checkedItemUrls.add(item.getUrl()); + } + } + + /** + * When set to true, skips database lookups in getLastSuccessfulItemByUrl() for URLs + * not already in the cache. Used for first-time imports where no previous items exist, + * eliminating thousands of DB queries that would all return null. + */ + public void setSkipDbItemLookups(boolean skip) { + this.skipDbItemLookups = skip; } public void clearCache() { @@ -139,6 +193,12 @@ public void clearCache() { conceptMaps.clear(); concepts.clear(); conceptReferenceTerms.clear(); + + // Note: itemsByUrl and checkedItemUrls are intentionally NOT cleared here. + // They must persist across flush/clear cycles so the mapping phase can look up + // concept items saved earlier without DB queries. For ~15,000 items this retains + // ~15K Item objects + String keys in memory, which is acceptable. The entire + // CacheService instance is GC'd when the import run completes. } public ConceptDatatype getConceptDatatypeByName(String name) { @@ -259,13 +319,13 @@ public ConceptMap getConceptMapByUuid(String uuid, ImportService importService) ConceptMap conceptMap = conceptMaps.get(uuid); if (conceptMap != null) { return conceptMap; - } else { - conceptMap = importService.getConceptMapByUuid(uuid); - if (conceptMap != null) { - conceptMaps.put(uuid, conceptMap); - } - return conceptMap; } + + conceptMap = importService.getConceptMapByUuid(uuid); + if (conceptMap != null) { + conceptMaps.put(uuid, conceptMap); + } + return conceptMap; } public Concept getConceptByUuid(String uuid) { @@ -276,14 +336,14 @@ public Concept getConceptByUuid(String uuid) { Concept concept = concepts.get(uuid); if (concept != null) { return concept; - } else { - concept = conceptService.getConceptByUuid(uuid); - if (concept != null) { - concepts.put(uuid, concept); - } - return concept; } - } + + concept = conceptService.getConceptByUuid(uuid); + if (concept != null) { + concepts.put(uuid, concept); + } + return concept; + } public Concept getConceptWithSameAsMapping(String source, String code) { if (source == null || code == null) { @@ -291,11 +351,13 @@ public Concept getConceptWithSameAsMapping(String source, String code) { } String cacheKey = source + ":" + code; Concept concept = concepts.get(cacheKey); - if (concept == null) { - concept = oclConceptService.getConceptWithSameAsMapping(code, source); - if (concept != null) { - concepts.put(cacheKey, concept); - } + if (concept != null) { + return concept; + } + + concept = oclConceptService.getConceptWithSameAsMapping(code, source); + if (concept != null) { + concepts.put(cacheKey, concept); } return concept; diff --git a/api/src/main/java/org/openmrs/module/openconceptlab/importer/Importer.java b/api/src/main/java/org/openmrs/module/openconceptlab/importer/Importer.java index 41fe085..dd43d29 100644 --- a/api/src/main/java/org/openmrs/module/openconceptlab/importer/Importer.java +++ b/api/src/main/java/org/openmrs/module/openconceptlab/importer/Importer.java @@ -30,6 +30,7 @@ import org.openmrs.module.openconceptlab.OpenConceptLabConstants; import org.openmrs.module.openconceptlab.Subscription; import org.openmrs.module.openconceptlab.Utils; +import org.openmrs.module.openconceptlab.ValidationType; import org.openmrs.module.openconceptlab.client.OclClient; import org.openmrs.module.openconceptlab.client.OclClient.OclResponse; import org.openmrs.module.openconceptlab.client.OclConcept; @@ -54,7 +55,12 @@ public class Importer implements Runnable { private static final Logger log = LoggerFactory.getLogger(Importer.class); - public final static int BATCH_SIZE = 256; + // Number of items to process before flushing/clearing the Hibernate session. + // Higher values reduce flush/clear cycles but increase session memory usage + // (each concept carries names, descriptions, mappings, etc.). The original value + // was 256; 512 is a moderate increase that balances fewer cycles with memory + // safety across varied OpenMRS deployment environments. + public final static int BATCH_SIZE = 512; private ImportService importService; @@ -320,9 +326,12 @@ private void processInput() throws IOException { return; } + // Look up subscription once for the entire import + Subscription subscription = importService.getSubscription(); + String baseUrl = ""; - if (importService.getSubscription() != null) { - baseUrl = importService.getSubscription().getUrl(); + if (subscription != null) { + baseUrl = subscription.getUrl(); if (baseUrl != null) { try { URI uri = new URI(baseUrl); @@ -339,6 +348,21 @@ private void processInput() throws IOException { CacheService cacheService = new CacheService(conceptService, oclConceptService); + // Determine validation type once for the entire import to avoid per-concept getSubscription() calls. + ValidationType validationType = ValidationType.FULL; + if (subscription != null && subscription.getValidationType() != null) { + validationType = subscription.getValidationType(); + } + + // If this is the first-ever import, skip DB lookups for previous items since none exist. + // We check <= 1 because the current in-progress import may already be visible in the + // query results (Hibernate auto-flushes before Criteria queries). getImportsInOrder() + // returns ALL imports (no status filtering), so size 0 or 1 both mean no prior import. + List previousImports = importService.getImportsInOrder(0, 2); + if (previousImports.size() <= 1) { + cacheService.setSkipDbItemLookups(true); + } + List items = new ArrayList<>(BATCH_SIZE); while (parser.nextToken() != JsonToken.END_ARRAY) { OclConcept oclConcept = parser.readValueAs(OclConcept.class); @@ -347,8 +371,9 @@ private void processInput() throws IOException { Item item; try { - item = saver.saveConcept(cacheService, anImport, oclConcept); - log.info("Imported concept {}", oclConcept); + item = saver.saveConcept(cacheService, anImport, oclConcept, validationType); + cacheService.cacheItem(item); + log.debug("Imported concept {}", oclConcept); } catch (Throwable e) { log.error("Failed to import concept {}", oclConcept, e); Context.clearSession(); @@ -393,7 +418,8 @@ private void processInput() throws IOException { Item item; try { item = saver.saveMapping(cacheService, anImport, oclMapping); - log.info("Imported mapping {}", oclMapping); + cacheService.cacheItem(item); + log.debug("Imported mapping {}", oclMapping); } catch (SavingException e) { log.error("Failed to save mapping {}", oclMapping, e); Context.clearSession(); diff --git a/api/src/main/java/org/openmrs/module/openconceptlab/importer/Saver.java b/api/src/main/java/org/openmrs/module/openconceptlab/importer/Saver.java index db4a740..e6ccbf0 100644 --- a/api/src/main/java/org/openmrs/module/openconceptlab/importer/Saver.java +++ b/api/src/main/java/org/openmrs/module/openconceptlab/importer/Saver.java @@ -76,6 +76,15 @@ public void setImportService(ImportService importService) { * @throws ImportException */ public Item saveConcept(final CacheService cacheService, final Import anImport, final OclConcept oclConcept) throws ImportException { + return saveConcept(cacheService, anImport, oclConcept, null); + } + + /** + * Saves a concept with the specified validation type. + * When validationType is non-null, it is used directly instead of looking up the subscription each time. + * This avoids repeated getSubscription() calls for every concept in the import. + */ + public Item saveConcept(final CacheService cacheService, final Import anImport, final OclConcept oclConcept, ValidationType validationType) throws ImportException { Import thisImport = anImport; Item item = cacheService.getLastSuccessfulItemByUrl(oclConcept.getUrl(), importService); @@ -98,16 +107,18 @@ public Item saveConcept(final CacheService cacheService, final Import anImport, while (true) { try { try { - ValidationType validationType; - if (importService.getSubscription() != null) { - validationType = importService.getSubscription().getValidationType(); - } else { - validationType = ValidationType.FULL; + ValidationType effectiveValidationType = validationType; + if (effectiveValidationType == null) { + if (importService.getSubscription() != null) { + effectiveValidationType = importService.getSubscription().getValidationType(); + } else { + effectiveValidationType = ValidationType.FULL; + } } - if (ValidationType.FULL.equals(validationType)) { + if (ValidationType.FULL.equals(effectiveValidationType)) { conceptService.saveConcept(concept); - } else if (ValidationType.NONE.equals(validationType)) { + } else if (ValidationType.NONE.equals(effectiveValidationType)) { importService.updateConceptWithoutValidation(concept); } @@ -440,7 +451,7 @@ public Item saveMapping(final CacheService cacheService, final Import update, fi ItemState state; // Get any existing ConceptMap entry by uuid - ConceptMap conceptMap = importService.getConceptMapByUuid(oclMapping.getExternalId()); + ConceptMap conceptMap = cacheService.getConceptMapByUuid(oclMapping.getExternalId(), importService); // If we find an existing Map by uuid, update it to match the passed in mapping if (conceptMap != null) { diff --git a/api/src/test/java/org/openmrs/module/openconceptlab/CacheServiceTest.java b/api/src/test/java/org/openmrs/module/openconceptlab/CacheServiceTest.java index 2503ff2..6006021 100644 --- a/api/src/test/java/org/openmrs/module/openconceptlab/CacheServiceTest.java +++ b/api/src/test/java/org/openmrs/module/openconceptlab/CacheServiceTest.java @@ -12,15 +12,21 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mock; +import org.openmrs.ConceptMap; import org.openmrs.ConceptSource; import org.openmrs.api.ConceptService; +import org.openmrs.module.openconceptlab.client.OclConcept; import java.util.ArrayList; import java.util.List; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class CacheServiceTest extends MockTest { @@ -28,6 +34,9 @@ public class CacheServiceTest extends MockTest { @Mock ConceptService conceptService; + @Mock + ImportService importService; + static final ConceptSource ICD_10 = conceptSource("ICD-10"); static final ConceptSource SNOMED_DASH_CT = conceptSource("SNOMED-CT"); static final ConceptSource SNOMED_SPACE_CT = conceptSource("Snomed CT"); @@ -84,4 +93,125 @@ public void cacheServiceShouldThrowAnExceptionIfNoExactConceptSourcesAndMultiple } assertThat(actualException, notNullValue()); } + + @Test + public void getLastSuccessfulItemByUrl_shouldCacheResultsAcrossCalls() { + CacheService cacheService = new CacheService(conceptService, null); + String url = "/orgs/OCL/sources/Diagnoses/concepts/123/"; + Item item = createItem(url, "v1", ItemState.ADDED); + when(importService.getLastSuccessfulItemByUrl(url, cacheService)).thenReturn(item); + + Item first = cacheService.getLastSuccessfulItemByUrl(url, importService); + Item second = cacheService.getLastSuccessfulItemByUrl(url, importService); + + assertThat(first, is(item)); + assertThat(second, is(item)); + verify(importService, times(1)).getLastSuccessfulItemByUrl(url, cacheService); + } + + @Test + public void getLastSuccessfulItemByUrl_shouldCacheNullResults() { + CacheService cacheService = new CacheService(conceptService, null); + String url = "/orgs/OCL/sources/Diagnoses/concepts/999/"; + when(importService.getLastSuccessfulItemByUrl(url, cacheService)).thenReturn(null); + + Item first = cacheService.getLastSuccessfulItemByUrl(url, importService); + Item second = cacheService.getLastSuccessfulItemByUrl(url, importService); + + assertThat(first, nullValue()); + assertThat(second, nullValue()); + verify(importService, times(1)).getLastSuccessfulItemByUrl(url, cacheService); + } + + @Test + public void getLastSuccessfulItemByUrl_shouldSkipDbWhenSkipDbItemLookupsIsTrue() { + CacheService cacheService = new CacheService(conceptService, null); + cacheService.setSkipDbItemLookups(true); + String url = "/orgs/OCL/sources/Diagnoses/concepts/123/"; + + Item result = cacheService.getLastSuccessfulItemByUrl(url, importService); + + assertThat(result, nullValue()); + verify(importService, never()).getLastSuccessfulItemByUrl(url, cacheService); + } + + @Test + public void getLastSuccessfulItemByUrl_shouldReturnCachedItemEvenWhenSkipDbItemLookupsIsTrue() { + CacheService cacheService = new CacheService(conceptService, null); + String url = "/orgs/OCL/sources/Diagnoses/concepts/123/"; + Item item = createItem(url, "v1", ItemState.ADDED); + cacheService.cacheItem(item); + cacheService.setSkipDbItemLookups(true); + + Item result = cacheService.getLastSuccessfulItemByUrl(url, importService); + + assertThat(result, is(item)); + verify(importService, never()).getLastSuccessfulItemByUrl(url, cacheService); + } + + @Test + public void cacheItem_shouldNotCacheErrorItems() { + CacheService cacheService = new CacheService(conceptService, null); + String url = "/orgs/OCL/sources/Diagnoses/concepts/456/"; + Item errorItem = createItem(url, "v1", ItemState.ERROR); + cacheService.cacheItem(errorItem); + + // URL not in cache, so should fall through to DB lookup + when(importService.getLastSuccessfulItemByUrl(url, cacheService)).thenReturn(null); + Item result = cacheService.getLastSuccessfulItemByUrl(url, importService); + + assertThat(result, nullValue()); + verify(importService, times(1)).getLastSuccessfulItemByUrl(url, cacheService); + } + + @Test + public void cacheItem_shouldPersistAcrossClearCacheCalls() { + CacheService cacheService = new CacheService(conceptService, null); + String url = "/orgs/OCL/sources/Diagnoses/concepts/789/"; + Item item = createItem(url, "v1", ItemState.ADDED); + cacheService.cacheItem(item); + + cacheService.clearCache(); + + Item result = cacheService.getLastSuccessfulItemByUrl(url, importService); + assertThat(result, is(item)); + verify(importService, never()).getLastSuccessfulItemByUrl(url, cacheService); + } + + @Test + public void getConceptMapByUuid_shouldCacheResults() { + CacheService cacheService = new CacheService(conceptService, null); + String uuid = "map-uuid-123"; + ConceptMap map = new ConceptMap(); + when(importService.getConceptMapByUuid(uuid)).thenReturn(map); + + ConceptMap first = cacheService.getConceptMapByUuid(uuid, importService); + ConceptMap second = cacheService.getConceptMapByUuid(uuid, importService); + + assertThat(first, is(map)); + assertThat(second, is(map)); + verify(importService, times(1)).getConceptMapByUuid(uuid); + } + + @Test + public void getConceptMapByUuid_shouldStillQueryDbWhenSkipDbItemLookupsIsTrue() { + CacheService cacheService = new CacheService(conceptService, null); + cacheService.setSkipDbItemLookups(true); + String uuid = "map-uuid-456"; + ConceptMap map = new ConceptMap(); + when(importService.getConceptMapByUuid(uuid)).thenReturn(map); + + ConceptMap result = cacheService.getConceptMapByUuid(uuid, importService); + + assertThat(result, is(map)); + verify(importService, times(1)).getConceptMapByUuid(uuid); + } + + private Item createItem(String url, String versionUrl, ItemState state) { + OclConcept concept = new OclConcept(); + concept.setUrl(url); + concept.setVersionUrl(versionUrl); + concept.setExternalId("ext-" + url.hashCode()); + return new Item(null, concept, state); + } } diff --git a/api/src/test/java/org/openmrs/module/openconceptlab/importer/ImporterTest.java b/api/src/test/java/org/openmrs/module/openconceptlab/importer/ImporterTest.java index 1c91ed7..92331f8 100644 --- a/api/src/test/java/org/openmrs/module/openconceptlab/importer/ImporterTest.java +++ b/api/src/test/java/org/openmrs/module/openconceptlab/importer/ImporterTest.java @@ -41,6 +41,7 @@ import org.openmrs.module.openconceptlab.client.OclClient; import org.openmrs.module.openconceptlab.client.OclClient.OclResponse; import org.openmrs.module.openconceptlab.client.OclConcept; +import org.openmrs.module.openconceptlab.ValidationType; import org.openmrs.module.openconceptlab.client.OclMapping; import org.openmrs.test.BaseContextMockTest; @@ -179,7 +180,7 @@ public Item answer(InvocationOnMock invocation) throws Throwable { OclConcept oclConcept = (OclConcept) invocation.getArguments()[2]; return new Item(update, oclConcept, ItemState.ADDED); } - }).when(saver).saveConcept(any(CacheService.class), any(Import.class), any(OclConcept.class)); + }).when(saver).saveConcept(any(CacheService.class), any(Import.class), any(OclConcept.class), any(ValidationType.class)); doAnswer(new Answer() {